From 2366b24ecadfee6cdb9a338dd3d36cd04cc490fb Mon Sep 17 00:00:00 2001 From: stack Date: Thu, 1 Aug 2019 13:53:53 -0700 Subject: [PATCH] HBASE-22777 Add a multi-region merge (for fixing overlaps) Makes MergeTableRegionsProcedure do more than just two regions at a time. Compatible as MTRP was done considering one day it'd do more than two at a time. Changes hardcoded assumption that merge parent regions are named mergeA and mergeB in a column on the resultant region. Instead can have N columns on the merged region, one for each parent merged. Column qualifiers all being with 'merge'. Most of code below is undoing the assumption that there are two parents on a merge only. --- .../hadoop/hbase/MetaTableAccessor.java | 173 +++++++--- .../hadoop/hbase/client/RegionInfo.java | 13 +- .../org/apache/hadoop/hbase/HConstants.java | 14 +- .../apache/hadoop/hbase/PrivateCellUtil.java | 19 ++ .../src/main/protobuf/MasterProcedure.proto | 17 +- .../hadoop/hbase/master/CatalogJanitor.java | 85 ++--- .../assignment/GCMergedRegionsProcedure.java | 3 +- .../GCMultipleMergedRegionsProcedure.java | 171 ++++++++++ .../MergeTableRegionsProcedure.java | 322 +++++++----------- .../hbase/regionserver/HRegionFileSystem.java | 12 +- .../apache/hadoop/hbase/util/HBaseFsck.java | 7 +- .../resources/hbase-webapps/master/hbck.jsp | 1 + .../hadoop/hbase/TestMetaTableAccessor.java | 34 +- .../TestMergeTableRegionsProcedure.java | 53 +++ .../TestRegionMergeTransactionOnCluster.java | 20 +- 15 files changed, 612 insertions(+), 332 deletions(-) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java index 147dddcea1..802f4dae73 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java @@ -59,6 +59,8 @@ import org.apache.hadoop.hbase.client.coprocessor.Batch; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.filter.Filter; import org.apache.hadoop.hbase.filter.FirstKeyOnlyFilter; +import org.apache.hadoop.hbase.filter.QualifierFilter; +import org.apache.hadoop.hbase.filter.RegexStringComparator; import org.apache.hadoop.hbase.filter.RowFilter; import org.apache.hadoop.hbase.filter.SubstringComparator; import org.apache.hadoop.hbase.ipc.CoprocessorRpcUtils; @@ -396,19 +398,101 @@ public class MetaTableAccessor { /** * Get regions from the merge qualifier of the specified merged region * @return null if it doesn't contain merge qualifier, else two merge regions + * @deprecated Since 2.2.1 and 2.3.0. Use {@link #getMergeRegions(Cell[])} */ @Nullable + @Deprecated public static Pair getRegionsFromMergeQualifier( Connection connection, byte[] regionName) throws IOException { - Result result = getRegionResult(connection, regionName); - RegionInfo mergeA = getRegionInfo(result, HConstants.MERGEA_QUALIFIER); - RegionInfo mergeB = getRegionInfo(result, HConstants.MERGEB_QUALIFIER); - if (mergeA == null && mergeB == null) { + List ris = getMergeRegions(connection, regionName); + if (ris == null) { return null; } - return new Pair<>(mergeA, mergeB); + // This method was built to presume merge only did two regions. + if (ris.size() != 2) { + throw new IOException("Wrong count of regions: " + ris.size()); + } + return new Pair(ris.get(0), ris.get(1)); } + /** + * @return Return all regions listed in the 'info:merge*' columns of regionName. + */ + @Nullable + public static List getMergeRegions(Connection connection, byte[] regionName) + throws IOException { + return getMergeRegions(getMergeRegionsRaw(connection, regionName)); + } + + /** + * @return Return all regions listed in the 'info:merge*' columns of regionName. + */ + @Nullable + public static List getMergeRegions(Cell [] cells) { + if (cells == null) { + return null; + } + List regionsToMerge = null; + for (Cell cell: cells) { + if (!isMergeQualifierPrefix(cell)) { + continue; + } + // Ok. This cell is that of a info:merge* column. + RegionInfo ri = RegionInfo.parseFromOrNull(cell.getValueArray(), cell.getValueOffset(), + cell.getValueLength()); + if (ri != null) { + if (regionsToMerge == null) { + regionsToMerge = new ArrayList(); + } + regionsToMerge.add(ri); + } + } + return regionsToMerge; + } + + /** + * @return True if any merge regions present in cells + */ + public static boolean hasMergeRegions(Cell [] cells) { + for (Cell cell: cells) { + if (!isMergeQualifierPrefix(cell)) { + continue; + } + return true; + } + return false; + } + + static boolean isMergeQualifierPrefix(Cell cell) { + // Check to see if has family and that qualifier starts with the merge qualifier 'merge' + return CellUtil.matchingFamily(cell, HConstants.CATALOG_FAMILY) && + PrivateCellUtil.qualifierStartsWith(cell, HConstants.MERGE_QUALIFIER_PREFIX); + } + + /** + * @return Array of Cells from the 'info:merge*' column of row regionName + */ + @Nullable + public static Cell [] getMergeRegionsRaw(Connection connection, byte [] regionName) + throws IOException { + Scan scan = new Scan().withStartRow(regionName). + setOneRowLimit(). + readVersions(1). + addFamily(HConstants.CATALOG_FAMILY). + setFilter(new QualifierFilter(CompareOperator.EQUAL, + new RegexStringComparator(HConstants.MERGE_QUALIFIER_PREFIX_STR+ ".*"))); + try (Table m = getMetaHTable(connection); ResultScanner scanner = m.getScanner(scan)) { + // Should be only one result in this scanner if any. + Result result = scanner.next(); + if (result == null) { + return null; + } + // Should be safe to just return all Cells found since we had filter in place. + // All values should be RegionInfos or something wrong. + return result.rawCells(); + } + } + /** * Checks if the specified table exists. Looks at the hbase:meta table hosted on * the specified server. @@ -420,8 +504,8 @@ public class MetaTableAccessor { final TableName tableName) throws IOException { // Catalog tables always exist. - return tableName.equals(TableName.META_TABLE_NAME) - || getTableState(connection, tableName) != null; + return tableName.equals(TableName.META_TABLE_NAME) || + getTableState(connection, tableName) != null; } /** @@ -1091,26 +1175,10 @@ public class MetaTableAccessor { return new PairOfSameType<>(splitA, splitB); } - /** - * Returns the merge regions by reading the corresponding columns of the catalog table - * Result. - * @param data a Result object from the catalog table scan - * @return a pair of RegionInfo or PairOfSameType(null, null) if the region is not a split - * parent - */ - public static PairOfSameType getMergeRegions(Result data) { - RegionInfo mergeA = getRegionInfo(data, HConstants.MERGEA_QUALIFIER); - RegionInfo mergeB = getRegionInfo(data, HConstants.MERGEB_QUALIFIER); - - return new PairOfSameType<>(mergeA, mergeB); - } - /** * Fetch table state for given table from META table * @param conn connection to use * @param tableName table to fetch state for - * @return state - * @throws IOException */ @Nullable public static TableState getTableState(Connection conn, TableName tableName) @@ -1576,6 +1644,29 @@ public class MetaTableAccessor { LOG.info("Added {} regions to meta.", puts.size()); } + public static Put addMergeRegions(Put put, List mergeRegions) throws IOException { + int limit = 10000; // Arbitrary limit. We don't have room in our formatted String below for more. + int max = mergeRegions.size(); + if (max > limit) { + // Should never happen!!!!! But just in case. + throw new RuntimeException("Can't merge " + max + " regions in one go; " + limit + + " is upper-limit."); + } + int counter = 0; + for (RegionInfo ri: mergeRegions) { + String qualifier = String.format(HConstants.MERGE_QUALIFIER_PREFIX_STR + "%04d", counter++); + put.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY). + setRow(put.getRow()). + setFamily(HConstants.CATALOG_FAMILY). + setQualifier(Bytes.toBytes(qualifier)). + setTimestamp(put.getTimestamp()). + setType(Type.Put). + setValue(RegionInfo.toByteArray(ri)). + build()); + }; + return put; + } + /** * Merge the two regions into one in an atomic operation. Deletes the two merging regions in * hbase:meta and adds the merged region with the information of two merging regions. @@ -1608,25 +1699,9 @@ public class MetaTableAccessor { mutations.add(makePutForReplicationBarrier(regionB, regionBOpenSeqNum, time)); replicationParents.add(regionB); } - // Put for parent Put putOfMerged = makePutFromRegionInfo(mergedRegion, time); - putOfMerged.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY) - .setRow(putOfMerged.getRow()) - .setFamily(HConstants.CATALOG_FAMILY) - .setQualifier(HConstants.MERGEA_QUALIFIER) - .setTimestamp(putOfMerged.getTimestamp()) - .setType(Type.Put) - .setValue(RegionInfo.toByteArray(regionA)) - .build()) - .add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY) - .setRow(putOfMerged.getRow()) - .setFamily(HConstants.CATALOG_FAMILY) - .setQualifier(HConstants.MERGEB_QUALIFIER) - .setTimestamp(putOfMerged.getTimestamp()) - .setType(Type.Put) - .setValue(RegionInfo.toByteArray(regionB)) - .build()); + putOfMerged = addMergeRegions(putOfMerged, Arrays.asList(regionA, regionB)); // Set initial state to CLOSED // NOTE: If initial state is not set to CLOSED then merged region gets added with the // default OFFLINE state. If Master gets restarted after this step, start up sequence of @@ -1908,13 +1983,19 @@ public class MetaTableAccessor { throws IOException { long time = EnvironmentEdgeManager.currentTime(); Delete delete = new Delete(mergedRegion.getRegionName()); - delete.addColumns(getCatalogFamily(), HConstants.MERGEA_QUALIFIER, time); - delete.addColumns(getCatalogFamily(), HConstants.MERGEB_QUALIFIER, time); + // NOTE: We are doing a new hbase:meta read here. + Cell [] cells = getMergeRegionsRaw(connection, mergedRegion.getRegionName()); + List qualifiers = new ArrayList<>(cells.length); + for (Cell cell: cells) { + byte [] qualifier = CellUtil.cloneQualifier(cell); + qualifiers.add(qualifier); + delete.addColumns(getCatalogFamily(), qualifier, time); + } deleteFromMetaTable(connection, delete); - LOG.info("Deleted references in merged region " - + mergedRegion.getRegionNameAsString() + ", qualifier=" - + Bytes.toStringBinary(HConstants.MERGEA_QUALIFIER) + " and qualifier=" - + Bytes.toStringBinary(HConstants.MERGEB_QUALIFIER)); + LOG.info("Deleted merge references in " + mergedRegion.getRegionNameAsString() + + ", deleted qualifiers " + + qualifiers.stream().map(s -> Bytes.toStringBinary(s)). + collect(Collectors.joining(", "))); } public static Put addRegionInfo(final Put p, final RegionInfo hri) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java index e8afeada69..24117eec5f 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java @@ -794,12 +794,23 @@ public interface RegionInfo { } /** - * @return True if regions are adjacent, if 'after' next. Does not do tablename compare. + * @return True if region is next, adjacent but 'after' this one. Does not do tablename compare. + * @see #isAdjacent(RegionInfo) + * @see #areAdjacent(RegionInfo, RegionInfo) */ default boolean isNext(RegionInfo after) { return Bytes.equals(getEndKey(), after.getStartKey()); } + /** + * @return True if region is adjacent, either just before or just after this one. + * Does not do tablename compare. + * @see #isNext(RegionInfo) + */ + default boolean isAdjacent(RegionInfo other) { + return areAdjacent(this, other); + } + /** * @return True if RegionInfo is degenerate... if startKey > endKey. */ diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java index dbfbbb379d..a8eacc1aca 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java @@ -485,11 +485,15 @@ public final class HConstants { /** The upper-half split region column qualifier */ public static final byte [] SPLITB_QUALIFIER = Bytes.toBytes("splitB"); - /** The lower-half merge region column qualifier */ - public static final byte[] MERGEA_QUALIFIER = Bytes.toBytes("mergeA"); - - /** The upper-half merge region column qualifier */ - public static final byte[] MERGEB_QUALIFIER = Bytes.toBytes("mergeB"); + /** + * Merge qualifier prefix. + * We used to only allow two regions merge; mergeA and mergeB. + * Now we allow many to merge. Each region to merge will be referenced + * in a column whose qualifier starts with this define. + */ + public static final String MERGE_QUALIFIER_PREFIX_STR = "merge"; + public static final byte [] MERGE_QUALIFIER_PREFIX = + Bytes.toBytes(MERGE_QUALIFIER_PREFIX_STR); /** The catalog family as a string*/ public static final String TABLE_FAMILY_STR = "table"; diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/PrivateCellUtil.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/PrivateCellUtil.java index 8d021c1984..e95216a0d4 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/PrivateCellUtil.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/PrivateCellUtil.java @@ -757,6 +757,25 @@ public final class PrivateCellUtil { left.getQualifierLength(), buf, offset, length); } + /** + * Finds if the start of the qualifier part of the Cell matches buf + * @param left the cell with which we need to match the qualifier + * @param startsWith the serialized keyvalue format byte[] + * @return true if the qualifier have same staring characters, false otherwise + */ + public static boolean qualifierStartsWith(final Cell left, final byte[] startsWith) { + if (startsWith == null || startsWith.length == 0) { + return false; + } + if (left instanceof ByteBufferExtendedCell) { + return ByteBufferUtils.equals(((ByteBufferExtendedCell) left).getQualifierByteBuffer(), + ((ByteBufferExtendedCell) left).getQualifierPosition(), startsWith.length, + startsWith, 0, startsWith.length); + } + return Bytes.equals(left.getQualifierArray(), left.getQualifierOffset(), + startsWith.length, startsWith, 0, startsWith.length); + } + public static boolean matchingColumn(final Cell left, final byte[] fam, final int foffset, final int flength, final byte[] qual, final int qoffset, final int qlength) { if (!matchingFamily(left, fam, foffset, flength)) { diff --git a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto index d5a390ca44..fbfa424f65 100644 --- a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto @@ -376,17 +376,32 @@ message GCRegionStateData { } enum GCMergedRegionsState { + // Use GCMultipleMergedRegionsState instead. + option deprecated = true; GC_MERGED_REGIONS_PREPARE = 1; GC_MERGED_REGIONS_PURGE = 2; GC_REGION_EDIT_METADATA = 3; } message GCMergedRegionsStateData { + // Use GCMultipleMergedRegionsStateData instead. + option deprecated = true; required RegionInfo parent_a = 1; required RegionInfo parent_b = 2; required RegionInfo merged_child = 3; } +enum GCMultipleMergedRegionsState { + GC_MULTIPLE_MERGED_REGIONS_PREPARE = 1; + GC_MULTIPLE_MERGED_REGIONS_PURGE = 2; + GC_MULTIPLE_REGION_EDIT_METADATA = 3; +} + +message GCMultipleMergedRegionsStateData { + repeated RegionInfo parents = 1; + required RegionInfo merged_child = 2; +} + enum PeerModificationState { PRE_PEER_MODIFICATION = 1; UPDATE_PEER_STORAGE = 2; @@ -607,4 +622,4 @@ enum SplitWALState{ ACQUIRE_SPLIT_WAL_WORKER = 1; DISPATCH_WAL_TO_WORKER = 2; RELEASE_SPLIT_WORKER = 3; -} \ No newline at end of file +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java index e6564a54e6..1fb126044d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java @@ -28,6 +28,7 @@ import java.util.Map; import java.util.Properties; import java.util.TreeMap; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -51,7 +52,7 @@ import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; -import org.apache.hadoop.hbase.master.assignment.GCMergedRegionsProcedure; +import org.apache.hadoop.hbase.master.assignment.GCMultipleMergedRegionsProcedure; import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; @@ -183,26 +184,15 @@ public class CatalogJanitor extends ScheduledChore { break; } - PairOfSameType p = MetaTableAccessor.getMergeRegions(e.getValue()); - RegionInfo regionA = p.getFirst(); - RegionInfo regionB = p.getSecond(); - if (regionA == null || regionB == null) { - LOG.warn("Unexpected references regionA=" - + (regionA == null ? "null" : regionA.getShortNameToLog()) - + ",regionB=" - + (regionB == null ? "null" : regionB.getShortNameToLog()) - + " in merged region " + e.getKey().getShortNameToLog()); - } else { - if (cleanMergeRegion(e.getKey(), regionA, regionB)) { - gcs++; - } + List parents = MetaTableAccessor.getMergeRegions(e.getValue().rawCells()); + if (cleanMergeRegion(e.getKey(), parents)) { + gcs++; } } // Clean split parents Map splitParents = report.splitParents; // Now work on our list of found parents. See if any we can clean up. - // regions whose parents are still around HashSet parentNotCleaned = new HashSet<>(); for (Map.Entry e : splitParents.entrySet()) { if (this.services.isInMaintenanceMode()) { @@ -251,10 +241,10 @@ public class CatalogJanitor extends ScheduledChore { * If merged region no longer holds reference to the merge regions, archive * merge region on hdfs and perform deleting references in hbase:meta * @return true if we delete references in merged region on hbase:meta and archive - * the files on the file system + * the files on the file system */ - private boolean cleanMergeRegion(final RegionInfo mergedRegion, - final RegionInfo regionA, final RegionInfo regionB) throws IOException { + private boolean cleanMergeRegion(final RegionInfo mergedRegion, List parents) + throws IOException { FileSystem fs = this.services.getMasterFileSystem().getFileSystem(); Path rootdir = this.services.getMasterFileSystem().getRootDir(); Path tabledir = FSUtils.getTableDir(rootdir, mergedRegion.getTable()); @@ -267,17 +257,19 @@ public class CatalogJanitor extends ScheduledChore { LOG.warn("Merged region does not exist: " + mergedRegion.getEncodedName()); } if (regionFs == null || !regionFs.hasReferences(htd)) { - LOG.debug("Deleting region " + regionA.getShortNameToLog() + " and " - + regionB.getShortNameToLog() - + " from fs because merged region no longer holds references"); + LOG.debug("Deleting parents ({}) from fs; merged child {} no longer holds references", + parents.stream().map(r -> RegionInfo.getShortNameToLog(r)). + collect(Collectors.joining(", ")), + mergedRegion); ProcedureExecutor pe = this.services.getMasterProcedureExecutor(); - pe.submitProcedure(new GCMergedRegionsProcedure(pe.getEnvironment(), - mergedRegion, regionA, regionB)); - // Remove from in-memory states - this.services.getAssignmentManager().getRegionStates().deleteRegion(regionA); - this.services.getAssignmentManager().getRegionStates().deleteRegion(regionB); - this.services.getServerManager().removeRegion(regionA); - this.services.getServerManager().removeRegion(regionB); + pe.submitProcedure(new GCMultipleMergedRegionsProcedure(pe.getEnvironment(), + mergedRegion, parents)); + for (RegionInfo ri: parents) { + // The above scheduled GCMultipleMergedRegionsProcedure does the below. + // Do we need this? + this.services.getAssignmentManager().getRegionStates().deleteRegion(ri); + this.services.getServerManager().removeRegion(ri); + } return true; } return false; @@ -325,11 +317,9 @@ public class CatalogJanitor extends ScheduledChore { */ boolean cleanParent(final RegionInfo parent, Result rowContent) throws IOException { - // Check whether it is a merged region and not clean reference - // No necessary to check MERGEB_QUALIFIER because these two qualifiers will - // be inserted/deleted together - if (rowContent.getValue(HConstants.CATALOG_FAMILY, HConstants.MERGEA_QUALIFIER) != null) { - // wait cleaning merge region first + // Check whether it is a merged region and if it is clean of references. + if (MetaTableAccessor.hasMergeRegions(rowContent.rawCells())) { + // Wait until clean of merge parent regions first return false; } // Run checks on each daughter split. @@ -422,28 +412,19 @@ public class CatalogJanitor extends ScheduledChore { } /** - * Checks if the specified region has merge qualifiers, if so, try to clean - * them - * @return true if the specified region doesn't have merge qualifier now + * Checks if the specified region has merge qualifiers, if so, try to clean them. + * @return true if no info:merge* columns; i.e. the specified region doesn't have + * any merge qualifiers. */ public boolean cleanMergeQualifier(final RegionInfo region) throws IOException { - // Get merge regions if it is a merged region and already has merge - // qualifier - Pair mergeRegions = MetaTableAccessor - .getRegionsFromMergeQualifier(this.services.getConnection(), - region.getRegionName()); - if (mergeRegions == null - || (mergeRegions.getFirst() == null && mergeRegions.getSecond() == null)) { + // Get merge regions if it is a merged region and already has merge qualifier + List parents = MetaTableAccessor.getMergeRegions(this.services.getConnection(), + region.getRegionName()); + if (parents == null || parents.isEmpty()) { // It doesn't have merge qualifier, no need to clean return true; } - // It shouldn't happen, we must insert/delete these two qualifiers together - if (mergeRegions.getFirst() == null || mergeRegions.getSecond() == null) { - LOG.error("Merged region " + region.getRegionNameAsString() - + " has only one merge qualifier in META."); - return false; - } - return cleanMergeRegion(region, mergeRegions.getFirst(), mergeRegions.getSecond()); + return cleanMergeRegion(region, parents); } /** @@ -613,7 +594,7 @@ public class CatalogJanitor extends ScheduledChore { if (regionInfo.isSplitParent()) { // splitParent means split and offline. this.report.splitParents.put(regionInfo, r); } - if (r.getValue(HConstants.CATALOG_FAMILY, HConstants.MERGEA_QUALIFIER) != null) { + if (MetaTableAccessor.hasMergeRegions(r.rawCells())) { this.report.mergedRegions.put(regionInfo, r); } } @@ -779,7 +760,7 @@ public class CatalogJanitor extends ScheduledChore { try (Table t = connection.getTable(TableName.META_TABLE_NAME)) { Result r = t.get(g); byte [] row = g.getRow(); - row[row.length - 2] <<= ((byte)row[row.length - 2]); + row[row.length - 2] <<= row[row.length - 2]; Put p = new Put(g.getRow()); p.addColumn(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER, r.getValue(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER)); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java index 610003df91..1e1ce1a868 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java @@ -40,8 +40,10 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.G *

This is a Table Procedure. We take a read lock on the Table. * We do NOT keep a lock for the life of this procedure. The subprocedures * take locks on the Regions they are purging. + * @deprecated 2.3.0 Use {@link GCMultipleMergedRegionsProcedure}. */ @InterfaceAudience.Private +@Deprecated public class GCMergedRegionsProcedure extends AbstractStateMachineTableProcedure { private static final Logger LOG = LoggerFactory.getLogger(GCMergedRegionsProcedure.class); @@ -51,7 +53,6 @@ extends AbstractStateMachineTableProcedure { public GCMergedRegionsProcedure(final MasterProcedureEnv env, final RegionInfo mergedChild, - final RegionInfo father, final RegionInfo mother) { super(env); this.father = father; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java new file mode 100644 index 0000000000..973a5a86d9 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMultipleMergedRegionsProcedure.java @@ -0,0 +1,171 @@ +/* + * 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 + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.master.assignment; + +import org.apache.hadoop.hbase.MetaTableAccessor; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.procedure.AbstractStateMachineTableProcedure; +import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; +import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; +import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException; +import org.apache.hadoop.hbase.procedure2.ProcedureYieldException; +import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; +import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.GCMultipleMergedRegionsState; +import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.GCMultipleMergedRegionsStateData; +import org.apache.yetus.audience.InterfaceAudience; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +/** + * GC regions that have been Merged. Caller determines if it is GC time. This Procedure does not + * check.This is a Table Procedure. We take a read lock on the Table. We do NOT keep a lock for + * the life of this procedure. The subprocedures take locks on the Regions they are purging. + * Replaces a Procedure that did two regions only at a time instead doing multiple merges in the + * one go. + */ +@InterfaceAudience.Private +public class GCMultipleMergedRegionsProcedure extends + AbstractStateMachineTableProcedure { + private static final Logger LOG = LoggerFactory.getLogger(GCMultipleMergedRegionsProcedure.class); + private List parents; + private RegionInfo mergedChild; + + public GCMultipleMergedRegionsProcedure(final MasterProcedureEnv env, + final RegionInfo mergedChild, final List parents) { + super(env); + this.parents = parents; + this.mergedChild = mergedChild; + } + + public GCMultipleMergedRegionsProcedure() { + // Required by the Procedure framework to create the procedure on replay + super(); + } + + @Override + public TableOperationType getTableOperationType() { + return TableOperationType.MERGED_REGIONS_GC; + } + + @Override + protected Flow executeFromState(MasterProcedureEnv env, GCMultipleMergedRegionsState state) + throws ProcedureSuspendedException, ProcedureYieldException, InterruptedException { + if (LOG.isTraceEnabled()) { + LOG.trace(this + " execute state=" + state); + } + try { + switch (state) { + case GC_MULTIPLE_MERGED_REGIONS_PREPARE: + // Nothing to do to prepare. + setNextState(GCMultipleMergedRegionsState.GC_MULTIPLE_MERGED_REGIONS_PURGE); + break; + case GC_MULTIPLE_MERGED_REGIONS_PURGE: + addChildProcedure(createGCRegionProcedures(env)); + setNextState(GCMultipleMergedRegionsState.GC_MULTIPLE_REGION_EDIT_METADATA); + break; + case GC_MULTIPLE_REGION_EDIT_METADATA: + MetaTableAccessor.deleteMergeQualifiers(env.getMasterServices().getConnection(), mergedChild); + return Flow.NO_MORE_STATE; + default: + throw new UnsupportedOperationException(this + " unhandled state=" + state); + } + } catch (IOException ioe) { + // TODO: This is going to spew log? + LOG.warn("Error trying to GC merged regions {}; retrying...", + this.parents.stream().map(r -> RegionInfo.getShortNameToLog(r)). + collect(Collectors.joining(", ")), + ioe); + } + return Flow.HAS_MORE_STATE; + } + + private GCRegionProcedure[] createGCRegionProcedures(final MasterProcedureEnv env) { + GCRegionProcedure [] procs = new GCRegionProcedure[2]; + int index = 0; + for (RegionInfo ri: this.parents) { + GCRegionProcedure proc = new GCRegionProcedure(env, ri); + proc.setOwner(env.getRequestUser().getShortName()); + procs[index++] = proc; + } + return procs; + } + + @Override + protected void rollbackState(MasterProcedureEnv env, GCMultipleMergedRegionsState state) + throws IOException, InterruptedException { + // no-op + } + + @Override + protected GCMultipleMergedRegionsState getState(int stateId) { + return GCMultipleMergedRegionsState.forNumber(stateId); + } + + @Override + protected int getStateId(GCMultipleMergedRegionsState state) { + return state.getNumber(); + } + + @Override + protected GCMultipleMergedRegionsState getInitialState() { + return GCMultipleMergedRegionsState.GC_MULTIPLE_MERGED_REGIONS_PREPARE; + } + + @Override + protected void serializeStateData(ProcedureStateSerializer serializer) + throws IOException { + super.serializeStateData(serializer); + final GCMultipleMergedRegionsStateData.Builder msg = + GCMultipleMergedRegionsStateData.newBuilder(). + addAllParents(this.parents.stream().map(ProtobufUtil::toRegionInfo). + collect(Collectors.toList())). + setMergedChild(ProtobufUtil.toRegionInfo(this.mergedChild)); + serializer.serialize(msg.build()); + } + + @Override + protected void deserializeStateData(ProcedureStateSerializer serializer) + throws IOException { + super.deserializeStateData(serializer); + final GCMultipleMergedRegionsStateData msg = + serializer.deserialize(GCMultipleMergedRegionsStateData.class); + this.parents = msg.getParentsList().stream().map(ProtobufUtil::toRegionInfo). + collect(Collectors.toList()); + this.mergedChild = ProtobufUtil.toRegionInfo(msg.getMergedChild()); + } + + @Override + public void toStringClassDetails(StringBuilder sb) { + sb.append(getClass().getSimpleName()); + sb.append(" child="); + sb.append(this.mergedChild.getShortNameToLog()); + sb.append(", parents:"); + sb.append(this.parents.stream().map(r -> RegionInfo.getShortNameToLog(r)). + collect(Collectors.joining(", "))); + } + + @Override + public TableName getTableName() { + return this.mergedChild.getTable(); + } +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java index 404d89c8df..65fa316d41 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java @@ -27,7 +27,6 @@ import java.util.stream.Stream; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaMutationAnnotation; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -57,7 +56,6 @@ import org.apache.hadoop.hbase.regionserver.HRegionFileSystem; import org.apache.hadoop.hbase.regionserver.HStoreFile; import org.apache.hadoop.hbase.regionserver.StoreFileInfo; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.wal.WALSplitUtil; import org.apache.yetus.audience.InterfaceAudience; @@ -72,12 +70,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.MergeTableRegionsState; /** - * The procedure to Merge a region in a table. - *

- * This procedure takes an exclusive table lock since it is working over multiple regions. - *

- * It holds the lock for the life of the procedure. - *

+ * The procedure to Merge regions in a table. This procedure takes an exclusive table + * lock since it is working over multiple regions. It holds the lock for the life of the procedure. * Throws exception on construction if determines context hostile to merge (cluster going down or * master is shutting down or table is disabled). */ @@ -85,11 +79,10 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.M public class MergeTableRegionsProcedure extends AbstractStateMachineTableProcedure { private static final Logger LOG = LoggerFactory.getLogger(MergeTableRegionsProcedure.class); - private Boolean traceEnabled; private ServerName regionLocation; private RegionInfo[] regionsToMerge; private RegionInfo mergedRegion; - private boolean forcible; + private boolean force; public MergeTableRegionsProcedure() { // Required by the Procedure framework to create the procedure on replay @@ -102,18 +95,18 @@ public class MergeTableRegionsProcedure public MergeTableRegionsProcedure(final MasterProcedureEnv env, final RegionInfo regionToMergeA, final RegionInfo regionToMergeB, - final boolean forcible) throws IOException { - this(env, new RegionInfo[] {regionToMergeA, regionToMergeB}, forcible); + final boolean force) throws IOException { + this(env, new RegionInfo[] {regionToMergeA, regionToMergeB}, force); } public MergeTableRegionsProcedure(final MasterProcedureEnv env, - final RegionInfo[] regionsToMerge, final boolean forcible) + final RegionInfo[] regionsToMerge, final boolean force) throws IOException { super(env); - // Check daughter regions and make sure that we have valid daughter regions - // before doing the real work. This check calls the super method #checkOnline also. - checkRegionsToMerge(env, regionsToMerge, forcible); + // Check parent regions. Make sure valid before doing real work. + // This check calls the super method #checkOnline also. + checkRegionsToMerge(env, regionsToMerge, force); // WARN: make sure there is no parent region of the two merging regions in // hbase:meta If exists, fixing up daughters would cause daughter regions(we @@ -123,112 +116,83 @@ public class MergeTableRegionsProcedure this.regionsToMerge = regionsToMerge; this.mergedRegion = createMergedRegionInfo(regionsToMerge); preflightChecks(env, true); - this.forcible = forcible; - } - - private static void checkRegionsToMerge(MasterProcedureEnv env, final RegionInfo[] regionsToMerge, - final boolean forcible) throws MergeRegionException { - // For now, we only merge 2 regions. - // It could be extended to more than 2 regions in the future. - if (regionsToMerge == null || regionsToMerge.length != 2) { - throw new MergeRegionException("Expected to merge 2 regions, got: " + - Arrays.toString(regionsToMerge)); - } - - checkRegionsToMerge(env, regionsToMerge[0], regionsToMerge[1], forcible); + this.force = force; } /** * One time checks. */ - private static void checkRegionsToMerge(MasterProcedureEnv env, final RegionInfo regionToMergeA, - final RegionInfo regionToMergeB, final boolean forcible) throws MergeRegionException { - if (!regionToMergeA.getTable().equals(regionToMergeB.getTable())) { - throw new MergeRegionException("Can't merge regions from two different tables: " + - regionToMergeA + ", " + regionToMergeB); - } - - if (regionToMergeA.getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID || - regionToMergeB.getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID) { - throw new MergeRegionException("Can't merge non-default replicas"); - } - - try { - checkOnline(env, regionToMergeA); - checkOnline(env, regionToMergeB); - } catch (DoNotRetryRegionException dnrre) { - throw new MergeRegionException(dnrre); - } + private static void checkRegionsToMerge(MasterProcedureEnv env, final RegionInfo[] regions, + final boolean force) throws MergeRegionException { + if (regions == null) { + throw new MergeRegionException("No Regions specified."); + } + RegionInfo previous = null; + for (RegionInfo ri: regions) { + if (previous != null) { + if (!previous.getTable().equals(ri.getTable())) { + throw new MergeRegionException("Can't merge regions from different tables: " + + previous + ", " + ri); + } + if (!force && !ri.isAdjacent(previous) && !ri.isOverlap(previous)) { + String msg = "Unable to merge non-adjacent or non-overlapping regions " + + previous.getShortNameToLog() + ", " + ri.getShortNameToLog() + " when force=" + force; + LOG.warn(msg); + throw new MergeRegionException(msg); + } + } + previous = ri; - if (!RegionInfo.areAdjacent(regionToMergeA, regionToMergeB)) { - String msg = "Unable to merge non-adjacent regions " + regionToMergeA.getShortNameToLog() + - ", " + regionToMergeB.getShortNameToLog() + " where forcible = " + forcible; - LOG.warn(msg); - if (!forcible) { - throw new MergeRegionException(msg); + if (ri.getReplicaId() != RegionInfo.DEFAULT_REPLICA_ID) { + throw new MergeRegionException("Can't merge non-default replicas; " + ri); + } + try { + checkOnline(env, ri); + } catch (DoNotRetryRegionException dnrre) { + throw new MergeRegionException(dnrre); } } } - - private static RegionInfo createMergedRegionInfo(final RegionInfo[] regionsToMerge) { - return createMergedRegionInfo(regionsToMerge[0], regionsToMerge[1]); - } - /** * Create merged region info through the specified two regions */ - private static RegionInfo createMergedRegionInfo(final RegionInfo regionToMergeA, - final RegionInfo regionToMergeB) { - // Choose the smaller as start key - final byte[] startKey; - if (RegionInfo.COMPARATOR.compare(regionToMergeA, regionToMergeB) <= 0) { - startKey = regionToMergeA.getStartKey(); - } else { - startKey = regionToMergeB.getStartKey(); - } - - // Choose the bigger as end key - final byte[] endKey; - if (Bytes.equals(regionToMergeA.getEndKey(), HConstants.EMPTY_BYTE_ARRAY) - || (!Bytes.equals(regionToMergeB.getEndKey(), HConstants.EMPTY_BYTE_ARRAY) - && Bytes.compareTo(regionToMergeA.getEndKey(), regionToMergeB.getEndKey()) > 0)) { - endKey = regionToMergeA.getEndKey(); - } else { - endKey = regionToMergeB.getEndKey(); - } - - // Merged region is sorted between two merging regions in META - return RegionInfoBuilder.newBuilder(regionToMergeA.getTable()) - .setStartKey(startKey) - .setEndKey(endKey) - .setSplit(false) - .setRegionId(getMergedRegionIdTimestamp(regionToMergeA, regionToMergeB)) - .build(); - } - - private static long getMergedRegionIdTimestamp(final RegionInfo regionToMergeA, - final RegionInfo regionToMergeB) { - long rid = EnvironmentEdgeManager.currentTime(); + private static RegionInfo createMergedRegionInfo(final RegionInfo[] regionsToMerge) { + byte [] lowestStartKey = null; + byte [] highestEndKey = null; // Region Id is a timestamp. Merged region's id can't be less than that of // merging regions else will insert at wrong location in hbase:meta (See HBASE-710). - if (rid < regionToMergeA.getRegionId() || rid < regionToMergeB.getRegionId()) { - LOG.warn("Clock skew; merging regions id are " + regionToMergeA.getRegionId() - + " and " + regionToMergeB.getRegionId() + ", but current time here is " + rid); - rid = Math.max(regionToMergeA.getRegionId(), regionToMergeB.getRegionId()) + 1; + long highestRegionId = -1; + for (RegionInfo ri: regionsToMerge) { + if (lowestStartKey == null) { + lowestStartKey = ri.getStartKey(); + } else if (Bytes.compareTo(ri.getStartKey(), lowestStartKey) < 0) { + lowestStartKey = ri.getStartKey(); + } + if (highestEndKey == null) { + highestEndKey = ri.getEndKey(); + } else if (ri.isLast() || Bytes.compareTo(ri.getEndKey(), lowestStartKey) > 0) { + highestEndKey = ri.getEndKey(); + } + highestRegionId = ri.getRegionId() > highestRegionId? ri.getRegionId(): highestRegionId; } - return rid; + // Merged region is sorted between two merging regions in META + return RegionInfoBuilder.newBuilder(regionsToMerge[0].getTable()). + setStartKey(lowestStartKey). + setEndKey(highestEndKey). + setSplit(false). + setRegionId(highestRegionId + 1/*Add one so new merged region is highest*/). + build(); } - private void removeNonDefaultReplicas(MasterProcedureEnv env) throws IOException { AssignmentManagerUtil.removeNonDefaultReplicas(env, Stream.of(regionsToMerge), - getRegionReplication(env)); + getRegionReplication(env)); } private void checkClosedRegions(MasterProcedureEnv env) throws IOException { - // theoretically this should not happen any more after we use TRSP, but anyway let's add a check - // here + // Theoretically this should not happen any more after we use TRSP, but anyway + // let's add a check here for (RegionInfo region : regionsToMerge) { AssignmentManagerUtil.checkClosedRegion(env, region); } @@ -314,9 +278,7 @@ public class MergeTableRegionsProcedure @Override protected void rollbackState(final MasterProcedureEnv env, final MergeTableRegionsState state) throws IOException { - if (isTraceEnabled()) { - LOG.trace(this + " rollback state=" + state); - } + LOG.trace("{} rollback state={}", this, state); try { switch (state) { @@ -400,9 +362,9 @@ public class MergeTableRegionsProcedure MasterProcedureProtos.MergeTableRegionsStateData.newBuilder() .setUserInfo(MasterProcedureUtil.toProtoUserInfo(getUser())) .setMergedRegionInfo(ProtobufUtil.toRegionInfo(mergedRegion)) - .setForcible(forcible); - for (int i = 0; i < regionsToMerge.length; ++i) { - mergeTableRegionsMsg.addRegionInfo(ProtobufUtil.toRegionInfo(regionsToMerge[i])); + .setForcible(force); + for (RegionInfo ri: regionsToMerge) { + mergeTableRegionsMsg.addRegionInfo(ProtobufUtil.toRegionInfo(ri)); } serializer.serialize(mergeTableRegionsMsg.build()); } @@ -432,8 +394,8 @@ public class MergeTableRegionsProcedure sb.append(getTableName()); sb.append(", regions="); sb.append(RegionInfo.getShortNameToLog(regionsToMerge)); - sb.append(", forcibly="); - sb.append(forcible); + sb.append(", force="); + sb.append(force); } @Override @@ -481,43 +443,14 @@ public class MergeTableRegionsProcedure */ private boolean prepareMergeRegion(final MasterProcedureEnv env) throws IOException { // Fail if we are taking snapshot for the given table - if (env.getMasterServices().getSnapshotManager() - .isTakingSnapshot(regionsToMerge[0].getTable())) { - throw new MergeRegionException( - "Skip merging regions " + RegionInfo.getShortNameToLog(regionsToMerge) + - ", because we are taking snapshot for the table " + regionsToMerge[0].getTable()); - } - // Note: the following logic assumes that we only have 2 regions to merge. In the future, - // if we want to extend to more than 2 regions, the code needs to be modified a little bit. - CatalogJanitor catalogJanitor = env.getMasterServices().getCatalogJanitor(); - boolean regionAHasMergeQualifier = !catalogJanitor.cleanMergeQualifier(regionsToMerge[0]); - if (regionAHasMergeQualifier - || !catalogJanitor.cleanMergeQualifier(regionsToMerge[1])) { - String msg = "Skip merging regions " + RegionInfo.getShortNameToLog(regionsToMerge) + - ", because region " - + (regionAHasMergeQualifier ? regionsToMerge[0].getEncodedName() : regionsToMerge[1] - .getEncodedName()) + " has merge qualifier"; - LOG.warn(msg); - throw new MergeRegionException(msg); - } - - RegionStates regionStates = env.getAssignmentManager().getRegionStates(); - RegionState regionStateA = regionStates.getRegionState(regionsToMerge[0].getEncodedName()); - RegionState regionStateB = regionStates.getRegionState(regionsToMerge[1].getEncodedName()); - if (regionStateA == null || regionStateB == null) { - throw new UnknownRegionException( - regionStateA == null ? - regionsToMerge[0].getEncodedName() : regionsToMerge[1].getEncodedName()); + TableName tn = regionsToMerge[0].getTable(); + if (env.getMasterServices().getSnapshotManager().isTakingSnapshot(tn)) { + throw new MergeRegionException("Skip merging regions " + + RegionInfo.getShortNameToLog(regionsToMerge) + ", because we are snapshotting " + tn); } - - if (!regionStateA.isOpened() || !regionStateB.isOpened()) { - throw new MergeRegionException( - "Unable to merge regions that are not online " + regionStateA + ", " + regionStateB); - } - if (!env.getMasterServices().isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { - String regionsStr = Arrays.deepToString(regionsToMerge); - LOG.warn("merge switch is off! skip merge of " + regionsStr); + String regionsStr = Arrays.deepToString(this.regionsToMerge); + LOG.warn("Merge switch is off! skip merge of " + regionsStr); super.setFailure(getClass().getSimpleName(), new IOException("Merge of " + regionsStr + " failed because merge switch is off")); return false; @@ -531,30 +464,34 @@ public class MergeTableRegionsProcedure return false; } - // Ask the remote regionserver if regions are mergeable. If we get an IOE, report it - // along with the failure, so we can see why regions are not mergeable at this time. - IOException mergeableCheckIOE = null; - boolean mergeable = false; - RegionState current = regionStateA; - try { - mergeable = isMergeable(env, current); - } catch (IOException e) { - mergeableCheckIOE = e; - } - if (mergeable && mergeableCheckIOE == null) { - current = regionStateB; + CatalogJanitor catalogJanitor = env.getMasterServices().getCatalogJanitor(); + RegionStates regionStates = env.getAssignmentManager().getRegionStates(); + for (RegionInfo ri: this.regionsToMerge) { + if (!catalogJanitor.cleanMergeQualifier(ri)) { + String msg = "Skip merging " + RegionInfo.getShortNameToLog(regionsToMerge) + + ", because parent " + RegionInfo.getShortNameToLog(ri) + " has a merge qualifier"; + LOG.warn(msg); + throw new MergeRegionException(msg); + } + RegionState state = regionStates.getRegionState(ri.getEncodedName()); + if (state == null) { + throw new UnknownRegionException("No state for " + RegionInfo.getShortNameToLog(ri)); + } + if (!state.isOpened()) { + throw new MergeRegionException("Unable to merge regions that are not online: " + ri); + } + // Ask the remote regionserver if regions are mergeable. If we get an IOE, report it + // along with the failure, so we can see why regions are not mergeable at this time. try { - mergeable = isMergeable(env, current); + if (!isMergeable(env, state)) { + return false; + } } catch (IOException e) { - mergeableCheckIOE = e; + IOException ioe = new IOException(RegionInfo.getShortNameToLog(ri) + " NOT mergeable", e); + super.setFailure(getClass().getSimpleName(), ioe); + return false; } } - if (!mergeable) { - IOException e = new IOException(current.getRegion().getShortNameToLog() + " NOT mergeable"); - if (mergeableCheckIOE != null) e.initCause(mergeableCheckIOE); - super.setFailure(getClass().getSimpleName(), e); - return false; - } // Update region states to Merging setRegionStateToMerging(env); @@ -589,8 +526,6 @@ public class MergeTableRegionsProcedure /** * Action after rollback a merge table regions action. - * @param env MasterProcedureEnv - * @throws IOException */ private void postRollBackMergeRegions(final MasterProcedureEnv env) throws IOException { final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); @@ -605,30 +540,35 @@ public class MergeTableRegionsProcedure private void setRegionStateToMerging(final MasterProcedureEnv env) { // Set State.MERGING to regions to be merged RegionStates regionStates = env.getAssignmentManager().getRegionStates(); - regionStates.getRegionStateNode(regionsToMerge[0]).setState(State.MERGING); - regionStates.getRegionStateNode(regionsToMerge[1]).setState(State.MERGING); + for (RegionInfo ri: this.regionsToMerge) { + regionStates.getRegionStateNode(ri).setState(State.MERGING); + } } /** * Create a merged region - * @param env MasterProcedureEnv */ private void createMergedRegion(final MasterProcedureEnv env) throws IOException { final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem(); final Path tabledir = FSUtils.getTableDir(mfs.getRootDir(), regionsToMerge[0].getTable()); final FileSystem fs = mfs.getFileSystem(); - HRegionFileSystem regionFs = HRegionFileSystem.openRegionFromFileSystem( - env.getMasterConfiguration(), fs, tabledir, regionsToMerge[0], false); - regionFs.createMergesDir(); - - mergeStoreFiles(env, regionFs, regionFs.getMergesDir()); - HRegionFileSystem regionFs2 = HRegionFileSystem.openRegionFromFileSystem( - env.getMasterConfiguration(), fs, tabledir, regionsToMerge[1], false); - mergeStoreFiles(env, regionFs2, regionFs.getMergesDir()); - - regionFs.commitMergedRegion(mergedRegion); + HRegionFileSystem mergeRegionFs = null; + for (RegionInfo ri: this.regionsToMerge) { + HRegionFileSystem regionFs = HRegionFileSystem.openRegionFromFileSystem( + env.getMasterConfiguration(), fs, tabledir, ri, false); + if (mergeRegionFs == null) { + // The tmp merges dir is made in the first region to be merged. + // See below in cleanupMergedRegion where we explicitly look in + // this.regionsToMerge[0] for the merge dir doing cleanup on rollback. + regionFs.createMergesDir(); + mergeRegionFs = regionFs; + } + mergeStoreFiles(env, regionFs, mergeRegionFs.getRegionDir()); + } + assert mergeRegionFs != null; + mergeRegionFs.commitMergedRegion(mergedRegion); - //Prepare to create merged regions + // Prepare to create merged regions env.getAssignmentManager().getRegionStates(). getOrCreateRegionStateNode(mergedRegion).setState(State.MERGING_NEW); } @@ -663,13 +603,15 @@ public class MergeTableRegionsProcedure } /** - * Clean up a merged region - * @param env MasterProcedureEnv + * Clean up a merged region on rollback after failure. */ private void cleanupMergedRegion(final MasterProcedureEnv env) throws IOException { final MasterFileSystem mfs = env.getMasterServices().getMasterFileSystem(); - final Path tabledir = FSUtils.getTableDir(mfs.getRootDir(), regionsToMerge[0].getTable()); + TableName tn = this.regionsToMerge[0].getTable(); + final Path tabledir = FSUtils.getTableDir(mfs.getRootDir(), tn); final FileSystem fs = mfs.getFileSystem(); + // See above where we build the merge files in a merge dir in first parent region + // to merge. HRegionFileSystem regionFs = HRegionFileSystem.openRegionFromFileSystem( env.getMasterConfiguration(), fs, tabledir, regionsToMerge[0], false); regionFs.cleanupMergedRegion(mergedRegion); @@ -696,8 +638,8 @@ public class MergeTableRegionsProcedure } private int getRegionReplication(final MasterProcedureEnv env) throws IOException { - final TableDescriptor htd = env.getMasterServices().getTableDescriptors().get(getTableName()); - return htd.getRegionReplication(); + return env.getMasterServices().getTableDescriptors().get(getTableName()). + getRegionReplication(); } /** @@ -785,18 +727,6 @@ public class MergeTableRegionsProcedure } } - /** - * The procedure could be restarted from a different machine. If the variable is null, we need to - * retrieve it. - * @return traceEnabled - */ - private Boolean isTraceEnabled() { - if (traceEnabled == null) { - traceEnabled = LOG.isTraceEnabled(); - } - return traceEnabled; - } - /** * @return The merged region. Maybe be null if called to early or we failed. */ @@ -810,6 +740,6 @@ public class MergeTableRegionsProcedure // Abort means rollback. We can't rollback all steps. HBASE-18018 added abort to all // Procedures. Here is a Procedure that has a PONR and cannot be aborted once it enters this // range of steps; what do we do for these should an operator want to cancel them? HBASE-20022. - return isRollbackSupported(getCurrentState())? super.abort(env): false; + return isRollbackSupported(getCurrentState()) && super.abort(env); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java index 24026e62e8..ccdbc183d1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java @@ -757,22 +757,22 @@ public class HRegionFileSystem { } /** - * Create the region merges directory. + * Create the region merges directory, a temporary directory to accumulate + * merges in. * @throws IOException If merges dir already exists or we fail to create it. * @see HRegionFileSystem#cleanupMergesDir() */ public void createMergesDir() throws IOException { Path mergesdir = getMergesDir(); if (fs.exists(mergesdir)) { - LOG.info("The " + mergesdir - + " directory exists. Hence deleting it to recreate it"); + LOG.info("{} directory exists. Deleting it to recreate it anew", mergesdir); if (!fs.delete(mergesdir, true)) { - throw new IOException("Failed deletion of " + mergesdir - + " before creating them again."); + throw new IOException("Failed deletion of " + mergesdir + " before recreate."); } } - if (!mkdirs(fs, conf, mergesdir)) + if (!mkdirs(fs, conf, mergesdir)) { throw new IOException("Failed create of " + mergesdir); + } } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java index 5b5cf1c90f..8b6ec9c3c0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/HBaseFsck.java @@ -2790,11 +2790,10 @@ public class HBaseFsck extends Configured implements Closeable { throw new IOException("Two entries in hbase:meta are same " + previous); } } - PairOfSameType mergeRegions = MetaTableAccessor.getMergeRegions(result); - for (RegionInfo mergeRegion : new RegionInfo[] { - mergeRegions.getFirst(), mergeRegions.getSecond() }) { + List mergeParents = MetaTableAccessor.getMergeRegions(result.rawCells()); + for (RegionInfo mergeRegion: mergeParents) { if (mergeRegion != null) { - // This region is already been merged + // This region is already being merged HbckRegionInfo hbInfo = getOrCreateInfo(mergeRegion.getEncodedName()); hbInfo.setMerged(true); } diff --git a/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp b/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp index fc212e838c..4036cc835f 100644 --- a/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp +++ b/hbase-server/src/main/resources/hbase-webapps/master/hbck.jsp @@ -27,6 +27,7 @@ import="java.time.ZonedDateTime" import="java.time.format.DateTimeFormatter" %> +<%@ page import="org.apache.hadoop.hbase.client.RegionInfo" %> <%@ page import="org.apache.hadoop.hbase.master.HbckChore" %> <%@ page import="org.apache.hadoop.hbase.master.HMaster" %> <%@ page import="org.apache.hadoop.hbase.ServerName" %> diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java index 15e276d6d1..8a4db853c2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessor.java @@ -17,11 +17,7 @@ */ package org.apache.hadoop.hbase; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import static org.mockito.ArgumentMatchers.anyObject; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -30,6 +26,7 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import java.io.IOException; +import java.util.ArrayList; import java.util.List; import java.util.Random; import org.apache.hadoop.conf.Configuration; @@ -37,6 +34,7 @@ import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.ConnectionFactory; import org.apache.hadoop.hbase.client.Get; +import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.RegionLocator; @@ -78,7 +76,6 @@ import org.apache.hbase.thirdparty.com.google.common.collect.Sets; @Category({MiscTests.class, MediumTests.class}) @SuppressWarnings("deprecation") public class TestMetaTableAccessor { - @ClassRule public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestMetaTableAccessor.class); @@ -107,6 +104,31 @@ public class TestMetaTableAccessor { UTIL.shutdownMiniCluster(); } + @Test + public void testGettingMergeRegions() throws IOException { + TableName tn = TableName.valueOf(this.name.getMethodName()); + Put put = new Put(Bytes.toBytes(this.name.getMethodName())); + List ris = new ArrayList<>(); + int limit = 10; + byte [] previous = HConstants.EMPTY_START_ROW; + for (int i = 0; i < limit; i++) { + RegionInfo ri = RegionInfoBuilder.newBuilder(tn). + setStartKey(previous).setEndKey(Bytes.toBytes(i)).build(); + ris.add(ri); + } + put = MetaTableAccessor.addMergeRegions(put, ris); + List cells = put.getFamilyCellMap().get(HConstants.CATALOG_FAMILY); + String previousQualifier = null; + assertEquals(limit, cells.size()); + for (Cell cell: cells) { + LOG.info(cell.toString()); + String qualifier = Bytes.toString(cell.getQualifierArray()); + assertTrue(qualifier.startsWith(HConstants.MERGE_QUALIFIER_PREFIX_STR)); + assertNotEquals(qualifier, previousQualifier); + previousQualifier = qualifier; + } + } + @Test public void testIsMetaWhenAllHealthy() throws InterruptedException { HMaster m = UTIL.getMiniHBaseCluster().getMaster(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java index 6a8c4b3ae5..8847fceded 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java @@ -24,11 +24,13 @@ import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableDescriptorBuilder; import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants; @@ -182,6 +184,57 @@ public class TestMergeTableRegionsProcedure { } } + /** + * This tests five region merges in one go. + */ + @Test + public void testMergeFiveRegions() throws Exception { + final TableName tableName = TableName.valueOf(this.name.getMethodName()); + final ProcedureExecutor procExec = getMasterProcedureExecutor(); + UTIL.createMultiRegionTable(tableName, HConstants.CATALOG_FAMILY); + List ris = MetaTableAccessor.getTableRegions(UTIL.getConnection(), tableName); + int originalRegionCount = ris.size(); + int mergeCount = 5; + assertTrue(originalRegionCount > mergeCount); + int countOfRowsLoaded = 0; + try (Table table = UTIL.getConnection().getTable(tableName)) { + countOfRowsLoaded = UTIL.loadTable(table, HConstants.CATALOG_FAMILY); + } + assertEquals(countOfRowsLoaded, UTIL.countRows(tableName)); + + // collect AM metrics before test + collectAssignmentManagerMetrics(); + + MergeTableRegionsProcedure proc = + new MergeTableRegionsProcedure(procExec.getEnvironment(), ris.subList(0, mergeCount). + toArray(new RegionInfo [] {}), true); + long procId = procExec.submitProcedure(proc); + ProcedureTestingUtility.waitProcedure(procExec, procId); + ProcedureTestingUtility.assertProcNotFailed(procExec, procId); + assertRegionCount(tableName, originalRegionCount - mergeCount); + + assertEquals(mergeSubmittedCount + 1, mergeProcMetrics.getSubmittedCounter().getCount()); + assertEquals(mergeFailedCount, mergeProcMetrics.getFailedCounter().getCount()); + assertEquals(assignSubmittedCount + 1, assignProcMetrics.getSubmittedCounter().getCount()); + assertEquals(assignFailedCount, assignProcMetrics.getFailedCounter().getCount()); + assertEquals(unassignSubmittedCount + 2, unassignProcMetrics.getSubmittedCounter().getCount()); + assertEquals(unassignFailedCount, unassignProcMetrics.getFailedCounter().getCount()); + + List mergeParents = MetaTableAccessor.getMergeRegions(UTIL.getConnection(), + proc.getMergedRegion().getRegionName()); + + // Can I purge the merged regions from hbase:meta? Check that all went + // well by looking at the merged row up in hbase:meta. It should have no + // more mention of the merged regions; they are purged as last step in + // the merged regions cleanup. + UTIL.getHBaseCluster().getMaster().setCatalogJanitorEnabled(true); + UTIL.getHBaseCluster().getMaster().getCatalogJanitor().triggerNow(); + for (List waitingTillEmpty = null; !waitingTillEmpty.isEmpty();) { + waitingTillEmpty = MetaTableAccessor.getMergeRegions(UTIL.getConnection(), + proc.getMergedRegion().getRegionName()); + } + } + /** * This tests two concurrent region merges */ diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java index 7b54ffb57d..dd6fd9548d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java @@ -1,4 +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 @@ -33,7 +33,6 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtility; -import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.MiniHBaseCluster; import org.apache.hadoop.hbase.ServerName; @@ -216,15 +215,12 @@ public class TestRegionMergeTransactionOnCluster { MASTER.getConnection(), mergedRegionInfo.getRegionName()); // contains merge reference in META - assertTrue(mergedRegionResult.getValue(HConstants.CATALOG_FAMILY, - HConstants.MERGEA_QUALIFIER) != null); - assertTrue(mergedRegionResult.getValue(HConstants.CATALOG_FAMILY, - HConstants.MERGEB_QUALIFIER) != null); + assertTrue(MetaTableAccessor.hasMergeRegions(mergedRegionResult.rawCells())); // merging regions' directory are in the file system all the same - PairOfSameType p = MetaTableAccessor.getMergeRegions(mergedRegionResult); - RegionInfo regionA = p.getFirst(); - RegionInfo regionB = p.getSecond(); + List p = MetaTableAccessor.getMergeRegions(mergedRegionResult.rawCells()); + RegionInfo regionA = p.get(0); + RegionInfo regionB = p.get(1); FileSystem fs = MASTER.getMasterFileSystem().getFileSystem(); Path rootDir = MASTER.getMasterFileSystem().getRootDir(); @@ -291,11 +287,7 @@ public class TestRegionMergeTransactionOnCluster { mergedRegionResult = MetaTableAccessor.getRegionResult( TEST_UTIL.getConnection(), mergedRegionInfo.getRegionName()); - assertFalse(mergedRegionResult.getValue(HConstants.CATALOG_FAMILY, - HConstants.MERGEA_QUALIFIER) != null); - assertFalse(mergedRegionResult.getValue(HConstants.CATALOG_FAMILY, - HConstants.MERGEB_QUALIFIER) != null); - + assertFalse(MetaTableAccessor.hasMergeRegions(mergedRegionResult.rawCells())); } finally { ADMIN.catalogJanitorSwitch(true); } -- 2.19.1