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

feat(producer): add MaxBufferBytes to limit retry buffer size #3088

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

wanwenli
Copy link
Contributor

@wanwenli wanwenli commented Jan 28, 2025

Summary

This PR introduces the MaxBufferBytes configuration to Sarama’s producer, allowing users to limit the retry buffer size based on the total byte size of messages. This complements the existing MaxBufferLength configuration, which limits the number of messages in the retry buffer. The implementation ensures more robust control over memory usage and prevents potential OOM issues when dealing with large messages.

The changes are built upon the foundation laid in #3026, further enhancing the retry buffer’s configurability and usability.

Key Changes

  • Added a new Producer.Retry.MaxBufferBytes configuration.
  • The type of MaxBufferBytes is int64, as the maximum value of int32 (approximately 2 GB) is insufficient for buffers holding many messages of large average size, such as 1 MB. Using int64 ensures the buffer can handle higher limits if needed.
  • Ensured that any value below the functional lower limit of 32 MB is pushed to this minimum.
  • Updated the retry handler to enforce both MaxBufferBytes and MaxBufferLength constraints.
  • Added relevant unit tests to validate functionality for both configurations, including edge cases and scenarios with unlimited settings.

This enhancement allows users to fine-tune producer behavior, particularly when working with large message payloads or memory-constrained environments.

@wanwenli wanwenli force-pushed the limit-producer-retry-buffer-by-bytes branch 2 times, most recently from e7f5cbc to 1524102 Compare January 28, 2025 01:13
@wanwenli wanwenli force-pushed the limit-producer-retry-buffer-by-bytes branch from 1524102 to e9ec73a Compare January 28, 2025 01:28
@wanwenli
Copy link
Contributor Author

@dnwe can you please review this PR? Thanks!

Copy link
Contributor

@puellanivis puellanivis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing left to comment about is a possible alternative, that may or may not be an improvement.

Otherwise, this looks like it does what it intends.

@wanwenli wanwenli force-pushed the limit-producer-retry-buffer-by-bytes branch from e9ec73a to 7291db1 Compare January 28, 2025 05:40
@dnwe dnwe added the feat label Jan 28, 2025
Copy link
Collaborator

@dnwe dnwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this contribution, and for refining it based on the review feedback. The changes LGTM!

@dnwe dnwe merged commit 26d7c7b into IBM:main Jan 28, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants