HBASE-23596 HBCKServerCrashProcedure can double assign

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

@@ -19,7 +19,9 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 
 import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.ServerName;
@@ -65,13 +67,13 @@ public HBCKServerCrashProcedure(final MasterProcedureEnv env, final ServerName s
   public HBCKServerCrashProcedure() {}
 
   /**
-   * Adds Regions found by super method any found scanning hbase:meta.
+   * Adds Regions found by super method to any found scanning hbase:meta.
    */
   @Override
   @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="NP_NULL_ON_SOME_PATH_EXCEPTION",
     justification="FindBugs seems confused on ps in below.")
   List<RegionInfo> getRegionsOnCrashedServer(MasterProcedureEnv env) {
-    // Super can return immutable emptyList.
+    // Super will return an immutable list (empty if nothing on this server).
     List<RegionInfo> ris = super.getRegionsOnCrashedServer(env);
     List<Pair<RegionInfo, ServerName>> ps = null;
     try {
@@ -84,12 +86,13 @@ List<RegionInfo> getRegionsOnCrashedServer(MasterProcedureEnv env) {
       LOG.warn("No regions found in hbase:meta");
       return ris;
     }
-    List<RegionInfo> aggregate = ris == null || ris.isEmpty()?
-        new ArrayList<>(): new ArrayList<>(ris);
+    // Use a Set here in case same Region in master memory and in hbase:meta (the usual case!).
+    // Otherwise, we will double-assign.
+    Set<RegionInfo> aggregate = new HashSet<>(ris);
     int before = aggregate.size();
     ps.stream().filter(p -> p.getSecond() != null && p.getSecond().equals(getServerName())).
         forEach(p -> aggregate.add(p.getFirst()));
     LOG.info("Found {} mentions of {} in hbase:meta", aggregate.size() - before, getServerName());
-    return aggregate;
+    return new ArrayList<RegionInfo>(aggregate);
   }
 }