jiajunwang 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_r389975868
##########
File path: helix-core/src/main/java/org/apache/helix/model/ResourceConfig.java
##########
@@ -842,5 +841,89 @@ 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.
+ */
+ public static ResourceConfig mergeIdealStateWithResourceConfig(
+ final ResourceConfig resourceConfig, final IdealState idealState) {
+ // 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.
+
+ // Return a newly constructed resource config to avoid the original value
be modified.
+ ResourceConfig mergedResourceConfig = new
ResourceConfig(resourceConfig.getRecord());
+ ZNRecord mergedZNRecord = mergedResourceConfig.getRecord();
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.INSTANCE_GROUP_TAG.name()))
{
+
mergedZNRecord.setSimpleField(ResourceConfig.ResourceConfigProperty.INSTANCE_GROUP_TAG.name(),
+ idealState.getInstanceGroupTag());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.MAX_PARTITIONS_PER_INSTANCE.name()))
{
+ mergedZNRecord
+
.setIntField(ResourceConfig.ResourceConfigProperty.MAX_PARTITIONS_PER_INSTANCE.name(),
+ idealState.getMaxPartitionsPerInstance());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.NUM_PARTITIONS.name())) {
+ mergedZNRecord
+ .setIntField(ResourceConfigProperty.NUM_PARTITIONS.name(),
idealState.getNumPartitions());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.STATE_MODEL_DEF_REF.name()))
{
+ mergedZNRecord
+
.setSimpleField(ResourceConfig.ResourceConfigProperty.STATE_MODEL_DEF_REF.name(),
+ idealState.getStateModelDefRef());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.STATE_MODEL_FACTORY_NAME.name()))
{
+ mergedZNRecord
+
.setSimpleField(ResourceConfig.ResourceConfigProperty.STATE_MODEL_FACTORY_NAME.name(),
+ idealState.getStateModelFactoryName());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.REPLICAS.name())) {
+
mergedZNRecord.setSimpleField(ResourceConfig.ResourceConfigProperty.REPLICAS.name(),
+ idealState.getReplicas());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.MIN_ACTIVE_REPLICAS.name()))
{
+
mergedZNRecord.setIntField(ResourceConfig.ResourceConfigProperty.MIN_ACTIVE_REPLICAS.name(),
+ idealState.getMinActiveReplicas());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.HELIX_ENABLED.name())) {
+
mergedZNRecord.setBooleanField(ResourceConfig.ResourceConfigProperty.HELIX_ENABLED.name(),
+ idealState.isEnabled());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.RESOURCE_GROUP_NAME.name()))
{
+ mergedZNRecord
+
.setSimpleField(ResourceConfig.ResourceConfigProperty.RESOURCE_GROUP_NAME.name(),
+ idealState.getResourceGroupName());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.RESOURCE_TYPE.name())) {
+
mergedZNRecord.setSimpleField(ResourceConfig.ResourceConfigProperty.RESOURCE_TYPE.name(),
+ idealState.getResourceType());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.EXTERNAL_VIEW_DISABLED.name()))
{
+ mergedZNRecord
+
.setBooleanField(ResourceConfig.ResourceConfigProperty.EXTERNAL_VIEW_DISABLED.name(),
+ idealState.isExternalViewDisabled());
+ }
+ if (null == mergedZNRecord
+
.getSimpleField(ResourceConfig.ResourceConfigProperty.DELAY_REBALANCE_ENABLED.name()))
{
+ mergedZNRecord
+
.setBooleanField(ResourceConfig.ResourceConfigProperty.DELAY_REBALANCE_ENABLED.name(),
+ idealState.isDelayRebalanceEnabled());
+ }
Review comment:
I tried.
1. Although the field keys are the same, the get logic is different. If I
copypaste directly based on the field names, I might change the IS logic. For
example, getReplicas(). It would be safer that I call the explicit IdealState
get methods.
Unfortunately, this will lead to a similar code. But if we take this as a
temporary backward compatible workaround, I think it is acceptable. For the
correctness.
2. The 2 lists are not exactly the same. DELAY_REBALANCE_DISABLED in IS and
DELAY_REBALANCE_ENABLED in the ResourceConfig.
To reduce this complex, I propose to add some setXXXIfAbsent in the ZNRecord
class. Will update soon.
----------------------------------------------------------------
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]
