You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/05/27 08:11:03 UTC
[GitHub] [helix] narendly opened a new pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
narendly opened a new pull request #1031:
URL: https://github.com/apache/helix/pull/1031
### Issues
- [x] My PR addresses the following Helix issues and references them in the PR description:
Resolves #1030
### Description
- [x] Here are some details about my PR, including screenshots of any UI changes:
This commit adds a method, getIdealAssignmentForWagedFullAuto() in HelixUtil that returns to the user the cluster-wide assignment result obtained from running a rebalance using WAGED. The user will be able to use this method to predict how Helix will be rebalancing resources using the WAGED rebalancer.
### Tests
- [ ] The following tests are written for this issue:
(List the names of added unit/integration tests)
- [ ] The following is the result of the "mvn test" command on the appropriate module:
(Copy & paste the result of "mvn test")
### Commits
- [x] My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Code Quality
- [x] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r432165438
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,82 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
Review comment:
Yes. Let me rename them. I'll be removing "new".
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r432165302
##########
File path: helix-core/src/test/java/org/apache/helix/integration/rebalancer/WagedRebalancer/TestWagedRebalance.java
##########
@@ -164,7 +214,7 @@ public void testWithInstanceTag() throws Exception {
validate(_replica);
}
- @Test(dependsOnMethods = "test")
+ @Test(dependsOnMethods = "testRebalanceTool")
Review comment:
Doesn't have to depend on testRebalanceTool, but I like to enforce strict ordering for the tests I write..
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r432166903
##########
File path: helix-core/src/test/java/org/apache/helix/integration/rebalancer/WagedRebalancer/TestWagedRebalance.java
##########
@@ -164,7 +214,7 @@ public void testWithInstanceTag() throws Exception {
validate(_replica);
}
- @Test(dependsOnMethods = "test")
+ @Test(dependsOnMethods = "testRebalanceTool")
Review comment:
I would prefer to configure the tests in a way of real logic dependencies. Easier for us to maintain.
Moreover, I think we won't remember why it was initially done like this. Let do it in a meaningful way the first time.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r432593955
##########
File path: helix-core/src/test/java/org/apache/helix/integration/rebalancer/WagedRebalancer/TestWagedRebalance.java
##########
@@ -164,7 +214,7 @@ public void testWithInstanceTag() throws Exception {
validate(_replica);
}
- @Test(dependsOnMethods = "test")
+ @Test(dependsOnMethods = "testRebalanceTool")
Review comment:
Okay. I reverted this change.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r432161839
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,82 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
Review comment:
The "new" here might be confusing. So the requirement here is, actually, the caller should include all the resource IS/Configs into the input, right?
##########
File path: helix-core/src/test/java/org/apache/helix/integration/rebalancer/WagedRebalancer/TestWagedRebalance.java
##########
@@ -164,7 +214,7 @@ public void testWithInstanceTag() throws Exception {
validate(_replica);
}
- @Test(dependsOnMethods = "test")
+ @Test(dependsOnMethods = "testRebalanceTool")
Review comment:
nit, but why the other tests need to depend on testRebalanceTool?
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] dasahcc commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r431412258
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
Review comment:
If it will be exposed to customers, let's put it HelixUtil. It is better to have some centralized place for user using that.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly merged pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly merged pull request #1031:
URL: https://github.com/apache/helix/pull/1031
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r431456337
##########
File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/ReadOnlyWagedRebalancer.java
##########
@@ -0,0 +1,88 @@
+package org.apache.helix.controller.rebalancer.waged;
Review comment:
I'm not sure if that's a good idea. I've considered that option and that might even be more confusing.
Note that users do not use rebalancers directly anyway. So I think this would be the appropriate package to put it in along with the original WagedRebalancer.
I'm also making it clear that this class is to be used for testing or tooling purposes in the JavaDoc.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r431456337
##########
File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/ReadOnlyWagedRebalancer.java
##########
@@ -0,0 +1,88 @@
+package org.apache.helix.controller.rebalancer.waged;
Review comment:
I'm not sure if that's a good idea. I've considered that option and that might even be more confusing.
Note that users do not use rebalancers directly anyway. So I think this would be the appropriate package to put it in along with the original WagedRebalancer.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
+ String metadataStoreAddress, ClusterConfig clusterConfig,
+ List<InstanceConfig> instanceConfigs, List<String> liveInstances,
+ List<IdealState> newIdealStates, List<ResourceConfig> newResourceConfigs) {
+ // Prepare a data accessor for a dataProvider (cache) refresh
+ BaseDataAccessor<ZNRecord> baseDataAccessor = new ZkBaseDataAccessor<>(metadataStoreAddress);
+ HelixDataAccessor helixDataAccessor =
+ new ZKHelixDataAccessor(clusterConfig.getClusterName(), baseDataAccessor);
+
+ // Obtain a refreshed dataProvider (cache) and overwrite cluster parameters with the given parameters
+ ResourceControllerDataProvider dataProvider =
+ new ResourceControllerDataProvider(clusterConfig.getClusterName());
+ dataProvider.requireFullRefresh();
+ dataProvider.refresh(helixDataAccessor);
+ dataProvider.setClusterConfig(clusterConfig);
+ dataProvider.setInstanceConfigMap(instanceConfigs.stream()
+ .collect(Collectors.toMap(InstanceConfig::getInstanceName, Function.identity())));
+ dataProvider.setLiveInstances(
+ liveInstances.stream().map(LiveInstance::new).collect(Collectors.toList()));
+ dataProvider.setIdealStates(newIdealStates);
+ dataProvider.setResourceConfigMap(newResourceConfigs.stream()
+ .collect(Collectors.toMap(ResourceConfig::getResourceName, Function.identity())));
+
+ // Create an instance of read-only WAGED rebalancer
+ ReadOnlyWagedRebalancer readOnlyWagedRebalancer =
+ new ReadOnlyWagedRebalancer(metadataStoreAddress, clusterConfig.getClusterName(),
+ clusterConfig.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);
+ event.addAttribute(AttributeName.ControllerDataProvider.name(), dataProvider);
+ event.addAttribute(AttributeName.STATEFUL_REBALANCER.name(), readOnlyWagedRebalancer);
+
+ try {
+ // Run the required stages to obtain the BestPossibleOutput
+ RebalanceUtil.runStage(event, new ResourceComputationStage());
+ RebalanceUtil.runStage(event, new CurrentStateComputationStage());
+ RebalanceUtil.runStage(event, new BestPossibleStateCalcStage());
+ } catch (Exception e) {
+ LOG.error("getIdealAssignmentForWagedFullAuto(): Failed to compute ResourceAssignments!", e);
+ } finally {
+ // Close all ZK connections
+ baseDataAccessor.close();
Review comment:
It shouldn't fail, but I can wrap the entire logic in a try-catch as well.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
+ String metadataStoreAddress, ClusterConfig clusterConfig,
+ List<InstanceConfig> instanceConfigs, List<String> liveInstances,
+ List<IdealState> newIdealStates, List<ResourceConfig> newResourceConfigs) {
+ // Prepare a data accessor for a dataProvider (cache) refresh
+ BaseDataAccessor<ZNRecord> baseDataAccessor = new ZkBaseDataAccessor<>(metadataStoreAddress);
+ HelixDataAccessor helixDataAccessor =
+ new ZKHelixDataAccessor(clusterConfig.getClusterName(), baseDataAccessor);
+
+ // Obtain a refreshed dataProvider (cache) and overwrite cluster parameters with the given parameters
+ ResourceControllerDataProvider dataProvider =
+ new ResourceControllerDataProvider(clusterConfig.getClusterName());
+ dataProvider.requireFullRefresh();
+ dataProvider.refresh(helixDataAccessor);
+ dataProvider.setClusterConfig(clusterConfig);
+ dataProvider.setInstanceConfigMap(instanceConfigs.stream()
Review comment:
@jiajunwang
I've considered the case you mentioned, and what we have here is I believe the right thing to do. We should overwrite completely (replace), that way, we could allow users to keep, modify, or remove certain items in the Collection. Users still have the ability to retrieve the existing ResourceConfigs in the cluster, so that's not a big concern.
Moreover, this fits nicely with the API methods already provided in the DataProvider interface. If we want to support merge or remove, that would require further change in the DataProvider interface, which is undesirable because we'd be making API changes for the sake of having a util method or for testing.
##########
File path: helix-core/src/main/java/org/apache/helix/util/RebalanceUtil.java
##########
@@ -164,4 +167,25 @@ public static void scheduleOnDemandPipeline(String clusterName, long delay,
clusterName);
}
}
+
+ /**
+ * runStage allows the run of individual stages. It can be used to mock a part of the Controller
+ * pipeline run.
+ *
+ * An example usage is as follows:
+ * runStage(event, new ResourceComputationStage());
+ * runStage(event, new CurrentStateComputationStage());
+ * runStage(event, new BestPossibleStateCalcStage());
+ * By running these stages, we are able to obtain BestPossibleStateOutput in the event object.
+ * @param event
+ * @param stage
+ * @throws Exception
+ */
+ public static void runStage(ClusterEvent event, Stage stage) throws Exception {
Review comment:
I have considered this option, and I decided against it because we lose generality. The nature of runStage sometimes requires the modification of which stages to run and depending on the test, it requires the tests to add/modify/remove Attributes in ClusterEvent. So this is the right granularity.
If we provide some composite method like runStages that runs multiple stages, the use case for it would be limited and not as configurable.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
Review comment:
Already reviewed that option, but if we were to place this in RebalanceUtil, we should move the existing function there too, but that would cause a backward-incompatible change. Let's keep it in here for consistency.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
narendly commented on pull request #1031:
URL: https://github.com/apache/helix/pull/1031#issuecomment-636061467
This PR is ready to be merged, approved by @dasahcc
Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
This commit adds a method, getIdealAssignmentForWagedFullAuto() in HelixUtil that returns to the user the cluster-wide assignment result obtained from running a rebalance using WAGED. The user will be able to use this method to predict how Helix will be rebalancing resources using the WAGED rebalancer.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1031: Add getIdealAssignmentForWagedFullAuto in HelixUtil for WAGED rebalancer
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1031:
URL: https://github.com/apache/helix/pull/1031#discussion_r431322627
##########
File path: helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/ReadOnlyWagedRebalancer.java
##########
@@ -0,0 +1,88 @@
+package org.apache.helix.controller.rebalancer.waged;
Review comment:
Let's put this in the util or tool package? It should not be used as a rebalancer.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
Review comment:
I feel RebalanceUtil is a better placement, although the existing method is already in the HelixUtil. What do you think?
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
+ String metadataStoreAddress, ClusterConfig clusterConfig,
+ List<InstanceConfig> instanceConfigs, List<String> liveInstances,
+ List<IdealState> newIdealStates, List<ResourceConfig> newResourceConfigs) {
+ // Prepare a data accessor for a dataProvider (cache) refresh
+ BaseDataAccessor<ZNRecord> baseDataAccessor = new ZkBaseDataAccessor<>(metadataStoreAddress);
+ HelixDataAccessor helixDataAccessor =
+ new ZKHelixDataAccessor(clusterConfig.getClusterName(), baseDataAccessor);
+
+ // Obtain a refreshed dataProvider (cache) and overwrite cluster parameters with the given parameters
+ ResourceControllerDataProvider dataProvider =
+ new ResourceControllerDataProvider(clusterConfig.getClusterName());
+ dataProvider.requireFullRefresh();
+ dataProvider.refresh(helixDataAccessor);
+ dataProvider.setClusterConfig(clusterConfig);
+ dataProvider.setInstanceConfigMap(instanceConfigs.stream()
Review comment:
The WAGED rebalancer considers all resources, so if the "newResourceConfigs" only contains user-specified items and we overwrite the cached map with this input map, it may return a different result. The other list/map fields have a similar concern.
1. Can we do merge instead of overwriting? This serves for the users who want to add or modify some items.
2. How to handle requests to remove some items? I didn't have the answer yet. Will update if I have a good idea.
##########
File path: helix-core/src/main/java/org/apache/helix/util/HelixUtil.java
##########
@@ -140,6 +160,81 @@ public static String serializeByComma(List<String> objects) {
}
}
+ /**
+ * Returns the expected ideal ResourceAssignments for the given resources in the cluster
+ * calculated using the read-only WAGED rebalancer.
+ * @param metadataStoreAddress
+ * @param clusterConfig
+ * @param instanceConfigs
+ * @param liveInstances
+ * @param newIdealStates
+ * @param newResourceConfigs
+ * @return
+ */
+ public static Map<String, ResourceAssignment> getIdealAssignmentForWagedFullAuto(
+ String metadataStoreAddress, ClusterConfig clusterConfig,
+ List<InstanceConfig> instanceConfigs, List<String> liveInstances,
+ List<IdealState> newIdealStates, List<ResourceConfig> newResourceConfigs) {
+ // Prepare a data accessor for a dataProvider (cache) refresh
+ BaseDataAccessor<ZNRecord> baseDataAccessor = new ZkBaseDataAccessor<>(metadataStoreAddress);
+ HelixDataAccessor helixDataAccessor =
+ new ZKHelixDataAccessor(clusterConfig.getClusterName(), baseDataAccessor);
+
+ // Obtain a refreshed dataProvider (cache) and overwrite cluster parameters with the given parameters
+ ResourceControllerDataProvider dataProvider =
+ new ResourceControllerDataProvider(clusterConfig.getClusterName());
+ dataProvider.requireFullRefresh();
+ dataProvider.refresh(helixDataAccessor);
+ dataProvider.setClusterConfig(clusterConfig);
+ dataProvider.setInstanceConfigMap(instanceConfigs.stream()
+ .collect(Collectors.toMap(InstanceConfig::getInstanceName, Function.identity())));
+ dataProvider.setLiveInstances(
+ liveInstances.stream().map(LiveInstance::new).collect(Collectors.toList()));
+ dataProvider.setIdealStates(newIdealStates);
+ dataProvider.setResourceConfigMap(newResourceConfigs.stream()
+ .collect(Collectors.toMap(ResourceConfig::getResourceName, Function.identity())));
+
+ // Create an instance of read-only WAGED rebalancer
+ ReadOnlyWagedRebalancer readOnlyWagedRebalancer =
+ new ReadOnlyWagedRebalancer(metadataStoreAddress, clusterConfig.getClusterName(),
+ clusterConfig.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);
+ event.addAttribute(AttributeName.ControllerDataProvider.name(), dataProvider);
+ event.addAttribute(AttributeName.STATEFUL_REBALANCER.name(), readOnlyWagedRebalancer);
+
+ try {
+ // Run the required stages to obtain the BestPossibleOutput
+ RebalanceUtil.runStage(event, new ResourceComputationStage());
+ RebalanceUtil.runStage(event, new CurrentStateComputationStage());
+ RebalanceUtil.runStage(event, new BestPossibleStateCalcStage());
+ } catch (Exception e) {
+ LOG.error("getIdealAssignmentForWagedFullAuto(): Failed to compute ResourceAssignments!", e);
+ } finally {
+ // Close all ZK connections
+ baseDataAccessor.close();
Review comment:
If the process fails before runStage, for example, while refreshing the cache, this accessor connection will be leaked.
##########
File path: helix-core/src/main/java/org/apache/helix/util/RebalanceUtil.java
##########
@@ -164,4 +167,25 @@ public static void scheduleOnDemandPipeline(String clusterName, long delay,
clusterName);
}
}
+
+ /**
+ * runStage allows the run of individual stages. It can be used to mock a part of the Controller
+ * pipeline run.
+ *
+ * An example usage is as follows:
+ * runStage(event, new ResourceComputationStage());
+ * runStage(event, new CurrentStateComputationStage());
+ * runStage(event, new BestPossibleStateCalcStage());
+ * By running these stages, we are able to obtain BestPossibleStateOutput in the event object.
+ * @param event
+ * @param stage
+ * @throws Exception
+ */
+ public static void runStage(ClusterEvent event, Stage stage) throws Exception {
Review comment:
It might be better if we have a runStage**s** method as util. It would be easier to use, and more generic.
Of course, we will have to refactor calcBestPossState() in the verifier to make the change.
----------------------------------------------------------------
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
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org