You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by zh...@apache.org on 2022/02/08 21:53:50 UTC

[helix] branch helix-virtual-group updated: Implement java API and utils for virtual topology group (#1935)

This is an automated email from the ASF dual-hosted git repository.

zhangmeng pushed a commit to branch helix-virtual-group
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/helix-virtual-group by this push:
     new 95b3113  Implement java API and utils for virtual topology group (#1935)
95b3113 is described below

commit 95b3113be4ca269fff3265bd43c75fb22039cdb2
Author: Qi (Quincy) Qu <qq...@linkedin.com>
AuthorDate: Tue Feb 8 16:53:40 2022 -0500

    Implement java API and utils for virtual topology group (#1935)
    
    Add comment to VirtualTopologyGroupService.
---
 .../constants/VirtualTopologyGroupConstants.java   |   1 +
 .../rebalancer/waged/model/AssignableNode.java     |   1 -
 .../org/apache/helix/model/InstanceConfig.java     |  21 ++-
 .../service/VirtualTopologyGroupService.java       | 197 +++++++++++++++++++++
 .../service/TestVirtualTopologyGroupService.java   | 192 ++++++++++++++++++++
 5 files changed, 407 insertions(+), 5 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/cloud/constants/VirtualTopologyGroupConstants.java b/helix-core/src/main/java/org/apache/helix/cloud/constants/VirtualTopologyGroupConstants.java
index a92e195..d97173a 100644
--- a/helix-core/src/main/java/org/apache/helix/cloud/constants/VirtualTopologyGroupConstants.java
+++ b/helix-core/src/main/java/org/apache/helix/cloud/constants/VirtualTopologyGroupConstants.java
@@ -23,6 +23,7 @@ package org.apache.helix.cloud.constants;
 public class VirtualTopologyGroupConstants {
   public static final String GROUP_NAME = "virtualTopologyGroupName";
   public static final String GROUP_NUMBER = "virtualTopologyGroupNumber";
+  public static final String AUTO_MAINTENANCE_MODE_DISABLED = "autoMaintenanceModeDisabled";
   public static final String GROUP_NAME_SPLITTER = "_";
   public static final String PATH_NAME_SPLITTER = "/";
   public static final String VIRTUAL_FAULT_ZONE_TYPE = "virtualZone";
diff --git a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableNode.java b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableNode.java
index aae2328..e29052a 100644
--- a/helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableNode.java
+++ b/helix-core/src/main/java/org/apache/helix/controller/rebalancer/waged/model/AssignableNode.java
@@ -19,7 +19,6 @@ package org.apache.helix.controller.rebalancer.waged.model;
  * under the License.
  */
 
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
diff --git a/helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java b/helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java
index 143b610..3ab3ea0 100644
--- a/helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java
+++ b/helix-core/src/main/java/org/apache/helix/model/InstanceConfig.java
@@ -63,6 +63,8 @@ public class InstanceConfig extends HelixProperty {
   public static final int WEIGHT_NOT_SET = -1;
   public static final int MAX_CONCURRENT_TASK_NOT_SET = -1;
   private static final int TARGET_TASK_THREAD_POOL_SIZE_NOT_SET = -1;
+  private static final String DOMAIN_FIELD_SPLITTER = ",";
+  private static final String DOMAIN_VALUE_JOINER = "=";
 
   private static final Logger _logger = LoggerFactory.getLogger(InstanceConfig.class.getName());
 
@@ -156,10 +158,9 @@ public class InstanceConfig extends HelixProperty {
     if (domain == null || domain.isEmpty()) {
       return domainAsMap;
     }
-
-    String[] pathPairs = domain.trim().split(",");
+    String[] pathPairs = domain.trim().split(DOMAIN_FIELD_SPLITTER);
     for (String pair : pathPairs) {
-      String[] values = pair.split("=");
+      String[] values = pair.split(DOMAIN_VALUE_JOINER);
       if (values.length != 2 || values[0].isEmpty() || values[1].isEmpty()) {
         throw new IllegalArgumentException(
             String.format("Domain-Value pair %s is not valid.", pair));
@@ -173,12 +174,24 @@ public class InstanceConfig extends HelixProperty {
   /**
    * Domain represents a hierarchy identifier for an instance.
    * Example:  "cluster=myCluster,zone=myZone1,rack=myRack,host=hostname,instance=instance001".
-   * @return
    */
   public void setDomain(String domain) {
     _record.setSimpleField(InstanceConfigProperty.DOMAIN.name(), domain);
   }
 
+  /**
+   * Set domain from its map representation.
+   * @param domainMap domain as a map
+   */
+  public void setDomain(Map<String, String> domainMap) {
+    String domain = domainMap
+        .entrySet()
+        .stream()
+        .map(entry -> entry.getKey() + DOMAIN_VALUE_JOINER + entry.getValue())
+        .collect(Collectors.joining(DOMAIN_FIELD_SPLITTER));
+    setDomain(domain);
+  }
+
   public int getWeight() {
     String w = _record.getSimpleField(InstanceConfigProperty.INSTANCE_WEIGHT.name());
     if (w != null) {
diff --git a/helix-rest/src/main/java/org/apache/helix/rest/server/service/VirtualTopologyGroupService.java b/helix-rest/src/main/java/org/apache/helix/rest/server/service/VirtualTopologyGroupService.java
new file mode 100644
index 0000000..997b880
--- /dev/null
+++ b/helix-rest/src/main/java/org/apache/helix/rest/server/service/VirtualTopologyGroupService.java
@@ -0,0 +1,197 @@
+package org.apache.helix.rest.server.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.helix.AccessOption;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.PropertyPathBuilder;
+import org.apache.helix.cloud.constants.VirtualTopologyGroupConstants;
+import org.apache.helix.cloud.topology.FifoVirtualGroupAssignmentAlgorithm;
+import org.apache.helix.cloud.topology.VirtualGroupAssignmentAlgorithm;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.ClusterConfig;
+import org.apache.helix.model.ClusterTopologyConfig;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.helix.rest.server.json.cluster.ClusterTopology;
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+import org.apache.helix.zookeeper.zkclient.DataUpdater;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+/**
+ * Service for virtual topology group.
+ * It's a virtualization layer on top of physical fault domain and topology in cloud environments.
+ * The service computes the mapping from virtual group to instances based on the current cluster topology and update the
+ * information to cluster and all instances in the cluster.
+ */
+public class VirtualTopologyGroupService {
+  private static final Logger LOG = LoggerFactory.getLogger(VirtualTopologyGroupService.class);
+
+  private final HelixAdmin _helixAdmin;
+  private final ClusterService _clusterService;
+  private final ConfigAccessor _configAccessor;
+  private final HelixDataAccessor _dataAccessor;
+  private final VirtualGroupAssignmentAlgorithm _assignmentAlgorithm;
+
+  public VirtualTopologyGroupService(HelixAdmin helixAdmin, ClusterService clusterService,
+      ConfigAccessor configAccessor, HelixDataAccessor dataAccessor) {
+    _helixAdmin = helixAdmin;
+    _clusterService = clusterService;
+    _configAccessor = configAccessor;
+    _dataAccessor = dataAccessor;
+    _assignmentAlgorithm = FifoVirtualGroupAssignmentAlgorithm.getInstance();
+  }
+
+  /**
+   * Add virtual topology group for a cluster.
+   * This includes calculating the virtual group assignment for all instances in the cluster then update instance config
+   * and cluster config. We override {@link ClusterConfig.ClusterConfigProperty#TOPOLOGY} and
+   * {@link ClusterConfig.ClusterConfigProperty#FAULT_ZONE_TYPE} for cluster config, and add new field to
+   * {@link InstanceConfig.InstanceConfigProperty#DOMAIN} that contains virtual topology group information.
+   * This is only supported for cloud environments. Cluster is expected to be in maintenance mode during config change.
+   * @param clusterName the cluster name.
+   * @param customFields custom fields, {@link VirtualTopologyGroupConstants#GROUP_NAME}
+   *                     and {@link VirtualTopologyGroupConstants#GROUP_NUMBER} are required,
+   *                     {@link VirtualTopologyGroupConstants#AUTO_MAINTENANCE_MODE_DISABLED} is optional.
+   *                     -- if set ture, the cluster will NOT automatically enter/exit maintenance mode during this API call;
+   *                     -- if set false or not set, the cluster will automatically enter maintenance mode and exit after
+   *                     the call succeeds. It won't proceed if the cluster is already in maintenance mode.
+   *                     Either case, the cluster must be in maintenance mode before config change.
+   */
+  public void addVirtualTopologyGroup(String clusterName, Map<String, String> customFields) {
+    // validation
+    CloudConfig cloudConfig = _configAccessor.getCloudConfig(clusterName);
+    if (cloudConfig == null || !cloudConfig.isCloudEnabled()) {
+      throw new HelixException(
+          "Cloud is not enabled, addVirtualTopologyGroup is not allowed to run in non-cloud environment.");
+    }
+    ClusterConfig clusterConfig = _configAccessor.getClusterConfig(clusterName);
+    Preconditions.checkState(clusterConfig.isTopologyAwareEnabled(),
+        "Topology-aware rebalance is not enabled in cluster " + clusterName);
+    String groupName = customFields.get(VirtualTopologyGroupConstants.GROUP_NAME);
+    String groupNumberStr = customFields.get(VirtualTopologyGroupConstants.GROUP_NUMBER);
+    Preconditions.checkArgument(!StringUtils.isEmpty(groupName), "virtualTopologyGroupName cannot be empty!");
+    Preconditions.checkArgument(!StringUtils.isEmpty(groupNumberStr), "virtualTopologyGroupNumber cannot be empty!");
+    int numGroups = 0;
+    try {
+      numGroups = Integer.parseInt(groupNumberStr);
+      Preconditions.checkArgument(numGroups > 0, "Number of virtual groups should be positive.");
+    } catch (NumberFormatException ex) {
+      throw new IllegalArgumentException("virtualTopologyGroupNumber " + groupNumberStr + " is not an integer.", ex);
+    }
+    LOG.info("Computing virtual topology group for cluster {} with param {}", clusterName, customFields);
+
+    // compute group assignment
+    ClusterTopology clusterTopology = _clusterService.getClusterTopology(clusterName);
+    Preconditions.checkArgument(numGroups <= clusterTopology.getAllInstances().size(),
+        "Number of virtual groups cannot be greater than the number of instances.");
+    Map<String, Set<String>> assignment =
+        _assignmentAlgorithm.computeAssignment(numGroups, groupName, clusterTopology.toZoneMapping());
+
+    boolean autoMaintenanceModeDisabled = Boolean.parseBoolean(
+        customFields.getOrDefault(VirtualTopologyGroupConstants.AUTO_MAINTENANCE_MODE_DISABLED, "false"));
+    // if auto mode is NOT disabled, let service enter maintenance mode and exit after the API succeeds.
+    if (!autoMaintenanceModeDisabled) {
+      Preconditions.checkState(!_helixAdmin.isInMaintenanceMode(clusterName),
+          "This operation is not allowed if cluster is already in maintenance mode before the API call. "
+              + "Please set autoMaintenanceModeDisabled=true if this is intended.");
+      _helixAdmin.manuallyEnableMaintenanceMode(clusterName, true,
+          "Enable maintenanceMode for virtual topology group change.", customFields);
+    }
+    Preconditions.checkState(_helixAdmin.isInMaintenanceMode(clusterName),
+        "Cluster is not in maintenance mode. This is required for virtual topology group setting. "
+            + "Please set autoMaintenanceModeDisabled=false (default) to let the cluster enter maintenance mode automatically, "
+            + "or use autoMaintenanceModeDisabled=true and control cluster maintenance mode in client side.");
+
+    updateConfigs(clusterName, clusterConfig, assignment);
+    if (!autoMaintenanceModeDisabled) {
+      _helixAdmin.manuallyEnableMaintenanceMode(clusterName, false,
+          "Disable maintenanceMode after virtual topology group change.", customFields);
+    }
+  }
+
+  private void updateConfigs(String clusterName, ClusterConfig clusterConfig, Map<String, Set<String>> assignment) {
+    List<String> zkPaths = new ArrayList<>();
+    List<DataUpdater<ZNRecord>> updaters = new ArrayList<>();
+    createInstanceConfigUpdater(clusterName, assignment).forEach((zkPath, updater) -> {
+      zkPaths.add(zkPath);
+      updaters.add(updater);
+    });
+    // update instance config
+    boolean[] results = _dataAccessor.updateChildren(zkPaths, updaters, AccessOption.EPHEMERAL);
+    for (int i = 0; i < results.length; i++) {
+      if (!results[i]) {
+        throw new HelixException("Failed to update instance config for path " + zkPaths.get(i));
+      }
+    }
+    // update cluster config
+    String virtualTopologyString = computeVirtualTopologyString(clusterConfig);
+    clusterConfig.setTopology(virtualTopologyString);
+    clusterConfig.setFaultZoneType(VirtualTopologyGroupConstants.VIRTUAL_FAULT_ZONE_TYPE);
+    _configAccessor.updateClusterConfig(clusterName, clusterConfig);
+    LOG.info("Successfully update instance and cluster config for {}", clusterName);
+  }
+
+  @VisibleForTesting
+  static String computeVirtualTopologyString(ClusterConfig clusterConfig) {
+    ClusterTopologyConfig clusterTopologyConfig = ClusterTopologyConfig.createFromClusterConfig(clusterConfig);
+    String endNodeType = clusterTopologyConfig.getEndNodeType();
+    String[] splits = new String[] {"", VirtualTopologyGroupConstants.VIRTUAL_FAULT_ZONE_TYPE, endNodeType};
+    return String.join(VirtualTopologyGroupConstants.PATH_NAME_SPLITTER, splits);
+  }
+
+  /**
+   * Create updater for instance config for async update.
+   * @param clusterName cluster name of the instances.
+   * @param assignment virtual group assignment.
+   * @return a map from instance zkPath to its {@link DataUpdater} to update.
+   */
+  @VisibleForTesting
+  static Map<String, DataUpdater<ZNRecord>> createInstanceConfigUpdater(
+      String clusterName, Map<String, Set<String>> assignment) {
+    Map<String, DataUpdater<ZNRecord>> updaters = new HashMap<>();
+    for (Map.Entry<String, Set<String>> entry : assignment.entrySet()) {
+      String virtualGroup = entry.getKey();
+      for (String instanceName : entry.getValue()) {
+        String path = PropertyPathBuilder.instanceConfig(clusterName, instanceName);
+        updaters.put(path, currentData -> {
+          InstanceConfig instanceConfig = new InstanceConfig(currentData);
+          Map<String, String> domainMap = instanceConfig.getDomainAsMap();
+          domainMap.put(VirtualTopologyGroupConstants.VIRTUAL_FAULT_ZONE_TYPE, virtualGroup);
+          instanceConfig.setDomain(domainMap);
+          return instanceConfig.getRecord();
+        });
+      }
+    }
+    return updaters;
+  }
+}
diff --git a/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestVirtualTopologyGroupService.java b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestVirtualTopologyGroupService.java
new file mode 100644
index 0000000..ab1c53f
--- /dev/null
+++ b/helix-rest/src/test/java/org/apache/helix/rest/server/service/TestVirtualTopologyGroupService.java
@@ -0,0 +1,192 @@
+package org.apache.helix.rest.server.service;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.helix.ConfigAccessor;
+import org.apache.helix.HelixAdmin;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.cloud.azure.AzureConstants;
+import org.apache.helix.cloud.constants.CloudProvider;
+import org.apache.helix.model.CloudConfig;
+import org.apache.helix.model.ClusterConfig;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.InstanceConfig;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.apache.helix.rest.server.json.cluster.ClusterTopology;
+import org.apache.helix.zookeeper.datamodel.ZNRecord;
+import org.apache.helix.zookeeper.zkclient.DataUpdater;
+import org.testng.Assert;
+import org.testng.annotations.BeforeTest;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+
+import static org.apache.helix.cloud.constants.VirtualTopologyGroupConstants.*;
+import static org.mockito.Mockito.*;
+
+
+public class TestVirtualTopologyGroupService {
+  private static final String TEST_CLUSTER = "Test_Cluster";
+  private static final String TEST_CLUSTER0 = "TestCluster_0";
+  private static final String TEST_CLUSTER1 = "TestCluster_1";
+
+  private final ConfigAccessor _configAccessor = mock(ConfigAccessor.class);
+  private final HelixDataAccessor _dataAccessor = mock(HelixDataAccessor.class);
+  private InstanceConfig _instanceConfig0;
+  private InstanceConfig _instanceConfig1;
+  private InstanceConfig _instanceConfig2;
+  private Map<String, DataUpdater<ZNRecord>> _updaterMap;
+  private HelixAdmin _helixAdmin;
+  private VirtualTopologyGroupService _service;
+
+  @BeforeTest
+  public void prepare() {
+    Map<String, Set<String>> assignment = new HashMap<>();
+    _instanceConfig0 = new InstanceConfig("instance_0");
+    _instanceConfig0.setDomain("helixZoneId=zone0");
+    _instanceConfig1 = new InstanceConfig("instance_1");
+    _instanceConfig1.setDomain("helixZoneId=zone0");
+    _instanceConfig2 = new InstanceConfig("instance_2");
+    _instanceConfig2.setDomain("helixZoneId=zone1");
+
+    assignment.put("virtual_group_0", ImmutableSet.of("instance_0", "instance_1"));
+    assignment.put("virtual_group_1", ImmutableSet.of("instance_2"));
+    _updaterMap = VirtualTopologyGroupService.createInstanceConfigUpdater(TEST_CLUSTER, assignment);
+
+    ClusterConfig clusterConfig = new ClusterConfig(TEST_CLUSTER0);
+    clusterConfig.setFaultZoneType(AzureConstants.AZURE_FAULT_ZONE_TYPE);
+    clusterConfig.setTopology(AzureConstants.AZURE_TOPOLOGY);
+    clusterConfig.setTopologyAwareEnabled(true);
+    when(_configAccessor.getClusterConfig(TEST_CLUSTER0)).thenReturn(clusterConfig);
+
+    CloudConfig.Builder cloudConfigBuilder = new CloudConfig.Builder();
+    cloudConfigBuilder.setCloudEnabled(true);
+    cloudConfigBuilder.setCloudProvider(CloudProvider.AZURE);
+    cloudConfigBuilder.setCloudID("TestID");
+    CloudConfig cloudConfig = cloudConfigBuilder.build();
+    when(_configAccessor.getCloudConfig(TEST_CLUSTER0)).thenReturn(cloudConfig);
+
+    _helixAdmin = mock(HelixAdmin.class);
+    when(_helixAdmin.isInMaintenanceMode(anyString())).thenReturn(true);
+
+    boolean[] results = new boolean[2];
+    results[0] = results[1] = true;
+    when(_dataAccessor.updateChildren(anyList(), anyList(), anyInt())).thenReturn(results);
+    ClusterService clusterService = mock(ClusterService.class);
+    when(clusterService.getClusterTopology(anyString())).thenReturn(prepareClusterTopology());
+    _service = new VirtualTopologyGroupService(_helixAdmin, clusterService, _configAccessor, _dataAccessor);
+  }
+
+  @Test(expectedExceptions = HelixException.class, expectedExceptionsMessageRegExp = "Cloud is not enabled.*")
+  public void testClusterCloudConfigSetup() {
+    ClusterConfig clusterConfig1 = new ClusterConfig(TEST_CLUSTER1);
+    when(_configAccessor.getClusterConfig(TEST_CLUSTER1)).thenReturn(clusterConfig1);
+    _service.addVirtualTopologyGroup(TEST_CLUSTER1, ImmutableMap.of(GROUP_NAME, "test-group", GROUP_NUMBER, "2"));
+  }
+
+  @Test
+  public void testVirtualTopologyGroupService() {
+    _service.addVirtualTopologyGroup(TEST_CLUSTER0, ImmutableMap.of(
+        GROUP_NAME, "test-group", GROUP_NUMBER, "2", AUTO_MAINTENANCE_MODE_DISABLED, "true"));
+    verify(_dataAccessor, times(1)).updateChildren(anyList(), anyList(), anyInt());
+    verify(_configAccessor, times(1)).updateClusterConfig(anyString(), any());
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class,
+      expectedExceptionsMessageRegExp = "This operation is not allowed if cluster is already in maintenance mode.*")
+  public void testMaintenanceModeCheckBeforeApiCall() {
+    _service.addVirtualTopologyGroup(TEST_CLUSTER0, ImmutableMap.of(GROUP_NAME, "test-group", GROUP_NUMBER, "2"));
+  }
+
+  @Test(expectedExceptions = IllegalStateException.class,
+  expectedExceptionsMessageRegExp = "Cluster is not in maintenance mode. This is required for virtual topology group setting. "
+      + "Please set autoMaintenanceModeDisabled=false.*")
+  public void testMaintenanceModeCheckAfter() {
+    try {
+      when(_helixAdmin.isInMaintenanceMode(anyString())).thenReturn(false);
+      _service.addVirtualTopologyGroup(TEST_CLUSTER0, ImmutableMap.of(GROUP_NAME, "test-group", GROUP_NUMBER, "2"));
+    } finally {
+      when(_helixAdmin.isInMaintenanceMode(anyString())).thenReturn(true);
+    }
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class,
+      expectedExceptionsMessageRegExp = "Number of virtual groups cannot be greater than the number of instances.*")
+  public void testNumberOfInstanceCheck() {
+    _service.addVirtualTopologyGroup(TEST_CLUSTER0, ImmutableMap.of(
+        GROUP_NAME, "test-group", GROUP_NUMBER, "10", AUTO_MAINTENANCE_MODE_DISABLED, "true"));
+  }
+
+  @Test(expectedExceptions = IllegalArgumentException.class)
+  public void testParamValidation() {
+    _service.addVirtualTopologyGroup(TEST_CLUSTER0, ImmutableMap.of(GROUP_NUMBER, "2"));
+  }
+
+  @Test(dataProvider = "instanceTestProvider")
+  public void testInstanceConfigUpdater(String zkPath, InstanceConfig instanceConfig, Map<String, String> expectedDomain) {
+    ZNRecord update = _updaterMap.get(zkPath).update(instanceConfig.getRecord());
+    InstanceConfig updatedConfig = new InstanceConfig(update);
+    Assert.assertEquals(updatedConfig.getDomainAsMap(), expectedDomain);
+  }
+
+  @DataProvider
+  public Object[][] instanceTestProvider() {
+    return new Object[][] {
+        {computeZkPath("instance_0"), _instanceConfig0,
+            ImmutableMap.of("helixZoneId", "zone0", VIRTUAL_FAULT_ZONE_TYPE, "virtual_group_0")},
+        {computeZkPath("instance_1"), _instanceConfig1,
+            ImmutableMap.of("helixZoneId", "zone0", VIRTUAL_FAULT_ZONE_TYPE, "virtual_group_0")},
+        {computeZkPath("instance_2"), _instanceConfig2,
+            ImmutableMap.of("helixZoneId", "zone1", VIRTUAL_FAULT_ZONE_TYPE, "virtual_group_1")}
+    };
+  }
+
+  @Test
+  public void testVirtualTopologyString() {
+    ClusterConfig testConfig = new ClusterConfig("testId");
+    testConfig.setTopologyAwareEnabled(true);
+    testConfig.setTopology("/zone/instance");
+    Assert.assertEquals(VirtualTopologyGroupService.computeVirtualTopologyString(testConfig),
+        "/virtualZone/instance");
+  }
+
+  private static ClusterTopology prepareClusterTopology() {
+    List<ClusterTopology.Zone> zones = ImmutableList.of(
+        new ClusterTopology.Zone("zone0", ImmutableList.of(
+            new ClusterTopology.Instance("instance_0"), new ClusterTopology.Instance("instance_1"))),
+        new ClusterTopology.Zone("zone1", ImmutableList.of(new ClusterTopology.Instance("instance_2"))));
+    return new ClusterTopology(TEST_CLUSTER0, zones, ImmutableSet.of("instance_0", "instance_1", "instance_2"));
+  }
+
+  private static String computeZkPath(String instanceName) {
+    HelixConfigScope scope = new HelixConfigScopeBuilder(HelixConfigScope.ConfigScopeProperty.PARTICIPANT)
+        .forCluster(TEST_CLUSTER)
+        .forParticipant(instanceName)
+        .build();
+    return scope.getZkPath();
+  }
+}