-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[improve][proxy] When adding new brokers resolve the DNS name more quickly #21207
[improve][proxy] When adding new brokers resolve the DNS name more quickly #21207
Conversation
@frankjkelly Please add the following content to your PR description and select a checkbox:
|
CC: @Technoboy- @tisonkun @lhotari as they touched |
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.
Thank you!
I suggest this to be an improvement rather than a bugfix? |
Makes sense - done. |
Merging.. Thanks for your contribution! |
Thanks @tisonkun - if I wanted to backport this change to active branches (e.g. 3.1/3.0/2.11/2.10 etc) does that make sense? |
@frankjkelly thanks for your question! I'd hear from @codelipenghui @Technoboy- & @eolivelli. Generally, I'm OK to backport this patch. For the target branches, 3.0 and 3.1 is undoubtedly supported, but I'm not sure for 2.10 and 2.11. The best way for such discussion would be that you send an email to the dev@pulsar.apache.org mailing list. Subscribe following the instruction here, and refer to previous discussions like this one. |
I think this is a bug, and I would like to backport this patch to the |
Fixes #21206
Motivation
When autoscaling brokers, the current default negative ttl is 10 seconds so it might take up to 10 seconds before the proxy realizes the broker was available and this could lead to errors at the pulsar client if the operation timeout was below 10 seconds .
Similarly this should have positive impact for broker --> bookie (when new bookies are autoscaling).
Modifications
Added a line to the Dockerfile to override the negative.ttl in the
java.security
fileVerifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: