[GitHub] [helix] mgao0 commented on a diff in pull request #2340: Add metrics for rebalance throttled because of error partition

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

mgao0 commented on code in PR #2340:
URL: https://github.com/apache/helix/pull/2340#discussion_r1081708151


##########
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/ClusterStatusMonitor.java:
##########
@@ -1121,4 +1120,13 @@ public long getPendingStateTransitionGuage() {
     }
     return total;
   }
+
+  @Override
+  public long getRebalanceThrottledByErrorPartitionGauge() {

Review Comment:
   The naming is confusing. It is called getRebalanceThrottledByErrorPartitionGauge both in ClusterStatusMonitor and in ResourceMonitor. You may consider something like numOfResourcesRebalanceThrottledGauge, or some other name you think is more clear.



##########
helix-core/src/test/java/org/apache/helix/controller/stages/TestIntermediateStateCalcStage.java:
##########
@@ -264,6 +268,136 @@ public void testWithClusterConfigChange() {
 
     IntermediateStateOutput output = event.getAttribute(AttributeName.INTERMEDIATE_STATE.name());
 
+
+    // Validate that there are 2 resourced load balance been throttled

Review Comment:
   Did you mean "0" here?



##########
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/ClusterStatusMonitor.java:
##########
@@ -1121,4 +1120,13 @@ public long getPendingStateTransitionGuage() {
     }
     return total;
   }
+
+  @Override
+  public long getRebalanceThrottledByErrorPartition() {
+    long total = 0;
+    for (Map.Entry<String, ResourceMonitor> entry : _resourceMonitorMap.entrySet()) {

Review Comment:
   This is a simple addition and no exceptions are thrown so lamda will be a good option. But also agree that we can keep the style consistent throughout the codebase and do a global replacement later once we know more about the performance impact. 



-- 
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: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org