HBASE-23737 [Flakey Tests] TestFavoredNodeTableImport fails 30% of the time; AMENDMENT

This is actual fix; previous added debug to test.

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

@@ -1,5 +1,4 @@
-/**
+/*
- *
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -23,7 +22,6 @@ import static org.apache.hadoop.hbase.favored.FavoredNodeAssignmentHelper.FAVORE
 import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.PRIMARY;
 import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.SECONDARY;
 import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.TERTIARY;
-
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
@@ -31,9 +29,8 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-
+import java.util.stream.Collectors;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.HBaseIOException;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.master.MasterServices;
@@ -56,22 +53,21 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Sets;
  * should be done through this class. There should only be one instance of
  * {@link FavoredNodesManager} in Master. {@link FavoredNodesPlan} and favored node information
  * from {@link SnapshotOfRegionAssignmentFromMeta} should not be used outside this class (except
- * for may be tools that only read or test cases). All other classes including Favored balancers
+ * for tools that only read or fortest cases). All other classes including Favored balancers
  * and {@link FavoredNodeAssignmentHelper} should use {@link FavoredNodesManager} for any
  * read/write/deletes to favored nodes.
  */
 @InterfaceAudience.Private
 public class FavoredNodesManager {
-
   private static final Logger LOG = LoggerFactory.getLogger(FavoredNodesManager.class);
 
-  private FavoredNodesPlan globalFavoredNodesAssignmentPlan;
+  private final FavoredNodesPlan globalFavoredNodesAssignmentPlan;
-  private Map<ServerName, List<RegionInfo>> primaryRSToRegionMap;
+  private final Map<ServerName, List<RegionInfo>> primaryRSToRegionMap;
-  private Map<ServerName, List<RegionInfo>> secondaryRSToRegionMap;
+  private final Map<ServerName, List<RegionInfo>> secondaryRSToRegionMap;
-  private Map<ServerName, List<RegionInfo>> teritiaryRSToRegionMap;
+  private final Map<ServerName, List<RegionInfo>> teritiaryRSToRegionMap;
 
-  private MasterServices masterServices;
+  private final MasterServices masterServices;
-  private RackManager rackManager;
+  private final RackManager rackManager;
 
   /**
    * Datanode port to be used for Favored Nodes.
@@ -87,15 +83,19 @@ public class FavoredNodesManager {
     this.rackManager = new RackManager(masterServices.getConfiguration());
   }
 
-  public void initialize(SnapshotOfRegionAssignmentFromMeta snapshotOfRegionAssignment)
+  public synchronized void initialize(SnapshotOfRegionAssignmentFromMeta snapshot) {
-      throws HBaseIOException {
+    // Add snapshot to structures made on creation. Current structures may have picked
-    globalFavoredNodesAssignmentPlan = snapshotOfRegionAssignment.getExistingAssignmentPlan();
+    // up data between construction and the scan of meta needed before this method
-    primaryRSToRegionMap = snapshotOfRegionAssignment.getPrimaryToRegionInfoMap();
+    // is called.  See HBASE-23737 "[Flakey Tests] TestFavoredNodeTableImport fails 30% of the time"
-    secondaryRSToRegionMap = snapshotOfRegionAssignment.getSecondaryToRegionInfoMap();
+    this.globalFavoredNodesAssignmentPlan.
-    teritiaryRSToRegionMap = snapshotOfRegionAssignment.getTertiaryToRegionInfoMap();
+      updateFavoredNodesMap(snapshot.getExistingAssignmentPlan());
-    datanodeDataTransferPort = getDataNodePort();
+    primaryRSToRegionMap.putAll(snapshot.getPrimaryToRegionInfoMap());
+    secondaryRSToRegionMap.putAll(snapshot.getSecondaryToRegionInfoMap());
+    teritiaryRSToRegionMap.putAll(snapshot.getTertiaryToRegionInfoMap());
+    datanodeDataTransferPort= getDataNodePort();
   }
 
+  @VisibleForTesting
   public int getDataNodePort() {
     HdfsConfiguration.init();
 
@@ -122,18 +122,10 @@ public class FavoredNodesManager {
 
   /**
    * Filter and return regions for which favored nodes is not applicable.
-   *
-   * @param regions - collection of regions
    * @return set of regions for which favored nodes is not applicable
    */
   public static Set<RegionInfo> filterNonFNApplicableRegions(Collection<RegionInfo> regions) {
-    Set<RegionInfo> fnRegions = Sets.newHashSet();
+    return regions.stream().filter(r -> !isFavoredNodeApplicable(r)).collect(Collectors.toSet());
-    for (RegionInfo regionInfo : regions) {
-      if (!isFavoredNodeApplicable(regionInfo)) {
-        fnRegions.add(regionInfo);
-      }
-    }
-    return fnRegions;
   }
 
   /*
@@ -194,8 +186,7 @@ public class FavoredNodesManager {
     }
 
     // Lets do a bulk update to meta since that reduces the RPC's
-    FavoredNodeAssignmentHelper.updateMetaWithFavoredNodesInfo(
+    FavoredNodeAssignmentHelper.updateMetaWithFavoredNodesInfo(regionToFavoredNodes,
-        regionToFavoredNodes,
         masterServices.getConnection());
     deleteFavoredNodesForRegions(regionToFavoredNodes.keySet());
 
@@ -208,8 +199,8 @@ public class FavoredNodesManager {
   }
 
   private synchronized void addToReplicaLoad(RegionInfo hri, List<ServerName> servers) {
-    ServerName serverToUse = ServerName.valueOf(servers.get(PRIMARY.ordinal()).getHostAndPort(),
+    ServerName serverToUse =
-        NON_STARTCODE);
+      ServerName.valueOf(servers.get(PRIMARY.ordinal()).getAddress().toString(), NON_STARTCODE);
     List<RegionInfo> regionList = primaryRSToRegionMap.get(serverToUse);
     if (regionList == null) {
       regionList = new ArrayList<>();

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

@@ -1,4 +1,4 @@
-/**
+/*
  *
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -18,10 +18,11 @@
  */
 package org.apache.hadoop.hbase.favored;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
-
+import java.util.stream.Collectors;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -35,9 +36,8 @@ import org.apache.yetus.audience.InterfaceAudience;
  */
 @InterfaceAudience.Private
 public class FavoredNodesPlan {
-
+  /** The map between each region name and its favored region server list */
-  /** the map between each region name and its favored region server list */
+  private final Map<String, List<ServerName>> favoredNodesMap;
-  private Map<String, List<ServerName>> favoredNodesMap;
 
   public static enum Position {
     PRIMARY,
@@ -46,13 +46,18 @@ public class FavoredNodesPlan {
   }
 
   public FavoredNodesPlan() {
-    favoredNodesMap = new ConcurrentHashMap<>();
+    this.favoredNodesMap = new ConcurrentHashMap<>();
+  }
+
+  /**
+   * Add to existing Map of FavoredNodes.
+   */
+  void updateFavoredNodesMap(FavoredNodesPlan fnp) {
+    this.favoredNodesMap.putAll(fnp.favoredNodesMap);
   }
 
   /**
    * Update an assignment to the plan
-   * @param region
-   * @param servers
    */
   public void updateFavoredNodesMap(RegionInfo region, List<ServerName> servers) {
     if (region == null || servers == null || servers.isEmpty()) {
@@ -63,15 +68,13 @@ public class FavoredNodesPlan {
 
   /**
    * Remove a favored node assignment
-   * @param region region
    * @return the list of favored region server for this region based on the plan
    */
-  public List<ServerName> removeFavoredNodes(RegionInfo region) {
+  List<ServerName> removeFavoredNodes(RegionInfo region) {
     return favoredNodesMap.remove(region.getRegionNameAsString());
   }
 
   /**
-   * @param region
    * @return the list of favored region server for this region based on the plan
    */
   public List<ServerName> getFavoredNodes(RegionInfo region) {
@@ -81,8 +84,6 @@ public class FavoredNodesPlan {
   /**
    * Return the position of the server in the favoredNodes list. Assumes the
    * favoredNodes list is of size 3.
-   * @param favoredNodes
-   * @param server
    * @return position
    */
   public static Position getFavoredServerPosition(
@@ -103,7 +104,13 @@ public class FavoredNodesPlan {
    * @return the mapping between each region to its favored region server list
    */
   public Map<String, List<ServerName>> getAssignmentMap() {
-    return favoredNodesMap;
+    // Make a deep copy so changes don't harm our copy of favoredNodesMap.
+    return this.favoredNodesMap.entrySet().stream().
+      collect(Collectors.toMap(k -> k.getKey(), v -> new ArrayList<ServerName>(v.getValue())));
+  }
+
+  public int size() {
+    return this.favoredNodesMap.size();
   }
 
   @Override
@@ -117,12 +124,13 @@ public class FavoredNodesPlan {
     if (getClass() != o.getClass()) {
       return false;
     }
-    // To compare the map from objec o is identical to current assignment map.
+    // To compare the map from object o is identical to current assignment map.
-    Map<String, List<ServerName>> comparedMap = ((FavoredNodesPlan)o).getAssignmentMap();
+    Map<String, List<ServerName>> comparedMap = ((FavoredNodesPlan)o).favoredNodesMap;
 
     // compare the size
-    if (comparedMap.size() != this.favoredNodesMap.size())
+    if (comparedMap.size() != this.favoredNodesMap.size()) {
       return false;
+    }
 
     // compare each element in the assignment map
     for (Map.Entry<String, List<ServerName>> entry :

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

@@ -697,7 +697,7 @@ public class RegionPlacementMaintainer implements Closeable {
             FutureUtils.get(rsAdmin.getServerInfo(RequestConverter.buildGetServerInfoRequest()))
               .getServerInfo() +
             " has updated " + updateFavoredNodesResponse.getResponse() + " / " +
-            singleServerPlan.getAssignmentMap().size() + " regions with the assignment plan");
+            singleServerPlan.size() + " regions with the assignment plan");
           succeededNum++;
         }
       } catch (Exception e) {