keith-turner commented on code in PR #4461:
URL: https://github.com/apache/accumulo/pull/4461#discussion_r1585588747


##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -638,6 +660,11 @@ public interface MetricsProducer {
   String METRICS_TSERVER_SCAN_RESULTS_BYTES = METRICS_TSERVER_PREFIX + 
"scan.results.bytes";
   String METRICS_TSERVER_SCANNED_ENTRIES = METRICS_TSERVER_PREFIX + 
"scan.scanned.entries";
 
+  String METRICS_SSERVER_PREFIX = "accumulo.sserver.";
+  String METRICS_SSERVER_REGISTRATION_TIMER = METRICS_SSERVER_PREFIX + 
"registration.timer";
+  String METRICS_SSERVER_BUSY_COUNTER = METRICS_SSERVER_PREFIX + "busy.count";
+  String METRICS_SSERVER_TABLET_METADATA_CACHE = METRICS_SSERVER_PREFIX + 
"tablet.metadata.cache";

Review Comment:
   Wondering if 
[this](https://github.com/apache/accumulo/blob/6c8e175eab871428e0340d65bdcedbef0d332c42/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java#L608)
 should be `accumulo.scans` instead of `accumulo.tserver.scans`.  That may be a 
change for main instead and not 2.1.3 though.  As you mentioned the tag will 
indicate what kind of process it is.



##########
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java:
##########
@@ -638,6 +660,11 @@ public interface MetricsProducer {
   String METRICS_TSERVER_SCAN_RESULTS_BYTES = METRICS_TSERVER_PREFIX + 
"scan.results.bytes";
   String METRICS_TSERVER_SCANNED_ENTRIES = METRICS_TSERVER_PREFIX + 
"scan.scanned.entries";
 
+  String METRICS_SSERVER_PREFIX = "accumulo.sserver.";
+  String METRICS_SSERVER_REGISTRATION_TIMER = METRICS_SSERVER_PREFIX + 
"registration.timer";
+  String METRICS_SSERVER_BUSY_COUNTER = METRICS_SSERVER_PREFIX + "busy.count";

Review Comment:
   This can be dropped in favor of the already existing metric 
[MetricProducer.METRICS_SCAN_BUSY_TIMEOUT](https://github.com/apache/accumulo/blob/6c8e175eab871428e0340d65bdcedbef0d332c42/core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java#L616)
   ```suggestion
   ```
   
   
   We may need to change the prefix on the existing metric,  its currently 
accumulo.tserver.scans.busy.timeout.  Busy timeouts should only happen on scan 
servers, never on tservers.



-- 
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: [email protected]

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

Reply via email to