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.

Sign up for GitHub

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

Jump to bottom

v2.11: Disable max_buffered_msgs by default, bump max_buffered_size #6633

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

v2.11: Disable max_buffered_msgs by default, bump max_buffered_size #6633

wants to merge 1 commit into from
+2 −2

Conversation

wallyqs
Copy link
Member

@wallyqs wallyqs commented Mar 10, 2025

  • Bump max_buffered_size to 256MB by default
  • Disable max_buffered_msgs and make it opt-in

This will avoid running into IPQ len on simple benchmarks or use cases that the server handles fine, the max_buffered_msgs is enough to protect the memory usage in most cases.

@wallyqs
Disable max_buffered_msgs by default, bump max_buffered_size
a0caae5 
- Bump max_buffered_size to 256MB by default
- Disable max_buffered_msgs and make it opt-in

This will avoid running into IPQ len on simple benchmarks or use cases
that the server handles fine, the max_buffered_msgs is enough to protect
the memory usage in most cases.

Signed-off-by: Waldemar Quevedo <wally@nats.io>
@wallyqs wallyqs requested a review from a team as a code owner March 10, 2025 22:03
neilalexander
neilalexander reviewed Mar 11, 2025
View reviewed changes
Copy link
Member

@neilalexander neilalexander left a comment

Choose a reason for hiding this comment

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

In principle I think this is OK, as we discussed last week, but I think it needs some kind of test to prove that the server copes and there aren't secondary effects.

Maybe copy TestJetStreamRateLimitHighStreamIngest into a new long-running test to prove it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@neilalexander neilalexander

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@wallyqs @neilalexander