mcvsubbu commented on a change in pull request #6037:
URL: https://github.com/apache/incubator-pinot/pull/6037#discussion_r492959921



##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/utils/CommonConstants.java
##########
@@ -279,7 +281,10 @@
     }
 
     public static final String DEFAULT_METRICS_PREFIX = "pinot.server.";
-    public static final boolean DEFAULT_METRICS_GLOBAL_ENABLED = false;
+    public static final String CONFIG_OF_ENABLE_TABLE_LEVEL_METRICS = 
"pinot.server.enableTableLevelMetrics";

Review comment:
       I think you are trying to keep the new configs you introduced in the 
same place. It is unfortunate that we don't have a BrokerConf (as yet), but let 
us keep the server-specific configs in the ServerConf file instead of moving 
that up to common constants. 

##########
File path: 
pinot-common/src/main/java/org/apache/pinot/common/metrics/AbstractMetrics.java
##########
@@ -47,21 +53,36 @@
 
   private final Map<String, AtomicLong> _gaugeValues = new 
ConcurrentHashMap<String, AtomicLong>();
 
-  protected final boolean _global;
+  private final boolean _isTableLevelMetricsEnabled;

Review comment:
       thanks for changing this name 




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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to