From 1408f3a744cf2dd0418a51ae443b65e01bdf2dd4 Mon Sep 17 00:00:00 2001
From: Zachary Pinto <zapinto@linkedin.com>
Date: Wed, 20 Sep 2023 13:34:37 -0700
Subject: [PATCH] Add clarifying comments and use CONCATENATE_CONFIG_JOINER
 constant in ConfigStringUtil to check if FAULT_DOMAIN is complete or needs to
 have _instanceName added instead of hardcoding '='.

---
 .../helix/api/cloud/CloudInstanceInformationV2.java    |  6 ++++++
 .../apache/helix/manager/zk/ParticipantManager.java    | 10 ++++++++--
 .../java/org/apache/helix/util/ConfigStringUtil.java   |  2 +-
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/api/cloud/CloudInstanceInformationV2.java b/helix-core/src/main/java/org/apache/helix/api/cloud/CloudInstanceInformationV2.java
index 66dd212794..9d464d1643 100644
--- a/helix-core/src/main/java/org/apache/helix/api/cloud/CloudInstanceInformationV2.java
+++ b/helix-core/src/main/java/org/apache/helix/api/cloud/CloudInstanceInformationV2.java
@@ -23,6 +23,12 @@
 
 import com.google.common.collect.ImmutableMap;
 
+/**
+ * Generic interface for cloud instance information which builds on top of CloudInstanceInformation.
+ * This interface adds a new method, getAll(), which returns all the key value pairs of a specific cloud instance.
+ * We call suffix the name of this interface with V2 to preserve backwards compatibility for all classes
+ * that implement CloudInstanceInformation.
+ */
 public interface CloudInstanceInformationV2 extends CloudInstanceInformation {
   /**
    * Get all the key value pairs of a specific cloud instance
diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java b/helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java
index cee573977f..5099ee6acf 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/ParticipantManager.java
@@ -59,6 +59,7 @@
 import org.apache.helix.participant.statemachine.StateModelFactory;
 import org.apache.helix.task.TaskConstants;
 import org.apache.helix.task.TaskUtil;
+import org.apache.helix.util.ConfigStringUtil;
 import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.datamodel.ZNRecordBucketizer;
@@ -231,8 +232,13 @@ private void joinCluster() {
         String cloudInstanceInformationFaultDomain = cloudInstanceInformation.get(
             CloudInstanceInformation.CloudInstanceField.FAULT_DOMAIN.name());
         instanceConfig = instanceConfigBuilder.setDomain(
-            cloudInstanceInformationFaultDomain.endsWith("=") ? cloudInstanceInformationFaultDomain
-                + _instanceName : cloudInstanceInformationFaultDomain).build(_instanceName);
+            // Previously, the FAULT_DOMAIN was expected to end with the final DOMAIN field key without a value,
+            // like "rack=25, host=" or "cabinet=A, rack=25, host=". This is because ParticipantManager would append
+            // the _instanceName to populate the value. This check has been added to preserve backwards compatibility
+            // while also allowing the auto-registration to construct the full DOMAIN which includes the last value.
+            cloudInstanceInformationFaultDomain.endsWith(ConfigStringUtil.CONCATENATE_CONFIG_JOINER)
+                ? cloudInstanceInformationFaultDomain + _instanceName
+                : cloudInstanceInformationFaultDomain).build(_instanceName);
       }
       instanceConfig.validateTopologySettingInInstanceConfig(
           _configAccessor.getClusterConfig(_clusterName), _instanceName);
diff --git a/helix-core/src/main/java/org/apache/helix/util/ConfigStringUtil.java b/helix-core/src/main/java/org/apache/helix/util/ConfigStringUtil.java
index 68f42026c5..e9d0aa4510 100644
--- a/helix-core/src/main/java/org/apache/helix/util/ConfigStringUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/util/ConfigStringUtil.java
@@ -25,7 +25,7 @@
 
 public final class ConfigStringUtil {
   private static final String CONCATENATE_CONFIG_SPLITTER = ",";
-  private static final String CONCATENATE_CONFIG_JOINER = "=";
+  public static final String CONCATENATE_CONFIG_JOINER = "=";
 
   private ConfigStringUtil() {
     throw new java.lang.UnsupportedOperationException(