Split compression size threshold from compressor #113
Conversation
| min int | ||
| } | ||
|
|
||
| // WithCompressMinBytes sets a minimum size threshold for compression: |
There was a problem hiding this comment.
I am not crazy about the name, but I can't really think of a better one. Maybe CompressThresholdBytes, but I'm assuming you thought of that. "compress min bytes" reads to me as "compress at least this number of bytes no matter what", which obviously doesn't make sense (what if I have less than that number of bytess?)
There was a problem hiding this comment.
WithCompressionThreshold?
Right now, it matches WithReadMaxBytes. If we change, should the two continue to match?
There was a problem hiding this comment.
I don't know here - I feel like they're slightly different, ReadMaxBytes says "error if you read more than this", while this option says "if you cross this number, don't do an action" - I think I'm being pedantic, you might have it right. Maybe it should be ReadMaxBytes and CompressMinBytes
There was a problem hiding this comment.
WithUncompressedSmallMessages(threshold int)?
I'm indifferent, you pick.
There was a problem hiding this comment.
No you. Lol.
WithCompressMinBytes is fine
I feel that this is an important and obvious optimization to allow users
to configure. After talking to @bufdev about #108, though, I'm convinced
that we should split it into a separate option from the compressor
itself and default to a less-surprising value.