dasahcc commented on a change in pull request #878: Get the MinActiveReplica
from Resource Config and Idea State
URL: https://github.com/apache/helix/pull/878#discussion_r390630659
##########
File path: helix-core/src/main/java/org/apache/helix/model/ResourceConfig.java
##########
@@ -842,5 +841,72 @@ public ResourceConfig build() {
_mapFields, _p2pMessageEnabled, _partitionCapacityMap);
}
}
+
+ /**
+ * For backward compatibility, propagate the critical simple fields from the
IdealState to
+ * the Resource Config.
+ * Eventually, Resource Config should be the only metadata node that
contains the required information.
+ *
+ * Note that the config fields get updated in this method shall be fully
compatible with ones in the IdealState.
+ * 1. The fields shall have exactly the same meaning.
+ * 2. The value shall be fully compatible, no additional calculation
involved.
+ * 3. Resource Config items have a high priority.
+ */
+ public static ResourceConfig mergeIdealStateWithResourceConfig(
+ final ResourceConfig resourceConfig, final IdealState idealState) {
+ if (idealState == null) {
+ return resourceConfig;
+ }
+ ResourceConfig mergedResourceConfig;
+ if (resourceConfig != null) {
+ if
(!resourceConfig.getResourceName().equals(idealState.getResourceName())) {
+ throw new IllegalArgumentException(String.format(
+ "Cannot merge the IdealState of resource %s with the
ResourceConfig of resource %s",
+ resourceConfig.getResourceName(), idealState.getResourceName()));
+ }
+ // Copy the resource config to avoid the original value being modified
unexpectedly.
+ mergedResourceConfig = new ResourceConfig(resourceConfig.getRecord());
+ } else {
+ // If no resource config specified, construct one based on the
Idealstate.
+ mergedResourceConfig = new ResourceConfig(idealState.getResourceName());
+ }
+ // Fill the compatible Idealstate fields to the ResourceConfig if possible.
+ ZNRecord mergedZNRecord = mergedResourceConfig.getRecord();
Review comment:
Can we make a list of constants for these necessary field and use a loop to
do the merge instead of have these code?
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
