Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: allow skipping per message size check #2892

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ae-govau
Copy link

@ae-govau ae-govau commented May 8, 2024

This is useful when producer compression is enabled, as it allows for much larger messages to be sent to the broker (providing the entire batch they are in stays small enough).

The previous workaround suggested in #2142 causes other ill-effects related to message batching (such as in #2797).

Fixes #2142, probably also #2797 and may help mitigate not having merged #2851.

It's not clear how easy it is to write a test for this but happy to try if we're happy enough with this approach.

This is useful when producer compression is enabled, as it allows for
much larger messages to be sent to the broker (providing the entire
batch they are in stays small enough).

Signed-off-by: Adam Eijdenberg <adam.eijdenberg@defence.gov.au>
@ae-govau
Copy link
Author

ae-govau commented May 8, 2024

This still doesn't solve everything -

sarama/produce_set.go

Lines 253 to 255 in 0ab2bb7

case ps.msgs[msg.Topic] != nil && ps.msgs[msg.Topic][msg.Partition] != nil &&
ps.msgs[msg.Topic][msg.Partition].bufferBytes+msg.ByteSize(version) >= ps.parent.conf.Producer.MaxMessageBytes:
return true
still is checking a before-compression amount for calculating the batch, but I think the above is probably better than erroneously returning errors for small messages when MaxMessageBytes is set too high.

@puellanivis
Copy link
Contributor

puellanivis commented May 8, 2024

The phrasing of “skip” here is potentially confusing, especially when batch processing. Not sure there’s a better phrasing possible though.

PS: Maybe Disable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compressed size is not used for validating message in producer when compression is enabled
2 participants