HBASE-24759 Refuse to update configuration of default group

apache · Aug 29, 2020 · 295617e · 295617e
1 parent 047e061
commit 295617e
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 31 deletions.
diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfo.java
@@ -216,15 +216,18 @@ public boolean equals(Object o) {
       return false;
     }
 
-    RSGroupInfo RSGroupInfo = (RSGroupInfo) o;
+    RSGroupInfo rsGroupInfo = (RSGroupInfo) o;
 
-    if (!name.equals(RSGroupInfo.name)) {
+    if (!name.equals(rsGroupInfo.name)) {
       return false;
     }
-    if (!servers.equals(RSGroupInfo.servers)) {
+    if (!servers.equals(rsGroupInfo.servers)) {
       return false;
     }
-    if (!tables.equals(RSGroupInfo.tables)) {
+    if (!tables.equals(rsGroupInfo.tables)) {
+      return false;
+    }
+    if (!configuration.equals(rsGroupInfo.configuration)) {
       return false;
     }
 

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
@@ -696,7 +696,7 @@ private void saveRSGroupMapToZK(Map<String, RSGroupInfo> newGroupMap) throws IOE
           ZKUtil.createAndFailSilent(watcher, znode);
           zkOps.add(ZKUtil.ZKUtilOp.deleteNodeFailSilent(znode));
           zkOps.add(ZKUtil.ZKUtilOp.createAndFailSilent(znode,
-              ProtobufUtil.prependPBMagic(proto.toByteArray())));
+            ProtobufUtil.prependPBMagic(proto.toByteArray())));
         }
       }
       LOG.debug("Writing ZK GroupInfo count: " + zkOps.size());
@@ -1270,6 +1270,9 @@ public synchronized void renameRSGroup(String oldName, String newName) throws IO
   @Override
   public synchronized void updateRSGroupConfig(String groupName, Map<String, String> configuration)
       throws IOException {
+    if (RSGroupInfo.DEFAULT_GROUP.equals(groupName)) {
+      throw new ConstraintException(RSGroupInfo.DEFAULT_GROUP + " can't be stored persistently");
+    }
     RSGroupInfo rsGroupInfo = getRSGroupInfo(groupName);
     rsGroupInfo.getConfiguration().forEach((k, v) -> rsGroupInfo.removeConfiguration(k));
     configuration.forEach((k, v) -> rsGroupInfo.setConfiguration(k, v));

diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java
@@ -18,14 +18,12 @@
 package org.apache.hadoop.hbase.rsgroup;
 
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 
-import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
-
 import org.apache.hadoop.hbase.HBaseClassTestRule;
-import org.apache.hadoop.hbase.client.Admin;
+import org.apache.hadoop.hbase.constraint.ConstraintException;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.apache.hadoop.hbase.testclassification.RSGroupTests;
 import org.junit.AfterClass;
@@ -38,8 +36,6 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
-
 @Category({ RSGroupTests.class, MediumTests.class })
 public class TestRSGroupConfig extends TestRSGroupsBase {
 
@@ -52,12 +48,9 @@ public class TestRSGroupConfig extends TestRSGroupsBase {
 
   protected static final Logger LOG = LoggerFactory.getLogger(TestRSGroupConfig.class);
 
-  private static Admin admin;
-
   @BeforeClass
   public static void setUpBeforeClass() throws Exception {
     TestRSGroupsBase.setUpTestBeforeClass();
-    admin = TEST_UTIL.getAdmin();
   }
 
   @AfterClass
@@ -66,33 +59,30 @@ public static void tearDownAfterClass() throws Exception {
   }
 
   @Test
-  public void testSetDefaultGroupConfiguration() throws IOException {
-    testSetConfiguration(RSGroupInfo.DEFAULT_GROUP);
+  public void testSetDefaultGroupConfiguration() throws Exception {
+    assertThrows(ConstraintException.class, () -> testSetConfiguration(RSGroupInfo.DEFAULT_GROUP));
   }
 
   @Test
-  public void testSetNonDefaultGroupConfiguration() throws IOException {
+  public void testSetNonDefaultGroupConfiguration() throws Exception {
     String group = GROUP_PREFIX + name.getMethodName();
-    admin.addRSGroup(group);
-    testSetConfiguration(RSGroupInfo.DEFAULT_GROUP);
-    admin.removeRSGroup(group);
+    ADMIN.addRSGroup(group);
+    testSetConfiguration(group);
+    ADMIN.removeRSGroup(group);
   }
 
-  private void testSetConfiguration(String group) throws IOException {
+  private void testSetConfiguration(String group) throws Exception {
     Map<String, String> configuration = new HashMap<>();
     configuration.put("aaa", "111");
     configuration.put("bbb", "222");
-    admin.updateRSGroupConfig(group, configuration);
-    RSGroupInfo rsGroup = admin.getRSGroup(group);
-    Map<String, String> configFromGroup = Maps.newHashMap(rsGroup.getConfiguration());
-    assertNotNull(configFromGroup);
-    assertEquals(2, configFromGroup.size());
-    assertEquals("111", configFromGroup.get("aaa"));
+    ADMIN.updateRSGroupConfig(group, configuration);
+    RSGroupInfo rsGroup = ADMIN.getRSGroup(group);
+    assertEquals(configuration, rsGroup.getConfiguration());
+
     // unset configuration
-    admin.updateRSGroupConfig(group, null);
-    rsGroup = admin.getRSGroup(group);
-    configFromGroup = rsGroup.getConfiguration();
-    assertNotNull(configFromGroup);
+    ADMIN.updateRSGroupConfig(group, null);
+    rsGroup = ADMIN.getRSGroup(group);
+    Map<String, String> configFromGroup = rsGroup.getConfiguration();
     assertEquals(0, configFromGroup.size());
   }