heesung-sn commented on code in PR #20512:
URL: https://github.com/apache/pulsar/pull/20512#discussion_r1223354649


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ModularLoadManagerImpl.java:
##########
@@ -603,6 +602,26 @@ private void updateBundleData() {
                 
LoadManagerShared.fillNamespaceToBundlesMap(preallocatedBundleData.keySet(), 
namespaceToBundleRange);
             }
         }
+
+        // Remove not active bundle from loadData
+        for (String bundle : bundleData.keySet()) {
+            if (!activeBundles.contains(bundle)) {
+                long notActiveTimes = notActiveBundleCounter
+                        .computeIfAbsent(bundle, k -> new 
AtomicLong()).incrementAndGet();
+
+                if (notActiveTimes > nonActiveBundleDeleteThreshold) {

Review Comment:
   Do we know what causes inconsistency between `activeBundles` and 
`bundleData.keySet()`?



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ModularLoadManagerImpl.java:
##########
@@ -198,6 +199,10 @@ public class ModularLoadManagerImpl implements 
ModularLoadManager {
     private final Set<String> knownBrokers = new HashSet<>();
     private Map<String, String> bundleBrokerAffinityMap;
 
+    public static long nonActiveBundleDeleteThreshold = 15;

Review Comment:
   Does that mean in the worst case, bundle deletion in the metadata store 
would take 15 mins in the worst case if the update cycle is every min?
   Why have we picked 15?



##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ModularLoadManagerImpl.java:
##########
@@ -603,6 +602,26 @@ private void updateBundleData() {
                 
LoadManagerShared.fillNamespaceToBundlesMap(preallocatedBundleData.keySet(), 
namespaceToBundleRange);
             }
         }
+
+        // Remove not active bundle from loadData
+        for (String bundle : bundleData.keySet()) {
+            if (!activeBundles.contains(bundle)) {
+                long notActiveTimes = notActiveBundleCounter
+                        .computeIfAbsent(bundle, k -> new 
AtomicLong()).incrementAndGet();
+
+                if (notActiveTimes > nonActiveBundleDeleteThreshold) {

Review Comment:
   Do we need to worry about other data inconsistencies, for example, between 
this bundleData(and deleteBundleDataFromMetadataStore) vs other data structures?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to