HBASE-19589 New regions should always be added with state CLOSED (followup of HBASE-19530)

hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java

@@ -1314,16 +1314,14 @@ public class MetaTableAccessor {
   /**
    * Generates and returns a Put containing the region into for the catalog table
    */
-  public static Put makePutFromRegionInfo(RegionInfo regionInfo)
+  public static Put makePutFromRegionInfo(RegionInfo regionInfo) throws IOException {
-    throws IOException {
     return makePutFromRegionInfo(regionInfo, EnvironmentEdgeManager.currentTime());
   }
 
   /**
    * Generates and returns a Put containing the region into for the catalog table
    */
-  public static Put makePutFromRegionInfo(RegionInfo regionInfo, long ts)
+  public static Put makePutFromRegionInfo(RegionInfo regionInfo, long ts) throws IOException {
-    throws IOException {
     Put put = new Put(regionInfo.getRegionName(), ts);
     addRegionInfo(put, regionInfo);
     return put;
@@ -1448,9 +1446,7 @@ public class MetaTableAccessor {
    */
   private static void put(final Table t, final List<Put> puts) throws IOException {
     try {
-      if (METALOG.isDebugEnabled()) {
+      debugLogMutations(puts);
-        METALOG.debug(mutationsToString(puts));
-      }
       t.put(puts);
     } finally {
       t.close();
@@ -1467,9 +1463,7 @@ public class MetaTableAccessor {
     throws IOException {
     Table t = getMetaHTable(connection);
     try {
-      if (METALOG.isDebugEnabled()) {
+      debugLogMutations(ps);
-        METALOG.debug(mutationsToString(ps));
-      }
       t.put(ps);
     } finally {
       t.close();
@@ -1499,9 +1493,7 @@ public class MetaTableAccessor {
     throws IOException {
     Table t = getMetaHTable(connection);
     try {
-      if (METALOG.isDebugEnabled()) {
+      debugLogMutations(deletes);
-        METALOG.debug(mutationsToString(deletes));
-      }
       t.delete(deletes);
     } finally {
       t.close();
@@ -1546,9 +1538,7 @@ public class MetaTableAccessor {
     throws IOException {
     Table t = getMetaHTable(connection);
     try {
-      if (METALOG.isDebugEnabled()) {
+      debugLogMutations(mutations);
-        METALOG.debug(mutationsToString(mutations));
-      }
       t.batch(mutations, null);
     } catch (InterruptedException e) {
       InterruptedIOException ie = new InterruptedIOException(e.getMessage());
@@ -1559,33 +1549,7 @@ public class MetaTableAccessor {
     }
   }
 
-  /**
+  static void addRegionStateToPut(Put put, RegionState.State  state) throws IOException {
-   * Adds a hbase:meta row for the specified new region.
-   * @param connection connection we're using
-   * @param regionInfo region information
-   * @throws IOException if problem connecting or updating meta
-   */
-  @VisibleForTesting
-  static void addRegionToMeta(Connection connection,
-                                     RegionInfo regionInfo)
-    throws IOException {
-    putToMetaTable(connection, makePutFromRegionInfo(regionInfo));
-    LOG.info("Added " + regionInfo.getRegionNameAsString());
-  }
-
-  /**
-   * Adds a hbase:meta row for the specified new region to the given catalog table. The
-   * Table is not flushed or closed.
-   * @param meta the Table for META
-   * @param regionInfo region information
-   * @throws IOException if problem connecting or updating meta
-   */
-  public static void addRegionToMeta(Table meta, RegionInfo regionInfo) throws IOException {
-    addRegionToMeta(meta, regionInfo, null, null);
-  }
-
-  static void addRegionStateToPut(Put put, RegionState.State  state)
-      throws IOException {
     put.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY)
         .setRow(put.getRow())
         .setFamily(HConstants.CATALOG_FAMILY)
@@ -1597,79 +1561,71 @@ public class MetaTableAccessor {
   }
 
   /**
-   * Adds a (single) hbase:meta row for the specified new region and its daughters. Note that this
+   * Adds daughter region infos to hbase:meta row for the specified region. Note that this does not
-   * does not add its daughter's as different rows, but adds information about the daughters
+   * add its daughter's as different rows, but adds information about the daughters in the same row
-   * in the same row as the parent. Use
+   * as the parent. Use
-   * {@link #splitRegion(Connection, RegionInfo, RegionInfo, RegionInfo, ServerName,int,boolean)}
-   * if you want to do that.
-   * @param meta the Table for META
-   * @param regionInfo region information
-   * @param splitA first split daughter of the parent regionInfo
-   * @param splitB second split daughter of the parent regionInfo
-   * @throws IOException if problem connecting or updating meta
-   */
-  public static void addRegionToMeta(Table meta, RegionInfo regionInfo,
-                                     RegionInfo splitA, RegionInfo splitB) throws IOException {
-    Put put = makePutFromRegionInfo(regionInfo);
-    addRegionStateToPut(put, RegionState.State.CLOSED);
-    addDaughtersToPut(put, splitA, splitB);
-    meta.put(put);
-    if (METALOG.isDebugEnabled()) {
-      METALOG.debug(mutationToString(put));
-    }
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Added " + regionInfo.getRegionNameAsString());
-    }
-  }
-
-  /**
-   * Adds a (single) hbase:meta row for the specified new region and its daughters. Note that this
-   * does not add its daughter's as different rows, but adds information about the daughters
-   * in the same row as the parent. Use
    * {@link #splitRegion(Connection, RegionInfo, RegionInfo, RegionInfo, ServerName,int,boolean)}
    * if you want to do that.
    * @param connection connection we're using
-   * @param regionInfo region information
+   * @param regionInfo RegionInfo of parent region
    * @param splitA first split daughter of the parent regionInfo
    * @param splitB second split daughter of the parent regionInfo
    * @throws IOException if problem connecting or updating meta
    */
-  public static void addRegionToMeta(Connection connection, RegionInfo regionInfo,
+  public static void addSpiltsToParent(Connection connection, RegionInfo regionInfo,
-                                     RegionInfo splitA, RegionInfo splitB) throws IOException {
+      RegionInfo splitA, RegionInfo splitB) throws IOException {
     Table meta = getMetaHTable(connection);
     try {
-      addRegionToMeta(meta, regionInfo, splitA, splitB);
+      Put put = makePutFromRegionInfo(regionInfo);
+      addDaughtersToPut(put, splitA, splitB);
+      meta.put(put);
+      debugLogMutation(put);
+      LOG.debug("Added region {}", regionInfo.getRegionNameAsString());
     } finally {
       meta.close();
     }
   }
 
   /**
-   * Adds a hbase:meta row for each of the specified new regions.
+   * Adds a hbase:meta row for the specified new region. Initial state of new region is CLOSED.
+   * @param connection connection we're using
+   * @param regionInfo region information
+   * @throws IOException if problem connecting or updating meta
+   */
+  @VisibleForTesting
+  public static void addRegionToMeta(Connection connection, RegionInfo regionInfo)
+      throws IOException {
+    addRegionsToMeta(connection, Collections.singletonList(regionInfo), 1);
+  }
+
+  /**
+   * Adds a hbase:meta row for each of the specified new regions. Initial state for new regions
+   * is CLOSED.
    * @param connection connection we're using
    * @param regionInfos region information list
    * @throws IOException if problem connecting or updating meta
    */
-  public static void addRegionsToMeta(Connection connection,
+  public static void addRegionsToMeta(Connection connection, List<RegionInfo> regionInfos,
-                                      List<RegionInfo> regionInfos, int regionReplication)
+      int regionReplication) throws IOException {
-    throws IOException {
     addRegionsToMeta(connection, regionInfos, regionReplication, HConstants.LATEST_TIMESTAMP);
   }
   /**
-   * Adds a hbase:meta row for each of the specified new regions.
+   * Adds a hbase:meta row for each of the specified new regions. Initial state for new regions
+   * is CLOSED.
    * @param connection connection we're using
    * @param regionInfos region information list
    * @param regionReplication
    * @param ts desired timestamp
    * @throws IOException if problem connecting or updating meta
    */
-  public static void addRegionsToMeta(Connection connection,
+  public static void addRegionsToMeta(Connection connection, List<RegionInfo> regionInfos,
-      List<RegionInfo> regionInfos, int regionReplication, long ts)
+      int regionReplication, long ts) throws IOException {
-          throws IOException {
     List<Put> puts = new ArrayList<>();
     for (RegionInfo regionInfo : regionInfos) {
       if (RegionReplicaUtil.isDefaultReplica(regionInfo)) {
         Put put = makePutFromRegionInfo(regionInfo, ts);
+        // New regions are added with initial state of CLOSED.
+        addRegionStateToPut(put, RegionState.State.CLOSED);
         // Add empty locations for region replicas so that number of replicas can be cached
         // whenever the primary region is looked up from meta
         for (int i = 1; i < regionReplication; i++) {
@@ -1679,27 +1635,7 @@ public class MetaTableAccessor {
       }
     }
     putsToMetaTable(connection, puts);
-    LOG.info("Added " + puts.size());
+    LOG.info("Added {} regions to meta.", puts.size());
-  }
-
-  /**
-   * Adds a daughter region entry to meta.
-   * @param regionInfo the region to put
-   * @param sn the location of the region
-   * @param openSeqNum the latest sequence number obtained when the region was open
-   */
-  public static void addDaughter(final Connection connection,
-      final RegionInfo regionInfo, final ServerName sn, final long openSeqNum)
-      throws NotAllMetaRegionsOnlineException, IOException {
-    long now = EnvironmentEdgeManager.currentTime();
-    Put put = new Put(regionInfo.getRegionName(), now);
-    addRegionInfo(put, regionInfo);
-    if (sn != null) {
-      addLocation(put, sn, openSeqNum, -1, regionInfo.getReplicaId());
-    }
-    putToMetaTable(connection, put);
-    LOG.info("Added daughter " + regionInfo.getEncodedName() +
-      (sn == null? ", serverName=null": ", serverName=" + sn.toString()));
   }
 
   /**
@@ -1891,9 +1827,7 @@ public class MetaTableAccessor {
   public static void multiMutate(Connection connection, final Table table, byte[] row,
       final List<Mutation> mutations)
   throws IOException {
-    if (METALOG.isDebugEnabled()) {
+    debugLogMutations(mutations);
-      METALOG.debug(mutationsToString(mutations));
-    }
     // TODO: Need rollback!!!!
     // TODO: Need Retry!!!
     // TODO: What for a timeout? Default write timeout? GET FROM HTABLE?
@@ -2066,44 +2000,12 @@ public class MetaTableAccessor {
     }
     deleteFromMetaTable(connection, deletes);
     LOG.info("Deleted " + regionsInfo.size() + " regions from META");
-    if (LOG.isDebugEnabled()) {
+    LOG.debug("Deleted regions: {}", regionsInfo);
-      LOG.debug("Deleted regions: " + regionsInfo);
-    }
   }
 
   /**
-   * Adds and Removes the specified regions from hbase:meta
+   * Overwrites the specified regions from hbase:meta. Deletes old rows for the given regions and
-   * @param connection connection we're using
+   * adds new ones. Regions added back have state CLOSED.
-   * @param regionsToRemove list of regions to be deleted from META
-   * @param regionsToAdd list of regions to be added to META
-   * @throws IOException
-   */
-  public static void mutateRegions(Connection connection,
-                                   final List<RegionInfo> regionsToRemove,
-                                   final List<RegionInfo> regionsToAdd)
-    throws IOException {
-    List<Mutation> mutation = new ArrayList<>();
-    if (regionsToRemove != null) {
-      for (RegionInfo hri: regionsToRemove) {
-        mutation.add(makeDeleteFromRegionInfo(hri));
-      }
-    }
-    if (regionsToAdd != null) {
-      for (RegionInfo hri: regionsToAdd) {
-        mutation.add(makePutFromRegionInfo(hri));
-      }
-    }
-    mutateMetaTable(connection, mutation);
-    if (regionsToRemove != null && regionsToRemove.size() > 0) {
-      LOG.debug("Deleted " + RegionInfo.getShortNameToLog(regionsToRemove));
-    }
-    if (regionsToAdd != null && regionsToAdd.size() > 0) {
-      LOG.debug("Added " + RegionInfo.getShortNameToLog(regionsToAdd));
-    }
-  }
-
-  /**
-   * Overwrites the specified regions from hbase:meta
    * @param connection connection we're using
    * @param regionInfos list of regions to be added to META
    * @throws IOException
@@ -2121,9 +2023,7 @@ public class MetaTableAccessor {
     // HBASE-13875 uses master timestamp for the mutations. The 20ms sleep is not needed
     addRegionsToMeta(connection, regionInfos, regionReplication, now+1);
     LOG.info("Overwritten " + regionInfos.size() + " regions to Meta");
-    if (LOG.isDebugEnabled()) {
+    LOG.debug("Overwritten regions: {} ", regionInfos);
-      LOG.debug("Overwritten regions: " + regionInfos);
-    }
   }
 
   /**
@@ -2216,18 +2116,19 @@ public class MetaTableAccessor {
                 .build());
   }
 
-  private static String mutationsToString(List<? extends Mutation> mutations) throws IOException {
+  private static void debugLogMutations(List<? extends Mutation> mutations) throws IOException {
-    StringBuilder sb = new StringBuilder();
+    if (!METALOG.isDebugEnabled()) {
-    String prefix = "";
+      return;
-    for (Mutation mutation : mutations) {
+    }
-      sb.append(prefix).append(mutationToString(mutation));
+    // Logging each mutation in separate line makes it easier to see diff between them visually
-      prefix = ", ";
+    // because of common starting indentation.
+    for (Mutation mutation : mutations) {
+      debugLogMutation(mutation);
     }
-    return sb.toString();
   }
 
-  private static String mutationToString(Mutation p) throws IOException {
+  private static void debugLogMutation(Mutation p) throws IOException {
-    return p.getClass().getSimpleName() + p.toJSON();
+    METALOG.debug("{} {}", p.getClass().getSimpleName(), p.toJSON());
   }
 
   public static Put addSequenceNum(final Put p, long openSeqNum, long time,
@@ -2328,16 +2229,10 @@ public class MetaTableAccessor {
     return map;
   }
 
-  /**
+  private static String getSerialReplicationColumnValue(Connection connection,
-   * Get daughter region(s) for a region, only used in serial replication.
+      byte[] encodedRegionName, byte[] columnQualifier) throws IOException {
-   * @param connection connection we're using
+    Get get = new Get(encodedRegionName);
-   * @param encodedName region's encoded name
+    get.addColumn(HConstants.REPLICATION_META_FAMILY, columnQualifier);
-   * @throws IOException
-   */
-  public static String getSerialReplicationDaughterRegion(Connection connection, byte[] encodedName)
-      throws IOException {
-    Get get = new Get(encodedName);
-    get.addColumn(HConstants.REPLICATION_META_FAMILY, daughterNameCq);
     Result result = get(getMetaHTable(connection), get);
     if (!result.isEmpty()) {
       Cell c = result.rawCells()[0];
@@ -2346,39 +2241,33 @@ public class MetaTableAccessor {
     return null;
   }
 
+  /**
+   * Get daughter region(s) for a region, only used in serial replication.
+   * @param connection connection we're using
+   * @param encodedName region's encoded name
+   */
+  public static String getSerialReplicationDaughterRegion(Connection connection, byte[] encodedName)
+      throws IOException {
+    return getSerialReplicationColumnValue(connection, encodedName, daughterNameCq);
+  }
+
   /**
    * Get parent region(s) for a region, only used in serial replication.
    * @param connection connection we're using
    * @param encodedName region's encoded name
-   * @throws IOException
    */
   public static String getSerialReplicationParentRegion(Connection connection, byte[] encodedName)
       throws IOException {
-    Get get = new Get(encodedName);
+    return getSerialReplicationColumnValue(connection, encodedName, parentNameCq);
-    get.addColumn(HConstants.REPLICATION_META_FAMILY, parentNameCq);
-    Result result = get(getMetaHTable(connection), get);
-    if (!result.isEmpty()) {
-      Cell c = result.rawCells()[0];
-      return Bytes.toString(c.getValueArray(), c.getValueOffset(), c.getValueLength());
-    }
-    return null;
   }
 
   /**
    * Get the table name for a region, only used in serial replication.
    * @param connection connection we're using
    * @param encodedName region's encoded name
-   * @throws IOException
    */
   public static String getSerialReplicationTableName(Connection connection, byte[] encodedName)
       throws IOException {
-    Get get = new Get(encodedName);
+    return getSerialReplicationColumnValue(connection, encodedName, tableNameCq);
-    get.addColumn(HConstants.REPLICATION_META_FAMILY, tableNameCq);
-    Result result = get(getMetaHTable(connection), get);
-    if (!result.isEmpty()) {
-      Cell c = result.rawCells()[0];
-      return Bytes.toString(c.getValueArray(), c.getValueOffset(), c.getValueLength());
-    }
-    return null;
   }
 }

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

@@ -461,6 +461,8 @@ public class CloneSnapshotProcedure
   private void addRegionsToMeta(final MasterProcedureEnv env) throws IOException {
     newRegions = CreateTableProcedure.addTableToMeta(env, tableDescriptor, newRegions);
 
+    // TODO: parentsToChildrenPairMap is always empty, which makes updateMetaParentRegions()
+    // a no-op. This part seems unnecessary. Figure out. - Appy 12/21/17
     RestoreSnapshotHelper.RestoreMetaChanges metaChanges =
         new RestoreSnapshotHelper.RestoreMetaChanges(
                 tableDescriptor, parentsToChildrenPairMap);

hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/RestoreSnapshotHelper.java

@@ -393,7 +393,7 @@ public class RestoreSnapshotHelper {
         }
 
         LOG.debug("Update splits parent " + regionInfo.getEncodedName() + " -> " + daughters);
-        MetaTableAccessor.addRegionToMeta(connection, regionInfo,
+        MetaTableAccessor.addSpiltsToParent(connection, regionInfo,
           regionsByName.get(daughters.getFirst()),
           regionsByName.get(daughters.getSecond()));
       }

hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java

@@ -2293,7 +2293,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility {
           .setStartKey(startKeys[i])
           .setEndKey(startKeys[j])
           .build();
-      MetaTableAccessor.addRegionToMeta(meta, hri);
+      MetaTableAccessor.addRegionToMeta(getConnection(), hri);
       newRegions.add(hri);
     }
 

hbase-server/src/test/java/org/apache/hadoop/hbase/util/BaseTestHBaseFsck.java

@@ -113,23 +113,6 @@ public class BaseTestHBaseFsck {
     Bytes.toBytes("00"), Bytes.toBytes("50"), Bytes.toBytes("A0"), Bytes.toBytes("A5"),
     Bytes.toBytes("B0"), Bytes.toBytes("B5"), Bytes.toBytes("C0"), Bytes.toBytes("C5") };
 
-
-  /**
-   * Create a new region in META.
-   */
-  protected RegionInfo createRegion(final HTableDescriptor
-      htd, byte[] startKey, byte[] endKey)
-      throws IOException {
-    Table meta = connection.getTable(TableName.META_TABLE_NAME, tableExecutorService);
-    RegionInfo hri = RegionInfoBuilder.newBuilder(htd.getTableName())
-        .setStartKey(startKey)
-        .setEndKey(endKey)
-        .build();
-    MetaTableAccessor.addRegionToMeta(meta, hri);
-    meta.close();
-    return hri;
-  }
-
   /**
    * Debugging method to dump the contents of meta.
    */

hbase-server/src/test/java/org/apache/hadoop/hbase/util/hbck/OfflineMetaRebuildTestCore.java

@@ -221,7 +221,7 @@ public class OfflineMetaRebuildTestCore {
     out.close();
 
     // add to meta.
-    MetaTableAccessor.addRegionToMeta(meta, hri);
+    MetaTableAccessor.addRegionToMeta(TEST_UTIL.getConnection(), hri);
     meta.close();
     return hri;
   }