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/08/10 06:41:12 UTC

[GitHub] [pulsar] lordcheng10 opened a new pull request, #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

lordcheng10 opened a new pull request, #17035:
URL: https://github.com/apache/pulsar/pull/17035

   ### Motivation
   We modified some cache dynamic configurations, such as: managedLedgerCacheSizeMB=100, but after we restarted the broker, we found that the dynamically modified cache configuration was invalid, and the value configured in broker.conf took effect.
   
   After restarting, the loading process of config is as follows:
   1. Load the broker.conf file and initialize the ServiceConfiguration object;
   
   2. Use the ServiceConfiguration object to build the ManagedLedgerStorage object:
   https://github.com/apache/pulsar/blob/4b757cf9f9046c7143329156b1009fe43217eaea/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L742~L746
   
   
   3. When creating the BrokerService object, it will read the dynamic configuration on zookeeper, update it to conf, and then register the listener for the relevant configuration update, but the execution of these listeners is not triggered:
   https://github.com/apache/pulsar/blob/4b757cf9f9046c7143329156b1009fe43217eaea/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java#L746~L748
   https://github.com/apache/pulsar/blob/4b757cf9f9046c7143329156b1009fe43217eaea/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java#L2205~L2250
   
   
   Therefore, after the broker is restarted, the cache-related configuration used is still the configuration in the broker.conf file.
   
   Solution:
   The listeners that load and trigger related configuration updates are loaded in the following order:
   1. Register and configure the listener;
   2. Read the dynamic configuration on zookeeper and trigger the corresponding listener;
   
   
   
   ### 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)
     
   - [x] `doc-not-needed` 
   (Please explain why)
     
   - [ ] `doc` 
   (Your PR contains doc changes)
   
   - [ ] `doc-complete`
   (Docs have been already 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 pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
mattisonchao commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1244833654

   Hi @lordcheng10  
   It looks like we got many conflicts when cherry-picking it to branch-2.9.
   Would you mind pushing a PR to branch-2.9? (To avoid cherry-picking involving bugs)


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1210514442

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1236028780

   /pulsarbot run-failure-checks


-- 
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] HQebupt commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
HQebupt commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1213181627

   Good catch 👍 


-- 
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] Jason918 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
Jason918 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1229155552

   @lordcheng10 Please solve CI failure.


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235073672

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1237591459

   > @lordcheng10 Please solve CI failure.
   
   Fixed,PTAL,thanks!  @codelipenghui @Jason918 


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228590296

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1236887283

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235317642

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228358150

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228307824

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235386596

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on a diff in pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on code in PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#discussion_r963329437


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -2113,33 +2114,44 @@ private void handlePoliciesUpdates(NamespaceName namespace) {
     }
 
     private void handleDynamicConfigurationUpdates() {
-        pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync()
-                .thenAccept(optMap -> {
-                    if (!optMap.isPresent()) {
-                        return;
-                    }
-                    Map<String, String> data = optMap.get();
-                    data.forEach((configKey, value) -> {
-                        Field configField = dynamicConfigurationMap.get(configKey).field;
-                        Object newValue = FieldParser.value(data.get(configKey), configField);
-                        if (configField != null) {
-                            Consumer listener = configRegisteredListeners.get(configKey);
-                            try {
-                                Object existingValue = configField.get(pulsar.getConfiguration());
-                                configField.set(pulsar.getConfiguration(), newValue);
-                                log.info("Successfully updated configuration {}/{}", configKey,
-                                        data.get(configKey));
-                                if (listener != null && !existingValue.equals(newValue)) {
-                                    listener.accept(newValue);
+        DynamicConfigurationResources dynamicConfigResources = null;
+        try {
+            dynamicConfigResources = pulsar()
+                    .getPulsarResources()
+                    .getDynamicConfigResources();

Review Comment:
   1. This is just to get the object, it will not block the thread.
   2. The try catch here is just to catch the situation where getPulsarResources or getDynamicConfigResources returns null, because some unit tests do not initialize pulsarResources or dynamicConfigResources.
   3. Try catch is also performed in the original call of updateDynamicServiceConfiguration:
   <img width="1010" alt="image" src="https://user-images.githubusercontent.com/19296967/188568999-c3e6aa55-c94d-4e1e-8514-4955b0651d44.png">
   



-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1211485548

   
   /pulsarbot run-failure-checks
   
   


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1236138181

   /pulsarbot run-failure-checks


-- 
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 merged pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
codelipenghui merged PR #17035:
URL: https://github.com/apache/pulsar/pull/17035


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228095548

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1234073834

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1236163800

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235385559

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on a diff in pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on code in PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#discussion_r963330549


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -2113,33 +2114,44 @@ private void handlePoliciesUpdates(NamespaceName namespace) {
     }
 
     private void handleDynamicConfigurationUpdates() {
-        pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync()
-                .thenAccept(optMap -> {
-                    if (!optMap.isPresent()) {
-                        return;
-                    }
-                    Map<String, String> data = optMap.get();
-                    data.forEach((configKey, value) -> {
-                        Field configField = dynamicConfigurationMap.get(configKey).field;
-                        Object newValue = FieldParser.value(data.get(configKey), configField);
-                        if (configField != null) {
-                            Consumer listener = configRegisteredListeners.get(configKey);
-                            try {
-                                Object existingValue = configField.get(pulsar.getConfiguration());
-                                configField.set(pulsar.getConfiguration(), newValue);
-                                log.info("Successfully updated configuration {}/{}", configKey,
-                                        data.get(configKey));
-                                if (listener != null && !existingValue.equals(newValue)) {
-                                    listener.accept(newValue);
+        DynamicConfigurationResources dynamicConfigResources = null;
+        try {
+            dynamicConfigResources = pulsar()
+                    .getPulsarResources()
+                    .getDynamicConfigResources();

Review Comment:
   handleDynamicConfigurationUpdates is executed via zk callbacks:
   <img width="1202" alt="image" src="https://user-images.githubusercontent.com/19296967/188569203-83ab4144-c50d-41cd-8eb2-4c6c6d4c9b6c.png">
   



-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228103024

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1229129149

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1237707587

   > Could you please help add tests? We are able to restart the broker during the test.
   
   @codelipenghui PTAL,thanks!


-- 
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] lordcheng10 commented on a diff in pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on code in PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#discussion_r963329437


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -2113,33 +2114,44 @@ private void handlePoliciesUpdates(NamespaceName namespace) {
     }
 
     private void handleDynamicConfigurationUpdates() {
-        pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync()
-                .thenAccept(optMap -> {
-                    if (!optMap.isPresent()) {
-                        return;
-                    }
-                    Map<String, String> data = optMap.get();
-                    data.forEach((configKey, value) -> {
-                        Field configField = dynamicConfigurationMap.get(configKey).field;
-                        Object newValue = FieldParser.value(data.get(configKey), configField);
-                        if (configField != null) {
-                            Consumer listener = configRegisteredListeners.get(configKey);
-                            try {
-                                Object existingValue = configField.get(pulsar.getConfiguration());
-                                configField.set(pulsar.getConfiguration(), newValue);
-                                log.info("Successfully updated configuration {}/{}", configKey,
-                                        data.get(configKey));
-                                if (listener != null && !existingValue.equals(newValue)) {
-                                    listener.accept(newValue);
+        DynamicConfigurationResources dynamicConfigResources = null;
+        try {
+            dynamicConfigResources = pulsar()
+                    .getPulsarResources()
+                    .getDynamicConfigResources();

Review Comment:
   1. This is just to get the object, it will not block the thread.
   2. The try catch here is just to catch the situation where getPulsarResources or getDynamicConfigResources returns null, because some unit tests do not initialize pulsarResources or dynamicConfigResources.
   3. Try cach is also performed in the original call of updateDynamicServiceConfiguration:
   <img width="1010" alt="image" src="https://user-images.githubusercontent.com/19296967/188568999-c3e6aa55-c94d-4e1e-8514-4955b0651d44.png">
   



-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1236809803

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235330421

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235339750

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1229158162

   > @lordcheng10 Please solve CI failure.
   
   OK


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235064605

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235436063

   /pulsarbot run-failure-checks


-- 
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] Jason918 commented on a diff in pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#discussion_r963315461


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -2113,33 +2114,44 @@ private void handlePoliciesUpdates(NamespaceName namespace) {
     }
 
     private void handleDynamicConfigurationUpdates() {
-        pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync()
-                .thenAccept(optMap -> {
-                    if (!optMap.isPresent()) {
-                        return;
-                    }
-                    Map<String, String> data = optMap.get();
-                    data.forEach((configKey, value) -> {
-                        Field configField = dynamicConfigurationMap.get(configKey).field;
-                        Object newValue = FieldParser.value(data.get(configKey), configField);
-                        if (configField != null) {
-                            Consumer listener = configRegisteredListeners.get(configKey);
-                            try {
-                                Object existingValue = configField.get(pulsar.getConfiguration());
-                                configField.set(pulsar.getConfiguration(), newValue);
-                                log.info("Successfully updated configuration {}/{}", configKey,
-                                        data.get(configKey));
-                                if (listener != null && !existingValue.equals(newValue)) {
-                                    listener.accept(newValue);
+        DynamicConfigurationResources dynamicConfigResources = null;
+        try {
+            dynamicConfigResources = pulsar()
+                    .getPulsarResources()
+                    .getDynamicConfigResources();

Review Comment:
   We are changing the async call to sync. Which thread is this method called? Is it possible to cause some dead lock to blocking request response thread?



-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1210509161

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1228017387

   > Could you please help add tests? We are able to restart the broker during the test.
   
   @codelipenghui @HQebupt @Technoboy-  Fixed, PTAL,thanks!


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1210716828

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1229158076

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235057646

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1235806780

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1229151384

   /pulsarbot run-failure-checks


-- 
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] lordcheng10 commented on pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
lordcheng10 commented on PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#issuecomment-1237152050

   /pulsarbot run-failure-checks


-- 
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] Jason918 commented on a diff in pull request #17035: [fix][broker]After the broker is restarted, the cache dynamic configuration is invalid

Posted by GitBox <gi...@apache.org>.
Jason918 commented on code in PR #17035:
URL: https://github.com/apache/pulsar/pull/17035#discussion_r963378607


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java:
##########
@@ -2113,33 +2114,44 @@ private void handlePoliciesUpdates(NamespaceName namespace) {
     }
 
     private void handleDynamicConfigurationUpdates() {
-        pulsar().getPulsarResources().getDynamicConfigResources().getDynamicConfigurationAsync()
-                .thenAccept(optMap -> {
-                    if (!optMap.isPresent()) {
-                        return;
-                    }
-                    Map<String, String> data = optMap.get();
-                    data.forEach((configKey, value) -> {
-                        Field configField = dynamicConfigurationMap.get(configKey).field;
-                        Object newValue = FieldParser.value(data.get(configKey), configField);
-                        if (configField != null) {
-                            Consumer listener = configRegisteredListeners.get(configKey);
-                            try {
-                                Object existingValue = configField.get(pulsar.getConfiguration());
-                                configField.set(pulsar.getConfiguration(), newValue);
-                                log.info("Successfully updated configuration {}/{}", configKey,
-                                        data.get(configKey));
-                                if (listener != null && !existingValue.equals(newValue)) {
-                                    listener.accept(newValue);
+        DynamicConfigurationResources dynamicConfigResources = null;
+        try {
+            dynamicConfigResources = pulsar()
+                    .getPulsarResources()
+                    .getDynamicConfigResources();

Review Comment:
   I see.



-- 
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