narendly commented on a change in pull request #1251:
URL: https://github.com/apache/helix/pull/1251#discussion_r469049584



##########
File path: 
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/ResourceMonitor.java
##########
@@ -71,8 +71,9 @@
   private SimpleDynamicMetric<Long> _successTopStateHandoffCounter;
   private SimpleDynamicMetric<Long> _failedTopStateHandoffCounter;
   private SimpleDynamicMetric<Long> _maxSinglePartitionTopStateHandoffDuration;
-  private SimpleDynamicMetric<Long> _totalMessageReceived;
-
+  @Deprecated
+  private SimpleDynamicMetric<Long> _totalMessageReceived; // This should be 
counter but it is not now

Review comment:
       Nit: I think it would be good to add here why this needs to be a 
counter. We could add something like:
   
   Because this is a metric indicating a "total" count (a.k.a. ever-increasing) 
value, we want to make sure this metric is a counter. If a gauge is used, the 
metric might get reset upon a controller switch, which won't be ever-increasing.
   
   What do you think?




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to