[GitHub] [pulsar] Jason918 commented on a change in pull request #12753: [Issue 12726][broker] Fix deadlock in metadata-store callback thread

[GitBox <gi...@apache.org>]

Jason918 commented on a change in pull request #12753:
URL: https://github.com/apache/pulsar/pull/12753#discussion_r747933348



##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/HierarchyTopicPolicies.java
##########
@@ -28,9 +30,11 @@
 public class HierarchyTopicPolicies {
     final PolicyHierarchyValue<InactiveTopicPolicies> inactiveTopicPolicies;
     final PolicyHierarchyValue<Integer> maxSubscriptionsPerTopic;
+    final Map<BacklogQuota.BacklogQuotaType, PolicyHierarchyValue<BacklogQuota>> backLogQuotaMap;
 
     public HierarchyTopicPolicies() {
         inactiveTopicPolicies = new PolicyHierarchyValue<>();
         maxSubscriptionsPerTopic = new PolicyHierarchyValue<>();
+        backLogQuotaMap = new HashMap<>();

Review comment:
       > Can the map be accessed by multiple threads?
   
   Yes, thanks for the reminding. ImmutableMap is a better choice here.
   
   > And I think it should be PolicyHierarchyValue<Map<BacklogQuota.BacklogQuotaType, BacklogQuota>>?
   
   With this, there will be a problem under this case:
   ```
   TopicLevelSettings : destination_storage --> BacklogQuota1, message_age --> null
   BrokerSettings : destination_storage --> null, message_age --> BacklogQuota2
   ```
   
   With PolicyHierarchyValue<Map<...>>,  the value of type `message_age` would be null.
   With Map<PolicyHierarchyValue>, the value will be "BacklogQuota2".
   
   With previous logic, the result seems to be "BacklogQuota2". 
   




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