-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][admin]allowAutoTopicCreation and allowAutoTopicCreationType etc should be dynamically configured #16023
[fix][admin]allowAutoTopicCreation and allowAutoTopicCreationType etc should be dynamically configured #16023
Conversation
…ationType etc should be dynamically configured
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.
The change looks good. Could you please add a UT for the new change?
Here is an example PR #13121 which added subscriptionTypesEnabled
dynamic configuration support.
OK, I will supplement the corresponding unit tests as soon as possible |
…ts. add description of brokerDeleteInactivePartitionedTopicMetadataEnabled to doc.
3dd5565
to
b6f768c
Compare
Unit tests that fail to run also happen on the master branch, it doesn't look like it's caused by my commits, I'll try my best to take the time to fix them. |
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.
Overall LGTM, just left some small comments.
...roker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceAutoTopicCreationTest.java
Outdated
Show resolved
Hide resolved
...roker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceAutoTopicCreationTest.java
Outdated
Show resolved
Hide resolved
...roker/src/test/java/org/apache/pulsar/broker/service/BrokerServiceAutoTopicCreationTest.java
Outdated
Show resolved
Hide resolved
Co-authored-by: Kai Wang <kwang@streamnative.io>
…topicConfigDynamically
/pulsarbot run-failure-checks |
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
/pulsarbot run-failure-checks |
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/InactiveTopicDeleteTest.java
Outdated
Show resolved
Hide resolved
...rc/test/java/org/apache/pulsar/broker/service/BrokerServiceAutoSubscriptionCreationTest.java
Outdated
Show resolved
Hide resolved
@Radiancebobo Please select only one documentation label for your PR. |
pulsar-broker/src/test/java/org/apache/pulsar/broker/service/InactiveTopicDeleteTest.java
Outdated
Show resolved
Hide resolved
…gestions from code review
…gestions from code review
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.
Could you add atMost
to other tests of InactiveTopicDeleteTest
as well?
ok |
…gestions from code review
/pulsarbot run-failure-checks |
1 similar comment
/pulsarbot run-failure-checks |
@BewareMyPower Can you help me merge this branch to master? |
… should be dynamically configured (apache#16023)
… should be dynamically configured (apache#16023)
… should be dynamically configured (apache#16023) (cherry picked from commit 0d8c7ef) Signed-off-by: Zixuan Liu <nodeces@gmail.com>
… should be dynamically configured (apache#16023) (cherry picked from commit 0d8c7ef) Signed-off-by: Zixuan Liu <nodeces@gmail.com>
… should be dynamically configured (apache#16023) (cherry picked from commit 0d8c7ef) Signed-off-by: Zixuan Liu <nodeces@gmail.com>
allowAutoTopicCreation and allowAutoTopicCreationType etc should be dynamically configured
Fixes #16020
Motivation
In the broker.conf configuration, there are these configurations:
I think these should be dynamically configurable.
Pulsar has namespace-level policy, but the broker-level dynamically configuration is missing.
Modifications
The change is very simple, just add dynamic identification to these configurations in ServiceConfiguration.java, and add a callback function when updating.
Documentation
Need to update docs?
doc-required
(Your PR needs to update docs and you will update later)
doc-complete
(Docs have been already added)