From c7b930a9e8f107cac9dfb03b7669f6c27c9b6ba1 Mon Sep 17 00:00:00 2001
From: XinSun <ddupgs@gmail.com>
Date: Fri, 4 Sep 2020 18:45:12 +0800
Subject: [PATCH] HBASE-24759 Refuse to update configuration of default group
 (#2350)

Signed-off-by: Guanghao Zhang <zghao@apache.org>
---
 .../hadoop/hbase/rsgroup/RSGroupInfo.java       | 11 +++++++----
 .../hbase/rsgroup/RSGroupInfoManagerImpl.java   |  6 ++++++
 .../hadoop/hbase/rsgroup/TestRSGroupConfig.java | 17 +++++++----------
 3 files changed, 20 insertions(+), 14 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
index 2fe2ba987f6..c96a80c3370 100644
--- 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
@@ -177,15 +177,18 @@ public class RSGroupInfo {
       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-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
index 2b727edbff5..74e7bc9dfeb 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupInfoManagerImpl.java
@@ -459,6 +459,12 @@ final class RSGroupInfoManagerImpl implements RSGroupInfoManager {
   @Override
   public void updateRSGroupConfig(String groupName, Map<String, String> configuration)
       throws IOException {
+    if (RSGroupInfo.DEFAULT_GROUP.equals(groupName)) {
+      // We do not persist anything of default group, therefore, it is not supported to update
+      // default group's configuration which lost once master down.
+      throw new ConstraintException("configuration of " + RSGroupInfo.DEFAULT_GROUP
+          + " can't be stored persistently");
+    }
     RSGroupInfo rsGroupInfo = getRSGroupInfo(groupName);
     new HashSet<>(rsGroupInfo.getConfiguration().keySet())
         .forEach(rsGroupInfo::removeConfiguration);
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java
index 79d0a1e01e4..e427d780c45 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/rsgroup/TestRSGroupConfig.java
@@ -19,12 +19,14 @@ 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.constraint.ConstraintException;
 import org.apache.hadoop.hbase.testclassification.MediumTests;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
@@ -36,8 +38,6 @@ import org.junit.rules.TestName;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import org.apache.hbase.thirdparty.com.google.common.collect.Maps;
-
 @Category(MediumTests.class)
 public class TestRSGroupConfig extends TestRSGroupsBase {
 
@@ -61,15 +61,15 @@ public class TestRSGroupConfig extends TestRSGroupsBase {
   }
 
   @Test
-  public void testSetDefaultGroupConfiguration() throws IOException {
-    testSetConfiguration(RSGroupInfo.DEFAULT_GROUP);
+  public void testSetDefaultGroupConfiguration() {
+    assertThrows(ConstraintException.class, () -> testSetConfiguration(RSGroupInfo.DEFAULT_GROUP));
   }
 
   @Test
   public void testSetNonDefaultGroupConfiguration() throws IOException {
     String group = getGroupName(name.getMethodName());
     rsGroupAdmin.addRSGroup(group);
-    testSetConfiguration(RSGroupInfo.DEFAULT_GROUP);
+    testSetConfiguration(group);
     rsGroupAdmin.removeRSGroup(group);
   }
 
@@ -79,14 +79,11 @@ public class TestRSGroupConfig extends TestRSGroupsBase {
     configuration.put("bbb", "222");
     rsGroupAdmin.updateRSGroupConfig(group, configuration);
     RSGroupInfo rsGroup = rsGroupAdmin.getRSGroupInfo(group);
-    Map<String, String> configFromGroup = Maps.newHashMap(rsGroup.getConfiguration());
-    assertNotNull(configFromGroup);
-    assertEquals(2, configFromGroup.size());
-    assertEquals("111", configFromGroup.get("aaa"));
+    assertEquals(configuration, rsGroup.getConfiguration());
     // unset configuration
     rsGroupAdmin.updateRSGroupConfig(group, null);
     rsGroup = rsGroupAdmin.getRSGroupInfo(group);
-    configFromGroup = rsGroup.getConfiguration();
+    Map<String, String> configFromGroup = rsGroup.getConfiguration();
     assertNotNull(configFromGroup);
     assertEquals(0, configFromGroup.size());
   }