[GitHub] [pulsar] codelipenghui commented on a diff in pull request #19749: [improve][broker] PIP-192: Add metrics for unload operation

["codelipenghui (via GitHub)" <gi...@apache.org>]

codelipenghui commented on code in PR #19749:
URL: https://github.com/apache/pulsar/pull/19749#discussion_r1132082205


##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/extensions/models/UnloadCounter.java:
##########
@@ -45,36 +48,57 @@ public class UnloadCounter {
     long unloadBrokerCount = 0;
     long unloadBundleCount = 0;
 
-    final Map<UnloadDecision.Label, Map<UnloadDecision.Reason, MutableLong>> breakdownCounters;
+    @Getter
+    @VisibleForTesting
+    final Map<UnloadDecision.Label, Map<UnloadDecision.Reason, AtomicLong>> breakdownCounters;
 
+    @Getter
+    @VisibleForTesting
     double loadAvg;
+    @Getter
+    @VisibleForTesting
     double loadStd;
 
+    private volatile long updatedAt = 0;
+
     public UnloadCounter() {
         breakdownCounters = Map.of(
                 Success, Map.of(
-                        Overloaded, new MutableLong(),
-                        Underloaded, new MutableLong()),
+                        Overloaded, new AtomicLong(),
+                        Underloaded, new AtomicLong(),
+                        Admin, new AtomicLong()),
                 Skip, Map.of(
-                        Balanced, new MutableLong(),
-                        NoBundles, new MutableLong(),
-                        CoolDown, new MutableLong(),
-                        OutDatedData, new MutableLong(),
-                        NoLoadData, new MutableLong(),
-                        NoBrokers, new MutableLong(),
-                        Unknown, new MutableLong()),
+                        Balanced, new AtomicLong(),
+                        NoBundles, new AtomicLong(),
+                        CoolDown, new AtomicLong(),
+                        OutDatedData, new AtomicLong(),
+                        NoLoadData, new AtomicLong(),
+                        NoBrokers, new AtomicLong(),
+                        Unknown, new AtomicLong()),
                 Failure, Map.of(
-                        Unknown, new MutableLong())
+                        Unknown, new AtomicLong())
         );
     }
 
     public void update(UnloadDecision decision) {
-        var unloads = decision.getUnloads();
-        unloadBrokerCount += unloads.keySet().size();
-        unloadBundleCount += unloads.values().size();
-        breakdownCounters.get(decision.getLabel()).get(decision.getReason()).increment();
-        loadAvg = decision.loadAvg;
-        loadStd = decision.loadStd;
+        unloadBrokerCount++;
+        unloadBundleCount++;
+        breakdownCounters.get(decision.getLabel()).get(decision.getReason()).incrementAndGet();
+        updatedAt = System.currentTimeMillis();
+    }
+
+    public void update(UnloadDecision.Label label, UnloadDecision.Reason reason) {
+        if (label == Success) {
+            unloadBundleCount++;
+            unloadBrokerCount++;

Review Comment:
   @Demogorgon314 @heesung-sn We'd better to not remove it in this PR. If it is worth to be removed, we can start a separate discussion and PR. Removing the metrics might break the Pulsar operators.



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