[GitHub] [helix] narendly commented on a change in pull request #1251: Add totalMessageReceivedCounter and decrepcate old totalMessageReceived

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

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:
users@infra.apache.org



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