[GitHub] [helix] jiajunwang commented on a change in pull request #542: Use a copy of the new best possible assignment for measuring baseline divergence

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

jiajunwang commented on a change in pull request #542: Use a copy of the new best possible assignment for measuring baseline divergence
URL: https://github.com/apache/helix/pull/542#discussion_r339788151
 
 

 ##########
 File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/WagedRebalancer.java
 ##########
 @@ -403,11 +404,13 @@ private void refreshBaseline(ResourceControllerDataProvider clusterData,
 
     // Asynchronously report baseline divergence metric before persisting to metadata store,
     // just in case if persisting fails, we still have the metric.
+    // To avoid changes of the new assignment and make it safe when being used to measure baseline
+    // divergence, use a copy of the new assignment.
     BaselineDivergenceGauge baselineDivergenceGauge = _metricCollector.getMetric(
         WagedRebalancerMetricCollector.WagedRebalancerMetricNames.BaselineDivergenceGauge.name(),
         BaselineDivergenceGauge.class);
     baselineDivergenceGauge.asyncMeasureAndUpdateValue(clusterData.getAsyncTasksThreadPool(),
-        currentBaseline, newAssignment);
+        currentBaseline, ImmutableMap.copyOf(newAssignment));
 
 Review comment:
   I think this will only copy the ref, not the deep copy we want.
   Please create new objects for each element.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org