-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[pulsar-broker] load-balancer support disabling max-session for bundle split #13108
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
Conversation
|
@rdhabalia:Thanks for your contribution. For this PR, do we need to update docs? |
lhotari
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
michaeljmarshall
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| @FieldContext( | ||
| category = CATEGORY_LOAD_BALANCER, | ||
| doc = "maximum sessions (producers + consumers) in a bundle, otherwise bundle split will be triggered" | ||
| + "(disable threshold check with value -1)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
| + "(disable threshold check with value -1)" | |
| + " (disable threshold check with value -1)" |
| loadBalancerNamespaceBundleMaxTopics=1000 | ||
|
|
||
| # maximum sessions (producers + consumers) in a bundle, otherwise bundle split will be triggered | ||
| # (disable threshold check with value -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rdhabalia for the doc side, there are 2 loadBalancerNamespaceBundleMaxSessions in the Pulsar configuration page, need to add the explanations for both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rdhabalia - will you be able to update this documentation? If not, I still think we should merge this PR and maybe someone can follow up with a PR to update the docs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, these config are not related to this change and already exist, but will create a separate PR to address documentation concerns.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rdhabalia do not forget to update docs, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rdhabalia any updates on the docs?

Motivation
Right now, Bundle split task considers max-session count on topic to split bundle and in some usecases, we want to disable this threshold-check because some usecases have expected number of high producer/consumer connection and we don't want to split based on this criteria. so, provide a way to disable this check by setting -1 value which skips the threshold check.