xyuanlu commented on a change in pull request #1129:
URL: https://github.com/apache/helix/pull/1129#discussion_r458485548
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/topology/Topology.java
##########
@@ -204,8 +172,9 @@ private Node createClusterTree() {
InstanceConfig insConfig = _instanceConfigMap.get(instanceName);
try {
LinkedHashMap<String, String> instanceTopologyMap =
-
computeInstanceTopologyMap(_clusterConfig.isTopologyAwareEnabled(),
instanceName,
- insConfig, _clusterTopologyKeys);
+
computeInstanceTopologyMapHelper(_clusterConfig.isTopologyAwareEnabled(),
instanceName,
Review comment:
TFTR. I think computeInstanceTopologyMap for users outside this
function. They need to call 'computeInstanceTopologyMap' to construct the
'clusterTopologyConfig' (line 323) first and then build the instance topology
path. Here in Topology it self, the 'clusterTopologyConfig' is computed in the
constructor. So we need another layer of abstraction.
I am open to discussion.
##########
File path:
helix-core/src/main/java/org/apache/helix/controller/rebalancer/topology/Topology.java
##########
@@ -230,20 +199,73 @@ private boolean isInstanceEnabled(ClusterConfig
clusterConfig, String instanceNa
|| !clusterConfig.getDisabledInstances().containsKey(instanceName)));
}
+ /**
+ * Populate faultZone, endNodetype and and a LinkedHashMap containing
pathKeys default values for
+ * clusterConfig.Topology. The LinkedHashMap will be empty if
clusterConfig.Topology is unset.
+ *
+ * @return an Instance of Topology.ClusterTopologyConfig.
+ */
+ private static ClusterTopologyConfig getClusterTopologySetting(ClusterConfig
clusterConfig) {
+
+ ClusterTopologyConfig clusterTopologyConfig = new ClusterTopologyConfig();
+ if (clusterConfig.isTopologyAwareEnabled()) {
+ String topologyDef = clusterConfig.getTopology();
+ if (topologyDef != null) {
+ String[] topologyKeys = topologyDef.trim().split("/");
+ int lastValidTypeIdx = 0;
+ for (int i = 0; i < topologyKeys.length; i++) {
+ if (topologyKeys[i].length() != 0) {
+ clusterTopologyConfig.topologyKeyDefaultValue
+ .put(topologyKeys[i], DEFAULT_DOMAIN_PREFIX + topologyKeys[i]);
+ lastValidTypeIdx = i;
+ }
+ }
+ if (clusterTopologyConfig.topologyKeyDefaultValue.size() == 0) {
+ throw new IllegalArgumentException("Invalid cluster topology
definition " + topologyDef);
+ }
+ clusterTopologyConfig.endNodeType = topologyKeys[lastValidTypeIdx];
+ String faultZoneType = clusterConfig.getFaultZoneType();
+ if (faultZoneType == null) {
+ clusterTopologyConfig.faultZoneType =
clusterTopologyConfig.endNodeType;
+ } else if
(!clusterTopologyConfig.topologyKeyDefaultValue.containsKey(faultZoneType)) {
+ throw new HelixException(String
+ .format("Invalid fault zone type %s, not present in topology
definition %s.",
+ faultZoneType, clusterConfig.getTopology()));
+ } else {
+ clusterTopologyConfig.faultZoneType = faultZoneType;
+ }
+ } else {
+ // Use default cluster topology definition, i,e. /root/zone/instance
+ clusterTopologyConfig.endNodeType = Types.INSTANCE.name();
+ clusterTopologyConfig.faultZoneType = Types.ZONE.name();
+ }
+ } else {
+ clusterTopologyConfig.endNodeType = Types.INSTANCE.name();
+ clusterTopologyConfig.faultZoneType = Types.INSTANCE.name();
+ }
+ return clusterTopologyConfig;
+ }
+
/**
* This function returns a LinkedHashMap<String, String> object representing
* the topology path for an instance.
* LinkedHashMap is used here since the order of the path needs to be
preserved
* when creating the topology tree.
*
+ * @param clusterTopologyKeyDefaultValue a LinkedHashMap where keys are
cluster topology path and
+ * values are their corresponding
default value. The entries
+ * are ordered by
ClusterConfig.topology setting.
+ * @param faultZoneForEarlyQuit this flag is set to true only if caller
wants the path
Review comment:
TFTR. Updated.
----------------------------------------------------------------
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]
