From fdff8ef46dc458d43b5dfee93eb3b4c4fae680cf Mon Sep 17 00:00:00 2001
From: Baiqiang Zhao <zbq.dean@gmail.com>
Date: Thu, 19 Nov 2020 09:20:01 +0800
Subject: [PATCH] HBASE-25298 hbase.rsgroup.fallback.enable should support
 dynamic configuration (#2668)

Signed-off-by: Guanghao Zhang <zghao@apache.org>
Signed-off-by: Viraj Jasani <vjasani@apache.org>
---
 .../rsgroup/RSGroupBasedLoadBalancer.java      | 12 ++++++++++--
 .../balancer/TestRSGroupBasedLoadBalancer.java | 18 ++++++++++++++++++
 src/main/asciidoc/_chapters/configuration.adoc |  1 +
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
index 92f84526d45..52f1681ca36 100644
--- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
+++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
@@ -247,7 +247,7 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
       }
       if (!fallbackRegions.isEmpty()) {
         List<ServerName> candidates = null;
-        if (fallbackEnabled) {
+        if (isFallbackEnabled()) {
           candidates = getFallBackCandidates(servers);
         }
         candidates = (candidates == null || candidates.isEmpty()) ?
@@ -383,6 +383,9 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
     return this.rsGroupInfoManager.isOnline();
   }
 
+  public boolean isFallbackEnabled() {
+    return fallbackEnabled;
+  }
 
   @Override
   public void regionOnline(RegionInfo regionInfo, ServerName sn) {
@@ -394,7 +397,12 @@ public class RSGroupBasedLoadBalancer implements RSGroupableBalancer {
 
   @Override
   public void onConfigurationChange(Configuration conf) {
-    //DO nothing for now
+    boolean newFallbackEnabled = conf.getBoolean(FALLBACK_GROUP_ENABLE_KEY, false);
+    if (fallbackEnabled != newFallbackEnabled) {
+      LOG.info("Changing the value of {} from {} to {}", FALLBACK_GROUP_ENABLE_KEY,
+        fallbackEnabled, newFallbackEnabled);
+      fallbackEnabled = newFallbackEnabled;
+    }
   }
 
   @Override
diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
index 9b0202e1f69..e7ee7a9eedb 100644
--- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
+++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.master.balancer;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
@@ -186,4 +187,21 @@ public class TestRSGroupBasedLoadBalancer extends RSGroupableBalancerTestBase {
         .roundRobinAssignment(regions, onlineServers);
     assertEquals(bogusRegion, assignments.get(LoadBalancer.BOGUS_SERVER_NAME).size());
   }
+
+  @Test
+  public void testOnConfigurationChange() {
+    // fallbackEnabled default is false
+    assertFalse(loadBalancer.isFallbackEnabled());
+
+    // change FALLBACK_GROUP_ENABLE_KEY from false to true
+    Configuration conf = loadBalancer.getConf();
+    conf.setBoolean(RSGroupBasedLoadBalancer.FALLBACK_GROUP_ENABLE_KEY, true);
+    loadBalancer.onConfigurationChange(conf);
+    assertTrue(loadBalancer.isFallbackEnabled());
+
+    // restore
+    conf.setBoolean(RSGroupBasedLoadBalancer.FALLBACK_GROUP_ENABLE_KEY, false);
+    loadBalancer.onConfigurationChange(conf);
+    assertFalse(loadBalancer.isFallbackEnabled());
+  }
 }
diff --git a/src/main/asciidoc/_chapters/configuration.adoc b/src/main/asciidoc/_chapters/configuration.adoc
index b25ee96cb1c..5e5ecd5eace 100644
--- a/src/main/asciidoc/_chapters/configuration.adoc
+++ b/src/main/asciidoc/_chapters/configuration.adoc
@@ -1383,6 +1383,7 @@ Here are those configurations:
 | hbase.master.balancer.stochastic.tableSkewCost
 | hbase.master.regions.recovery.check.interval
 | hbase.regions.recovery.store.file.ref.count
+| hbase.rsgroup.fallback.enable
 |===
 
 ifdef::backend-docbook[]