jiajunwang commented on a change in pull request #490: Add latency metric components for WAGED rebalancer URL: https://github.com/apache/helix/pull/490#discussion_r329770758
########## File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java ########## @@ -90,31 +92,39 @@ private static AssignmentMetadataStore constructAssignmentStore(HelixManager hel } public WagedRebalancer(HelixManager helixManager, - Map<ClusterConfig.GlobalRebalancePreferenceKey, Integer> preferences) { + Map<ClusterConfig.GlobalRebalancePreferenceKey, Integer> preferences, + MetricCollector metricCollector) throws JMException { this(constructAssignmentStore(helixManager), ConstraintBasedAlgorithmFactory.getInstance(preferences), // Use DelayedAutoRebalancer as the mapping calculator for the final assignment output. // Mapping calculator will translate the best possible assignment into the applicable state // mapping based on the current states. // TODO abstract and separate the main mapping calculator logic from DelayedAutoRebalancer - new DelayedAutoRebalancer()); + new DelayedAutoRebalancer(), metricCollector); } private WagedRebalancer(AssignmentMetadataStore assignmentMetadataStore, - RebalanceAlgorithm algorithm, MappingCalculator mappingCalculator) { + RebalanceAlgorithm algorithm, MappingCalculator mappingCalculator, + MetricCollector metricCollector) throws JMException { if (assignmentMetadataStore == null) { LOG.warn("Assignment Metadata Store is not configured properly." + " The rebalancer will not access the assignment store during the rebalance."); } _assignmentMetadataStore = assignmentMetadataStore; _rebalanceAlgorithm = algorithm; _mappingCalculator = mappingCalculator; + _metricCollector = metricCollector; Review comment: For aligning with the other metric providers (or collectors), shall we allow null object here? Basically null means we skip the metrics emitting. Due to the metric count limitation, probably we don't want any MBean to be registered during the unrelated tests. ---------------------------------------------------------------- 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 With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org