[improve][cli]Pulsar Dynamic Configuration Feature for Broker-level Configuration #20052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gaozhangmin
requested review from
coderzc,
mattisonchao and
Technoboy-
and removed request for
coderzc and
mattisonchao
|
@gaozhangmin Please add the following content to your PR description and select a checkbox: |
github-actions
bot
added
doc-required
gaozhangmin
changed the title
gaozhangmin
changed the title
gaozhangmin
changed the title
tisonkun
reviewed
Apr 20, 2023
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/BrokersBase.java
Outdated
Show resolved
Hide resolved
gaozhangmin
force-pushed
the
dynamic-configuration
branch
from
5e0f321 to
57988a3
Compare
Codecov Report
@@ Coverage Diff @@
## master #20052 +/- ##
============================================
+ Coverage 72.25% 72.95% +0.70%
- Complexity 31946 31959 +13
============================================
Files 1855 1868 +13
Lines 138228 138486 +258
Branches 15224 15241 +17
============================================
+ Hits 99871 101034 +1163
+ Misses 30397 29404 -993
- Partials 7960 8048 +88
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
The pr had no activity for 30 days, mark with Stale label. |
tisonkun
reviewed
May 22, 2023
| @@ -69,29 +69,38 @@ private class UpdateConfigurationCmd extends CliCommand { | |||
| @Parameter(names = {"-v", "--value"}, description = "service-configuration value", required = true) | |||
| private String configValue; | |||
|
|
|||
| @Parameter(names = {"-s", "--scope"}, description = "cluster or broker address, default is cluster", | |||
There was a problem hiding this comment.
Shall we use an enum Scope instead of String here? Ditto other occurance.
There was a problem hiding this comment.
Scope is either cluster or broker address。 unable to enumerate
|
cc @Technoboy- @AnonHxy maybe you can give a review also? |
|
The pr had no activity for 30 days, mark with Stale label. |
gaozhangmin
force-pushed
the
dynamic-configuration
branch
from
57988a3 to
7f3e408
Compare
|
Closed as stale and conflict. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
Currently, Pulsar's Dynamic Configuration Feature only supports cluster-level configuration, meaning that any changes made to the dynamic configuration apply to all brokers in the cluster.
This pull request proposes a new feature that allows for broker-level configuration, meaning that changes made to the dynamic configuration will only affect specific machines in the cluster, rather than all of them.
Modifications
Add new parameters
scopeto admin commandsupdate-dynamic-config,delete-dynamic-config,get-all-dynamic-config, scope value is cluster or broker's LookupServiceAddres.Verifying this change
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: gaozhangmin#12