You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/03/24 12:10:57 UTC

[GitHub] [pulsar] Technoboy- opened a new pull request #14850: [improve][broker] Add log when update namespace policies with error.

Technoboy- opened a new pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850


   ### Motivation
   
   When namespace policies update, it will update the cache. There is a long piece of logic in the method PersistentTopic#onPoliciesUpdate :
   https://github.com/apache/pulsar/blob/b5b0967f12174ba35baaf25092ac521f281e6b7d/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java#L2371-L2438
   
   There is no log to track if some of the futures go to failure. so it's hard to troubleshoot the problem when error.
   #14266 is one of the cases.
   
   ### Documentation
     
   - [x] `no-need-doc` 
     
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #14850: [improve][broker] Add log when update namespace policies with error.

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850#issuecomment-1082492182


   @Technoboy- Please help push a PR to branch-2.9 directly, I'm not able to cherry-pick it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Technoboy- merged pull request #14850: [improve][broker] Add log when update namespace policies with error.

Posted by GitBox <gi...@apache.org>.
Technoboy- merged pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Technoboy- commented on a change in pull request #14850: [improve][broker] Add log when update namespace policies with error.

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on a change in pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850#discussion_r835837377



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -2433,6 +2433,9 @@ private boolean shouldTopicBeRetained() {
                 return CompletableFuture.allOf(replicationFuture, dedupFuture, persistentPoliciesFuture,
                         preCreateSubscriptionForCompactionIfNeeded());
             });
+        }).exceptionally(ex -> {
+            log.error("update namespace polices : {} error", data, ex);

Review comment:
       Yes, right. added.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] mattisonchao commented on a change in pull request #14850: [improve][broker] Add log when update namespace policies with error.

Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850#discussion_r835769573



##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java
##########
@@ -2433,6 +2433,9 @@ private boolean shouldTopicBeRetained() {
                 return CompletableFuture.allOf(replicationFuture, dedupFuture, persistentPoliciesFuture,
                         preCreateSubscriptionForCompactionIfNeeded());
             });
+        }).exceptionally(ex -> {
+            log.error("update namespace polices : {} error", data, ex);

Review comment:
       Could we add a namespace name to show which namespace update policy fail?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Technoboy- commented on pull request #14850: [improve][broker] Add log when update namespace policies with error.

Posted by GitBox <gi...@apache.org>.
Technoboy- commented on pull request #14850:
URL: https://github.com/apache/pulsar/pull/14850#issuecomment-1079817037


   > Do we need to add this log to the non-persistent topic?
   
   Yes, right. Updated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org