HBASE-17101: FavoredNodes should not apply to system tables

Signed-off-by: Francis Liu <toffer@apache.org>

hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java

@@ -114,7 +114,7 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored
           currentServer.getPort(), ServerName.NON_STARTCODE);
       List<HRegionInfo> list = entry.getValue();
       for (HRegionInfo region : list) {
-        if(region.getTable().isSystemTable()) {
+        if(!FavoredNodesManager.isFavoredNodeApplicable(region)) {
           continue;
         }
         List<ServerName> favoredNodes = fnm.getFavoredNodes(region);
@@ -209,7 +209,8 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored
           new FavoredNodeAssignmentHelper(servers, rackManager);
       assignmentHelper.initialize();
       ServerName primary = super.randomAssignment(regionInfo, servers);
-      if (!assignmentHelper.canPlaceFavoredNodes()) {
+      if (!FavoredNodesManager.isFavoredNodeApplicable(regionInfo)
+          || !assignmentHelper.canPlaceFavoredNodes()) {
         return primary;
       }
       List<ServerName> favoredNodes = fnm.getFavoredNodes(regionInfo);

hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesManager.java

@@ -83,6 +83,14 @@ public class FavoredNodesManager {
     return this.globalFavoredNodesAssignmentPlan.getFavoredNodes(regionInfo);
   }
 
+  /*
+   * Favored nodes are not applicable for system tables. We will use this to check before
+   * we apply any favored nodes logic on a region.
+   */
+  public static boolean isFavoredNodeApplicable(HRegionInfo regionInfo) {
+    return !regionInfo.isSystemTable();
+  }
+
   public synchronized void updateFavoredNodes(Map<HRegionInfo, List<ServerName>> regionFNMap)
       throws IOException {
 
@@ -99,8 +107,8 @@ public class FavoredNodesManager {
         throw new IOException("Duplicates found: " + servers);
       }
 
-      if (regionInfo.isSystemTable()) {
+      if (!isFavoredNodeApplicable(regionInfo)) {
-        throw new IOException("Can't update FN for system region: "
+        throw new IOException("Can't update FN for a un-applicable region: "
             + regionInfo.getRegionNameAsString() + " with " + servers);
       }
 

hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java

@@ -69,6 +69,7 @@ import org.apache.hadoop.hbase.client.TableState;
 import org.apache.hadoop.hbase.executor.EventHandler;
 import org.apache.hadoop.hbase.executor.EventType;
 import org.apache.hadoop.hbase.executor.ExecutorService;
+import org.apache.hadoop.hbase.favored.FavoredNodesManager;
 import org.apache.hadoop.hbase.favored.FavoredNodesPromoter;
 import org.apache.hadoop.hbase.ipc.FailedServerException;
 import org.apache.hadoop.hbase.ipc.RpcClient;
@@ -622,7 +623,7 @@ public class AssignmentManager {
 
   void processFavoredNodesForDaughters(HRegionInfo parent,
     HRegionInfo regionA, HRegionInfo regionB) throws IOException {
-    if (shouldAssignRegionsWithFavoredNodes) {
+    if (shouldAssignFavoredNodes(parent)) {
       List<ServerName> onlineServers = this.serverManager.getOnlineServersList();
       ((FavoredNodesPromoter) this.balancer).
           generateFavoredNodesForDaughter(onlineServers, parent, regionA, regionB);
@@ -631,12 +632,21 @@ public class AssignmentManager {
 
   void processFavoredNodesForMerge(HRegionInfo merged, HRegionInfo regionA, HRegionInfo regionB)
     throws IOException {
-    if (shouldAssignRegionsWithFavoredNodes) {
+    if (shouldAssignFavoredNodes(merged)) {
       ((FavoredNodesPromoter)this.balancer).
         generateFavoredNodesForMergedRegion(merged, regionA, regionB);
     }
   }
 
+  /*
+   * Favored nodes should be applied only when FavoredNodes balancer is configured and the region
+   * belongs to a non-system table.
+   */
+  private boolean shouldAssignFavoredNodes(HRegionInfo region) {
+    return this.shouldAssignRegionsWithFavoredNodes
+        && FavoredNodesManager.isFavoredNodeApplicable(region);
+  }
+
   /**
    * Marks the region as online.  Removes it from regions in transition and
    * updates the in-memory assignment information.
@@ -794,7 +804,7 @@ public class AssignmentManager {
           regionStates.updateRegionState(
             region, State.PENDING_OPEN, destination);
           List<ServerName> favoredNodes = ServerName.EMPTY_SERVER_LIST;
-          if (this.shouldAssignRegionsWithFavoredNodes) {
+          if (shouldAssignFavoredNodes(region)) {
             favoredNodes = server.getFavoredNodesManager().getFavoredNodes(region);
           }
           regionOpenInfos.add(new Pair<HRegionInfo, List<ServerName>>(
@@ -1102,7 +1112,7 @@ public class AssignmentManager {
             " to " + plan.getDestination();
         try {
           List<ServerName> favoredNodes = ServerName.EMPTY_SERVER_LIST;
-          if (this.shouldAssignRegionsWithFavoredNodes) {
+          if (shouldAssignFavoredNodes(region)) {
             favoredNodes = server.getFavoredNodesManager().getFavoredNodes(region);
           }
           serverManager.sendRegionOpen(plan.getDestination(), region, favoredNodes);
@@ -1847,7 +1857,7 @@ public class AssignmentManager {
                   return; // Region is not in the expected state any more
                 }
                 List<ServerName> favoredNodes = ServerName.EMPTY_SERVER_LIST;
-                if (shouldAssignRegionsWithFavoredNodes) {
+                if (shouldAssignFavoredNodes(hri)) {
                   favoredNodes =
                     ((MasterServices)server).getFavoredNodesManager().getFavoredNodes(hri);
                 }

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java

@@ -1801,8 +1801,10 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
             regionServer.service.submit(new OpenMetaHandler(
               regionServer, regionServer, region, htd, masterSystemTime));
           } else {
-            regionServer.updateRegionFavoredNodesMapping(region.getEncodedName(),
+            if (regionOpenInfo.getFavoredNodesCount() > 0) {
-              regionOpenInfo.getFavoredNodesList());
+              regionServer.updateRegionFavoredNodesMapping(region.getEncodedName(),
+                  regionOpenInfo.getFavoredNodesList());
+            }
             if (htd.getPriority() >= HConstants.ADMIN_QOS || region.getTable().isSystemTable()) {
               regionServer.service.submit(new OpenPriorityRegionHandler(
                 regionServer, regionServer, region, htd, masterSystemTime));
@@ -2100,8 +2102,10 @@ public class RSRpcServices implements HBaseRPCErrorHandler,
     UpdateFavoredNodesResponse.Builder respBuilder = UpdateFavoredNodesResponse.newBuilder();
     for (UpdateFavoredNodesRequest.RegionUpdateInfo regionUpdateInfo : openInfoList) {
       HRegionInfo hri = HRegionInfo.convert(regionUpdateInfo.getRegion());
-      regionServer.updateRegionFavoredNodesMapping(hri.getEncodedName(),
+      if (regionUpdateInfo.getFavoredNodesCount() > 0) {
-        regionUpdateInfo.getFavoredNodesList());
+        regionServer.updateRegionFavoredNodesMapping(hri.getEncodedName(),
+          regionUpdateInfo.getFavoredNodesList());
+      }
     }
     respBuilder.setResponse(openInfoList.size());
     return respBuilder.build();

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestTableFavoredNodes.java

@@ -35,6 +35,7 @@ import java.util.concurrent.TimeUnit;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.NamespaceDescriptor;
 import org.apache.hadoop.hbase.favored.FavoredNodeAssignmentHelper;
 import org.apache.hadoop.hbase.favored.FavoredNodeLoadBalancer;
 import org.apache.hadoop.hbase.favored.FavoredNodesManager;
@@ -325,6 +326,30 @@ public class TestTableFavoredNodes {
     }
   }
 
+  /*
+   * Check favored nodes for system tables
+   */
+  @Test
+  public void testSystemTables() throws Exception {
+
+    TableName tableName = TableName.valueOf("createTable");
+    TEST_UTIL.createTable(tableName, Bytes.toBytes("f"), splitKeys);
+    TEST_UTIL.waitUntilAllRegionsAssigned(tableName);
+
+    // All regions should have favored nodes
+    checkIfFavoredNodeInformationIsCorrect(tableName);
+
+    for (TableName sysTable :
+        admin.listTableNamesByNamespace(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR)) {
+      List<HRegionInfo> regions = admin.getTableRegions(sysTable);
+      for (HRegionInfo region : regions) {
+        assertNull("FN should be null for sys region", fnm.getFavoredNodes(region));
+      }
+    }
+
+    TEST_UTIL.deleteTable(tableName);
+  }
+
   private void checkIfDaughterInherits2FN(List<ServerName> parentFN, List<ServerName> daughterFN) {
 
     assertNotNull(parentFN);