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(java): configurable buffer size limit #1963

Merged
merged 1 commit into from
Dec 5, 2024

Conversation

theigl
Copy link
Contributor

@theigl theigl commented Dec 1, 2024

What does this PR do?

This PR introduces a new configuration option bufferSizeLimitBytes that replaces the hard-coded default of 128kb.

Related issues

#1950

Does this PR introduce any user-facing change?

The PR introduces a new configuration option bufferSizeLimitBytes.

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Discussion

This PR solves my problem, but I'm not sure if it is the right way to move forward. This is quite a low-level configuration option, but a potentially very important one. Every user whose average payload size is >=128kb, will need to increase this value for maximum performance. Maybe the default limit should be increased to something less conservative like 1MB, so fewer users will need to adjust this setting?

@theigl theigl requested a review from chaokunyang as a code owner December 1, 2024 14:11
@chaokunyang
Copy link
Collaborator

I think bufferSizeLimitBytes is OK, if we adjust the default value to bigger, fury will take up many memory. For example, if we have a thound of Fury instance, and every fury instance takes up 1M memory, Fury will take up 1G memory, which may be unexpected.

Copy link
Collaborator

@chaokunyang chaokunyang left a comment

Choose a reason for hiding this comment

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

LGTM

@chaokunyang chaokunyang merged commit b3f531c into apache:main Dec 5, 2024
37 checks passed
@theigl theigl deleted the 1950-buffer-size-limit branch December 5, 2024 13:57
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.

2 participants