[GitHub] [helix] qqu0127 commented on a diff in pull request #2011: WIP -- Attempt to fix TestDropResourceMetricsReset

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

qqu0127 commented on code in PR #2011:
URL: https://github.com/apache/helix/pull/2011#discussion_r841985016


##########
helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java:
##########
@@ -96,7 +96,7 @@ public void execute(final ClusterEvent event) throws Exception {
 
     // Keep MBeans for existing resources and unregister MBeans for dropped resources
     if (clusterStatusMonitor != null) {
-      clusterStatusMonitor.retainResourceMonitor(monitoringResources);
+      //clusterStatusMonitor.retainResourceMonitor(monitoringResources);

Review Comment:
   Thanks for the comment!
   
   > where would be the "correct" place for metric-cleanup logic to be?
   
   With limited context and knowledge, I think both `ResourceValidationStage` and `ExternalViewComputeStage` make sense. MetricsMonitors belong to, or at least associated with resources. On the other hand, generally speaking, it's also a "view". 
   it's also applicable to be in a standalone stage instead of mixing up with other logic.
   
   > async stage...
   
   Yeah, this is a good point I wasn't aware of. So essentially, we'd want to run it in async way, and make sure all related cluster events trigger such computation. 
   
   > add a CurrentState change event to artificially trigger the stage
   
   This is definitely an option for the fix. But could it introduce more overhead and side effect? And what the difference from adding `externalViewPipeline` to `IdealStateChange` registry? All we need is refresh the resource monitors, but we will be running a few other pipelines.



-- 
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: reviews-unsubscribe@helix.apache.org

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