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_r330687419
##########
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:
As I checked the code about how _metricCollector is leveraged, there is no
null check after this point.
Since we still have a public constructor that accepts the instance, we need
to check for null to prevent NPE here. Or check for null everywhere it is used.
Either way is fine to me.
----------------------------------------------------------------
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
