tele(taskbroker): check zlib compression viability #91693
Conversation
github-actions
bot
added
the
Scope: Backend
src/sentry/options/defaults.py
Outdated
| register( | ||
| "taskworker.try_compress.profile_metrics", | ||
| default=False, | ||
| type=Bool, |
There was a problem hiding this comment.
Shoulds this be a float, so we could sample instead of compressing all or nothing?
There was a problem hiding this comment.
Sounds good, I made it a float. Also moved the uncompress length metric to outside of this option flag so we can get a better picture of the sizes we're dealing with
john-z-yang
force-pushed
the
john/check-compress-param
branch
from
acc3484 to
b75f430
Compare
| start_time = time.perf_counter() | ||
| metrics.distribution( | ||
| "profiling.profile_metrics.compressed_bytes", | ||
| len(b64encode(zlib.compress(message.payload.value))), |
There was a problem hiding this comment.
So zipping a base64 encoded msgpack payload 🤔 Wouldn't zlib.compress() return bytes which also need to be b64 encoded, as not all bytes can be json encoded.
Should we be zipping the msgpack, and then b64 encoding the zip? That might entirely defeat the benefits of zip though.
There was a problem hiding this comment.
Oh I did not know that message.payload.value is a msgpack payload. Why do we b64 encode it?
Ideally we should zip the bytes and then b64 encode the zipped values.
There was a problem hiding this comment.
We currently have to b64 encode the msgpack, as not all bytes can be json encoded 😢
There was a problem hiding this comment.
I think I got myself mixed up here 😵 . I see now that you're doing b64(zip(msgpack)) which is as good as we can get for now.
john-z-yang
force-pushed
the
john/check-compress-param
branch
from
b75f430 to
97d002b
Compare
john-z-yang
force-pushed
the
john/check-compress-param
branch
from
97d002b to
1acbc4f
Compare
john-z-yang
force-pushed
the
john/check-compress-param
branch
from
1acbc4f to
01b0a77
Compare
No description provided.