Re: [PR] [fix][broker] Avoid load shedding and metadata writes from a former leader [pulsar]

["void-ptr974 (via GitHub)" <gi...@apache.org>]

void-ptr974 commented on code in PR #26253:
URL: https://github.com/apache/pulsar/pull/26253#discussion_r3981135138


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/impl/ModularLoadManagerImpl.java:
##########
@@ -651,61 +669,73 @@ public synchronized void doLoadShedding() {
         Set<String> sheddingExcludedNamespaces = conf.getLoadBalancerSheddingExcludedNamespaces();
         final Multimap<String, String> bundlesToUnload = loadSheddingStrategy.findBundlesForUnloading(loadData, conf);
 
-        bundlesToUnload.asMap().forEach((broker, bundles) -> {
-            AtomicBoolean unloadBundleForBroker = new AtomicBoolean(false);
-            bundles.forEach(bundle -> {
+        for (Map.Entry<String, Collection<String>> entry : bundlesToUnload.asMap().entrySet()) {
+            String broker = entry.getKey();
+            boolean unloadBundleForBroker = false;
+            for (String bundle : entry.getValue()) {
+                if (!isLeader()) {
+                    return;
+                }

Review Comment:
   Thanks for catching this. Addressed in 98fae47918d. The leadership-loss paths now exit the bundle loop, finalize accounting for the current broker, and then stop further processing, so updateBundleUnloadingMetrics() still publishes unloads completed before leadership was lost. I also added testLoadSheddingPublishesMetricsWhenLeadershipChangesAfterUnload, which loses leadership after the first successful unload and verifies that only one bundle is unloaded while both broker and bundle totals are published as 1. The focused tests and broker checkstyle checks pass locally.



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