HBASE-16956 Refactor FavoredNodePlan to use regionNames as keys (Thiruvel Thirumoolan)

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

@@ -409,7 +409,7 @@ public class RegionPlacementMaintainer {
           favoredServers.add(ServerName.valueOf(s.getHostname(), s.getPort(),
               ServerName.NON_STARTCODE));
           // Update the assignment plan
-          plan.updateAssignmentPlan(regions.get(i), favoredServers);
+          plan.updateFavoredNodesMap(regions.get(i), favoredServers);
         }
         LOG.info("Generated the assignment plan for " + numRegions +
             " regions from table " + tableName + " with " +
@@ -444,7 +444,7 @@ public class RegionPlacementMaintainer {
           favoredServers.add(ServerName.valueOf(s.getHostname(), s.getPort(),
               ServerName.NON_STARTCODE));
           // Update the assignment plan
-          plan.updateAssignmentPlan(regions.get(i), favoredServers);
+          plan.updateFavoredNodesMap(regions.get(i), favoredServers);
         }
         LOG.info("Generated the assignment plan for " + numRegions +
             " regions from table " + tableName + " with " +
@@ -614,13 +614,13 @@ public class RegionPlacementMaintainer {
     if (plan == null) return;
     LOG.info("========== Start to print the assignment plan ================");
     // sort the map based on region info
-    Map<HRegionInfo, List<ServerName>> assignmentMap =
-      new TreeMap<HRegionInfo, List<ServerName>>(plan.getAssignmentMap());
+    Map<String, List<ServerName>> assignmentMap =
+      new TreeMap<String, List<ServerName>>(plan.getAssignmentMap());
 
-    for (Map.Entry<HRegionInfo, List<ServerName>> entry : assignmentMap.entrySet()) {
+    for (Map.Entry<String, List<ServerName>> entry : assignmentMap.entrySet()) {
 
       String serverList = FavoredNodeAssignmentHelper.getFavoredNodesAsString(entry.getValue());
-      String regionName = entry.getKey().getRegionNameAsString();
+      String regionName = entry.getKey();
       LOG.info("Region: " + regionName );
       LOG.info("Its favored nodes: " + serverList);
     }
@@ -636,9 +636,15 @@ public class RegionPlacementMaintainer {
   throws IOException {
     try {
       LOG.info("Start to update the hbase:meta with the new assignment plan");
-      Map<HRegionInfo, List<ServerName>> assignmentMap =
-        plan.getAssignmentMap();
-      FavoredNodeAssignmentHelper.updateMetaWithFavoredNodesInfo(assignmentMap, conf);
+      Map<String, List<ServerName>> assignmentMap = plan.getAssignmentMap();
+      Map<HRegionInfo, List<ServerName>> planToUpdate = new HashMap<>(assignmentMap.size());
+      Map<String, HRegionInfo> regionToRegionInfoMap =
+        getRegionAssignmentSnapshot().getRegionNameToRegionInfoMap();
+      for (Map.Entry<String, List<ServerName>> entry : assignmentMap.entrySet()) {
+        planToUpdate.put(regionToRegionInfoMap.get(entry.getKey()), entry.getValue());
+      }
+
+      FavoredNodeAssignmentHelper.updateMetaWithFavoredNodesInfo(planToUpdate, conf);
       LOG.info("Updated the hbase:meta with the new assignment plan");
     } catch (Exception e) {
       LOG.error("Failed to update hbase:meta with the new assignment" +
@@ -680,7 +686,7 @@ public class RegionPlacementMaintainer {
               singleServerPlan = new FavoredNodesPlan();
             }
             // Update the single server update
-            singleServerPlan.updateAssignmentPlan(region, favoredServerList);
+            singleServerPlan.updateFavoredNodesMap(region, favoredServerList);
             regionUpdateInfos.add(
               new Pair<HRegionInfo, List<ServerName>>(region, favoredServerList));
           }
@@ -1104,7 +1110,7 @@ public class RegionPlacementMaintainer {
             LOG.error("Cannot parse the invalid favored nodes because " + e);
           }
           FavoredNodesPlan newPlan = new FavoredNodesPlan();
-          newPlan.updateAssignmentPlan(regionInfo, favoredNodes);
+          newPlan.updateFavoredNodesMap(regionInfo, favoredNodes);
           rp.updateAssignmentPlan(newPlan);
         }
       } else {

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

@@ -22,14 +22,12 @@ import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.hbase.classification.InterfaceAudience;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.ServerName;
 
 /**
- * This class contains the mapping information between each region and
+ * This class contains the mapping information between each region name and
  * its favored region server list. Used by {@link FavoredNodeLoadBalancer} set
  * of classes and from unit tests (hence the class is public)
  *
@@ -37,40 +35,38 @@ import org.apache.hadoop.hbase.ServerName;
  */
 @InterfaceAudience.Private
 public class FavoredNodesPlan {
-  private static final Log LOG = LogFactory.getLog(
-      FavoredNodesPlan.class.getName());
 
-  /** the map between each region and its favored region server list */
-  private Map<HRegionInfo, List<ServerName>> favoredNodesMap;
+  /** the map between each region name and its favored region server list */
+  private Map<String, List<ServerName>> favoredNodesMap;
 
   public static enum Position {
     PRIMARY,
     SECONDARY,
-    TERTIARY;
-  };
+    TERTIARY
+  }
 
   public FavoredNodesPlan() {
-    favoredNodesMap = new ConcurrentHashMap<HRegionInfo, List<ServerName>>();
+    favoredNodesMap = new ConcurrentHashMap<String, List<ServerName>>();
   }
 
   /**
-   * Add an assignment to the plan
+   * Update an assignment to the plan
    * @param region
    * @param servers
    */
-  public synchronized void updateFavoredNodesMap(HRegionInfo region,
-      List<ServerName> servers) {
-    if (region == null || servers == null || servers.size() ==0)
+  public void updateFavoredNodesMap(HRegionInfo region, List<ServerName> servers) {
+    if (region == null || servers == null || servers.size() == 0) {
       return;
-    this.favoredNodesMap.put(region, servers);
+    }
+    this.favoredNodesMap.put(region.getRegionNameAsString(), servers);
   }
 
   /**
    * @param region
    * @return the list of favored region server for this region based on the plan
    */
-  public synchronized List<ServerName> getFavoredNodes(HRegionInfo region) {
-    return favoredNodesMap.get(region);
+  public List<ServerName> getFavoredNodes(HRegionInfo region) {
+    return favoredNodesMap.get(region.getRegionNameAsString());
   }
 
   /**
@@ -97,23 +93,8 @@ public class FavoredNodesPlan {
   /**
    * @return the mapping between each region to its favored region server list
    */
-  public synchronized Map<HRegionInfo, List<ServerName>> getAssignmentMap() {
-    return this.favoredNodesMap;
-  }
-
-  /**
-   * Add an assignment to the plan
-   * @param region
-   * @param servers
-   */
-  public synchronized void updateAssignmentPlan(HRegionInfo region,
-      List<ServerName> servers) {
-    if (region == null || servers == null || servers.size() ==0)
-      return;
-    this.favoredNodesMap.put(region, servers);
-    LOG.info("Update the assignment plan for region " +
-        region.getRegionNameAsString() + " ; favored nodes " +
-        FavoredNodeAssignmentHelper.getFavoredNodesAsString(servers));
+  public Map<String, List<ServerName>> getAssignmentMap() {
+    return favoredNodesMap;
   }
 
   @Override
@@ -128,15 +109,14 @@ public class FavoredNodesPlan {
       return false;
     }
     // To compare the map from objec o is identical to current assignment map.
-    Map<HRegionInfo, List<ServerName>> comparedMap=
-      ((FavoredNodesPlan)o).getAssignmentMap();
+    Map<String, List<ServerName>> comparedMap = ((FavoredNodesPlan)o).getAssignmentMap();
 
     // compare the size
     if (comparedMap.size() != this.favoredNodesMap.size())
       return false;
 
     // compare each element in the assignment map
-    for (Map.Entry<HRegionInfo, List<ServerName>> entry :
+    for (Map.Entry<String, List<ServerName>> entry :
       comparedMap.entrySet()) {
       List<ServerName> serverList = this.favoredNodesMap.get(entry.getKey());
       if (serverList == null && entry.getValue() != null) {

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRegionPlacement.java

@@ -299,12 +299,13 @@ public class TestRegionPlacement {
    * @return
    */
   private FavoredNodesPlan shuffleAssignmentPlan(FavoredNodesPlan plan,
-      FavoredNodesPlan.Position p1, FavoredNodesPlan.Position p2) {
+      FavoredNodesPlan.Position p1, FavoredNodesPlan.Position p2) throws IOException {
     FavoredNodesPlan shuffledPlan = new FavoredNodesPlan();
 
-    for (Map.Entry<HRegionInfo, List<ServerName>> entry :
+    Map<String, HRegionInfo> regionToHRegion =
+        rp.getRegionAssignmentSnapshot().getRegionNameToRegionInfoMap();
+    for (Map.Entry<String, List<ServerName>> entry :
       plan.getAssignmentMap().entrySet()) {
-      HRegionInfo region = entry.getKey();
 
       // copy the server list from the original plan
       List<ServerName> shuffledServerList = new ArrayList<ServerName>();
@@ -315,7 +316,7 @@ public class TestRegionPlacement {
       shuffledServerList.set(p2.ordinal(), entry.getValue().get(p1.ordinal()));
 
       // update the plan
-      shuffledPlan.updateAssignmentPlan(region, shuffledServerList);
+      shuffledPlan.updateFavoredNodesMap(regionToHRegion.get(entry.getKey()), shuffledServerList);
     }
     return shuffledPlan;
   }