HBASE-22642 Make move operations of RSGroup idempotent (apache#387)

Signed-off-by: Guanghao Zhang <zghao@apache.org>
infraio · Sep 20, 2019 · 8abf0a7 · 8abf0a7
1 parent 1926385
commit 8abf0a7
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 30 deletions.
diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupAdminServer.java
@@ -163,10 +163,7 @@ private void checkServersAndTables(Set<Address> servers, Set<TableName> tables,
               + " does not exist.");
     }
     RSGroupInfo srcGrp = new RSGroupInfo(tmpSrcGrp);
-    if (srcGrp.getName().equals(targetGroupName)) {
-      throw new ConstraintException("Target RSGroup " + targetGroupName +
-              " is same as source " + srcGrp.getName() + " RSGroup.");
-    }
+
     // Only move online servers
     checkOnlineServersOnly(servers);
 
@@ -328,10 +325,6 @@ public void moveServers(Set<Address> servers, String targetGroupName) throws IOE
         throw new ConstraintException("Source RSGroup for server " + firstServer
             + " does not exist.");
       }
-      if (srcGrp.getName().equals(targetGroupName)) {
-        throw new ConstraintException("Target RSGroup " + targetGroupName +
-            " is same as source " + srcGrp + " RSGroup.");
-      }
       // Only move online servers (when moving from 'default') or servers from other
       // groups. This prevents bogus servers from entering groups
       if (RSGroupInfo.DEFAULT_GROUP.equals(srcGrp.getName())) {
@@ -383,16 +376,6 @@ public void moveTables(Set<TableName> tables, String targetGroup) throws IOExcep
           throw new ConstraintException("Target RSGroup must have at least one server.");
         }
       }
-
-      for (TableName table : tables) {
-        String srcGroup = rsGroupInfoManager.getRSGroupOfTable(table);
-        if(srcGroup != null && srcGroup.equals(targetGroup)) {
-          throw new ConstraintException(
-              "Source RSGroup " + srcGroup + " is same as target " + targetGroup +
-              " RSGroup for table " + table);
-        }
-        LOG.info("Moving table {} to RSGroup {}", table.getNameAsString(), targetGroup);
-      }
       rsGroupInfoManager.moveTables(tables, targetGroup);
 
       // targetGroup is null when a table is being deleted. In this case no further

diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin2.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupsAdmin2.java
@@ -20,6 +20,7 @@
 import static org.apache.hadoop.hbase.util.Threads.sleep;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
@@ -29,6 +30,7 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.hadoop.hbase.ClusterMetrics.Option;
@@ -54,6 +56,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.xiaomi.infra.thirdparty.com.google.common.base.Preconditions;
 import com.xiaomi.infra.thirdparty.com.google.common.collect.Sets;
 
 @Category({ LargeTests.class })
@@ -338,17 +341,9 @@ public boolean evaluate() throws Exception {
       assertTrue(msg + " " + ex.getMessage(), ex.getMessage().contains(exp));
     }
 
-    // test fail server move
-    try {
-      rsGroupAdmin.moveServersAndTables(Sets.newHashSet(targetServer.getAddress()),
-        Sets.newHashSet(tableName), RSGroupInfo.DEFAULT_GROUP);
-      fail("servers shouldn't have been successfully moved.");
-    } catch (IOException ex) {
-      String exp = "Target RSGroup " + RSGroupInfo.DEFAULT_GROUP + " is same as source " +
-        RSGroupInfo.DEFAULT_GROUP + " RSGroup.";
-      String msg = "Expected '" + exp + "' in exception message: ";
-      assertTrue(msg + " " + ex.getMessage(), ex.getMessage().contains(exp));
-    }
+    // test move when src = dst
+    rsGroupAdmin.moveServersAndTables(Sets.newHashSet(targetServer.getAddress()),
+      Sets.newHashSet(tableName), RSGroupInfo.DEFAULT_GROUP);
 
     // verify default group info
     Assert.assertEquals(oldDefaultGroupServerSize,
@@ -655,5 +650,4 @@ public boolean evaluate() {
       }
     });
   }
-
 }