pkuwm commented on a change in pull request #1056:
URL: https://github.com/apache/helix/pull/1056#discussion_r435448027



##########
File path: 
helix-core/src/main/java/org/apache/helix/monitoring/mbeans/JobMonitor.java
##########
@@ -41,6 +45,12 @@
   private static final Logger LOG = LoggerFactory.getLogger(JobMonitor.class);
   private static final long DEFAULT_RESET_INTERVAL_MS = 60 * 60 * 1000; // 1 
hour
 
+  // The root path of ZNodes that are used to sync certain metrics to 
ZooKeeper. This path is the
+  // first level, and its children nodes will represent each job type. Since 
the metrics are
+  // aggregated by job types, the metric values will be recorded in each ZNode 
belonging to a job
+  // type as simple fields.
+  private static final String SYNC_ZK_ROOT_PATH = "/JOB_MONITOR_METRICS";

Review comment:
       I would not think it is a good idea to use ZK as syncing store for 
metrics.. ZK is not designed for this purpose. ZK is such a core part of infra. 
Syncing metrics to ZK definitely increase read/write traffic to ZK, which may 
impact the other critical services. If we have a better design not to use ZK as 
a syncing store, exposing metrics to client is fine.
   
   Could you explain any case "users that cannot rely directly on the 
controller emitted metrics to see the metric values."?  I would like to see if 
there is an alternative solution that doesn't use ZK.




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