jiajunwang commented on a change in pull request #1058:
URL: https://github.com/apache/helix/pull/1058#discussion_r436902771
##########
File path:
helix-core/src/main/java/org/apache/helix/tools/ClusterVerifiers/BestPossibleExternalViewVerifier.java
##########
@@ -428,4 +432,20 @@ public String toString() {
return verifierName + "(" + _clusterName + "@" + _zkClient + "@resources["
+ (_resources != null ? Arrays.toString(_resources.toArray()) : "") +
"])";
}
+
+ private class ReadOnlyWagedRebalancer extends
org.apache.helix.controller.rebalancer.waged.ReadOnlyWagedRebalancer {
Review comment:
Let's have a different name for this private class. I would still prefer
the old name DryrunWagedRebalancer.
But feel free to have a better name for this method.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -175,27 +175,34 @@ public static String serializeByComma(List<String>
objects) {
String metadataStoreAddress, ClusterConfig clusterConfig,
List<InstanceConfig> instanceConfigs, List<String> liveInstances,
List<IdealState> idealStates, List<ResourceConfig> resourceConfigs) {
+ // Copy the cluster config and make globalRebalance happen synchronously
+ // Otherwise, globalRebalance may not complete and this util might end up
returning
+ // an empty assignment.
+ ClusterConfig globalSyncClusterConfig = new
ClusterConfig(clusterConfig.getRecord());
+ globalSyncClusterConfig.setGlobalRebalanceAsyncMode(false);
+
// Prepare a data accessor for a dataProvider (cache) refresh
BaseDataAccessor<ZNRecord> baseDataAccessor = new
ZkBaseDataAccessor<>(metadataStoreAddress);
HelixDataAccessor helixDataAccessor =
- new ZKHelixDataAccessor(clusterConfig.getClusterName(),
baseDataAccessor);
+ new ZKHelixDataAccessor(globalSyncClusterConfig.getClusterName(),
baseDataAccessor);
// Create an instance of read-only WAGED rebalancer
ReadOnlyWagedRebalancer readOnlyWagedRebalancer =
- new ReadOnlyWagedRebalancer(metadataStoreAddress,
clusterConfig.getClusterName(),
- clusterConfig.getGlobalRebalancePreference());
+ new ReadOnlyWagedRebalancer(metadataStoreAddress,
globalSyncClusterConfig.getClusterName(),
+ globalSyncClusterConfig.getGlobalRebalancePreference());
// Use a dummy event to run the required stages for BestPossibleState
calculation
// Attributes RESOURCES and RESOURCES_TO_REBALANCE are populated in
ResourceComputationStage
- ClusterEvent event = new ClusterEvent(clusterConfig.getClusterName(),
ClusterEventType.Unknown);
+ ClusterEvent event =
+ new ClusterEvent(globalSyncClusterConfig.getClusterName(),
ClusterEventType.Unknown);
try {
// Obtain a refreshed dataProvider (cache) and overwrite cluster
parameters with the given parameters
ResourceControllerDataProvider dataProvider =
- new ResourceControllerDataProvider(clusterConfig.getClusterName());
+ new
ResourceControllerDataProvider(globalSyncClusterConfig.getClusterName());
dataProvider.requireFullRefresh();
dataProvider.refresh(helixDataAccessor);
- dataProvider.setClusterConfig(clusterConfig);
+ dataProvider.setClusterConfig(globalSyncClusterConfig);
dataProvider.setInstanceConfigMap(instanceConfigs.stream()
.collect(Collectors.toMap(InstanceConfig::getInstanceName,
Function.identity())));
dataProvider.setLiveInstances(
Review comment:
As we see in the test, the current way overriding the live instance info
won't work. We need to retain the session Id somehow. Otherwise, the current
state won't be read correctly.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
