-
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
[broker][admin]Add api for update topic properties #17238
Conversation
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
Outdated
Show resolved
Hide resolved
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
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/impl/PersistentTopicsBase.java
Show resolved
Hide resolved
pulsar-broker/src/main/java/org/apache/pulsar/broker/admin/v2/PersistentTopics.java
Show resolved
Hide resolved
pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/Topics.java
Show resolved
Hide resolved
Can you help open a new PR to branch-2.10? There are a lot conflict when cherry-pick directly. @AnonHxy |
@Jason918 we cannot add new APIs on released branches |
Sure, I will remove the label. |
(cherry picked from commit b21f728)
(cherry picked from commit b21f728)
(cherry picked from commit b21f728)
|
||
@Override | ||
public void updatePropertiesComplete(Map<String, String> properties, Object ctx) { | ||
managedLedger.getConfig().getProperties().putAll(properties); |
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.
managedLedger.getConfig().getProperties()
can return NPE if the topic is from the older code without the properties
.
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.
Thanks for you to fix it!
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) (cherry picked from commit 6f8dbc7) * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) (cherry picked from commit 723ad75) * [fix][broker] Fix NPE when update topic properties. ### Motivation In Pulsar 2.10, when creating a producer/consumer that auto-creates a topic and then attempts to update its properties, a NullPointerException (NPE) occurs. The reason is that Pulsar 2.10 does not correctly handle the case when the properties are `null`. ### Modifications Add an NPE check. (cherry picked from commit 0b02321) * remove release profile * import * Revert "remove release profile" This reverts commit 6bec650. --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) (cherry picked from commit 6f8dbc7) * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) (cherry picked from commit 723ad75) * [fix][broker] Fix NPE when update topic properties. ### Motivation In Pulsar 2.10, when creating a producer/consumer that auto-creates a topic and then attempts to update its properties, a NullPointerException (NPE) occurs. The reason is that Pulsar 2.10 does not correctly handle the case when the properties are `null`. ### Modifications Add an NPE check. (cherry picked from commit 0b02321) * remove release profile * import * Revert "remove release profile" This reverts commit 6bec650. --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
(cherry picked from commit b21f728) Signed-off-by: Zixuan Liu <[email protected]>
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) Signed-off-by: Zixuan Liu <[email protected]> * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) Signed-off-by: Zixuan Liu <[email protected]> * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) Signed-off-by: Zixuan Liu <[email protected]> * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) Signed-off-by: Zixuan Liu <[email protected]> * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) Signed-off-by: Zixuan Liu <[email protected]> * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) Signed-off-by: Zixuan Liu <[email protected]> * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
* [broker][admin]Add api for update topic properties (apache#17238) (cherry picked from commit b21f728) Signed-off-by: Zixuan Liu <[email protected]> * [broker][admin] Add cmd to remove topic properties (apache#17337) * [broker][admin] Add cmd to remove topic properties * address comment * address comment (cherry picked from commit 7075a5c) Signed-off-by: Zixuan Liu <[email protected]> * [fix][broker] Fix NPE when updating topic's properties (apache#17352) Co-authored-by: bjhuxiaohua <[email protected]> (cherry picked from commit f1d1158) --------- Co-authored-by: Xiaoyu Hou <[email protected]> Co-authored-by: Ruguo Yu <[email protected]> Co-authored-by: Flowermin <[email protected]>
Motivation
Maybe this modification should be part of PIP 110: Topic metadata #12629 . As described in the PIP document "These metadata could be set during topic creation and also updated.", we have implemented the
creation
method, but have not implement theupdated
method.This PR will implement the topic meta update
Modifications
updateProperties
Verifying this change
org.apache.pulsar.broker.admin.AdminApi2Test#testUpdatePartitionedTopicProperties
org.apache.pulsar.broker.admin.AdminApi2Test#testUpdateNonPartitionedTopicProperties
Documentation
Check the box below or label this PR directly.
Need to update docs?
doc-required
(Your PR needs to update docs and you will update later)
doc-not-needed
(Please explain why)
doc
(Your PR contains doc changes)
doc-complete
(Docs have been already added)