narendly commented on code in PR #2011:
URL: https://github.com/apache/helix/pull/2011#discussion_r843056826
##########
helix-core/src/test/java/org/apache/helix/monitoring/mbeans/TestDropResourceMetricsReset.java:
##########
@@ -148,6 +148,10 @@ public void testDropWithNoCurrentState() throws Exception {
// Drop the resource
setupTool.dropResourceFromCluster(clusterName, RESOURCE_NAME);
+ // TEMP WORKAROUND, trigger a CurrentStateChange to remove the resource
monitor
+ // https://github.com/apache/helix/issues/1980
+ participant.syncStart();
Review Comment:
could we give a little more context? (e.g. adding a liveinstance has an
effect of creating a CurrentStageChange event, **which in turn triggers
ExternalViewStage that includes the metric cleanup logic**). this way, the
intention is clear and whoever sees this later can immediately understand why
this is being worked around this way. Also consider putting down a `FIXME` tag
here?
also, should we stop this participant when we clean up (line 159 and
beyond)? otherwise, it will leave a dangling thread.
lastly, you might want to consider renaming the PR title - it's no longer an
"attempt" but this PR will actually fix it :)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
