From 4df536e31d23d3b76c97b9e9cc5b2fcf2247fc15 Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Mon, 5 Oct 2020 21:29:55 +0800 Subject: [PATCH] HBASE-25121 Refactor MetaTableAccessor.addRegionsToMeta and its usage places (#2476) Signed-off-by: stack --- .../hadoop/hbase/MetaTableAccessor.java | 59 +++------- .../master/assignment/RegionStateStore.java | 106 +++++++++++++++++- .../hbase/master/janitor/MetaFixer.java | 15 ++- .../procedure/CreateTableProcedure.java | 15 ++- .../procedure/DeleteTableProcedure.java | 32 ++++-- .../procedure/ModifyTableProcedure.java | 70 ++---------- .../hbase/util/ServerRegionReplicaUtil.java | 43 +++---- .../hadoop/hbase/HBaseTestingUtility.java | 2 +- .../hadoop/hbase/TestMetaTableAccessor.java | 47 +------- .../hadoop/hbase/client/TestEnableTable.java | 6 +- .../assignment/TestAssignmentManager.java | 4 +- .../assignment/TestRegionStateStore.java | 59 +++++++++- 12 files changed, 250 insertions(+), 208 deletions(-) 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 5b35fc5d4e3..4c341c5dba6 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 @@ -31,12 +31,12 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.NavigableMap; -import java.util.Set; -import java.util.TreeMap; import java.util.SortedMap; +import java.util.TreeMap; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.Cell.Type; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.Consistency; @@ -358,7 +358,7 @@ public class MetaTableAccessor { String regionEncodedName) throws IOException { RowFilter rowFilter = new RowFilter(CompareOperator.EQUAL, new SubstringComparator(regionEncodedName)); - Scan scan = getMetaScan(connection, 1); + Scan scan = getMetaScan(connection.getConfiguration(), 1); scan.setFilter(rowFilter); try (Table table = getMetaHTable(connection); ResultScanner resultScanner = table.getScanner(scan)) { @@ -563,25 +563,23 @@ public class MetaTableAccessor { * @param tableName bytes of table's name * @return configured Scan object */ - public static Scan getScanForTableName(Connection connection, TableName tableName) { + public static Scan getScanForTableName(Configuration conf, TableName tableName) { // Start key is just the table name with delimiters byte[] startKey = getTableStartRowForMeta(tableName, QueryType.REGION); // Stop key appends the smallest possible char to the table name byte[] stopKey = getTableStopRowForMeta(tableName, QueryType.REGION); - Scan scan = getMetaScan(connection, -1); + Scan scan = getMetaScan(conf, -1); scan.setStartRow(startKey); scan.setStopRow(stopKey); return scan; } - private static Scan getMetaScan(Connection connection, int rowUpperLimit) { + private static Scan getMetaScan(Configuration conf, int rowUpperLimit) { Scan scan = new Scan(); - int scannerCaching = connection.getConfiguration() - .getInt(HConstants.HBASE_META_SCANNER_CACHING, - HConstants.DEFAULT_HBASE_META_SCANNER_CACHING); - if (connection.getConfiguration().getBoolean(HConstants.USE_META_REPLICAS, - HConstants.DEFAULT_USE_META_REPLICAS)) { + int scannerCaching = conf.getInt(HConstants.HBASE_META_SCANNER_CACHING, + HConstants.DEFAULT_HBASE_META_SCANNER_CACHING); + if (conf.getBoolean(HConstants.USE_META_REPLICAS, HConstants.DEFAULT_USE_META_REPLICAS)) { scan.setConsistency(Consistency.TIMELINE); } if (rowUpperLimit > 0) { @@ -591,6 +589,7 @@ public class MetaTableAccessor { scan.setCaching(scannerCaching); return scan; } + /** * Do not use this method to get meta table regions, use methods in MetaTableLocator instead. * @param connection connection we're using @@ -774,7 +773,7 @@ public class MetaTableAccessor { @Nullable final byte[] stopRow, QueryType type, @Nullable Filter filter, int maxRows, final Visitor visitor) throws IOException { int rowUpperLimit = maxRows > 0 ? maxRows : Integer.MAX_VALUE; - Scan scan = getMetaScan(connection, rowUpperLimit); + Scan scan = getMetaScan(connection.getConfiguration(), rowUpperLimit); for (byte[] family : type.getFamilies()) { scan.addFamily(family); @@ -824,7 +823,7 @@ public class MetaTableAccessor { private static RegionInfo getClosestRegionInfo(Connection connection, @NonNull final TableName tableName, @NonNull final byte[] row) throws IOException { byte[] searchRow = RegionInfo.createRegionName(tableName, row, HConstants.NINES, false); - Scan scan = getMetaScan(connection, 1); + Scan scan = getMetaScan(connection.getConfiguration(), 1); scan.setReversed(true); scan.withStartRow(searchRow); try (ResultScanner resultScanner = getMetaHTable(connection).getScanner(scan)) { @@ -887,8 +886,7 @@ public class MetaTableAccessor { * @param replicaId the replicaId of the region * @return a byte[] for state qualifier */ - @VisibleForTesting - static byte[] getRegionStateColumn(int replicaId) { + public static byte[] getRegionStateColumn(int replicaId) { return replicaId == 0 ? HConstants.STATE_QUALIFIER : Bytes.toBytes(HConstants.STATE_QUALIFIER_STR + META_REPLICA_ID_DELIMITER + String.format(RegionInfo.REPLICA_ID_FORMAT, replicaId)); @@ -1419,35 +1417,6 @@ public class MetaTableAccessor { } } - /** - * Deletes some replica columns corresponding to replicas for the passed rows - * @param metaRows rows in hbase:meta - * @param replicaIndexToDeleteFrom the replica ID we would start deleting from - * @param numReplicasToRemove how many replicas to remove - * @param connection connection we're using to access meta table - */ - public static void removeRegionReplicasFromMeta(Set metaRows, - int replicaIndexToDeleteFrom, int numReplicasToRemove, Connection connection) - throws IOException { - int absoluteIndex = replicaIndexToDeleteFrom + numReplicasToRemove; - for (byte[] row : metaRows) { - long now = EnvironmentEdgeManager.currentTime(); - Delete deleteReplicaLocations = new Delete(row); - for (int i = replicaIndexToDeleteFrom; i < absoluteIndex; i++) { - deleteReplicaLocations.addColumns(getCatalogFamily(), - getServerColumn(i), now); - deleteReplicaLocations.addColumns(getCatalogFamily(), - getSeqNumColumn(i), now); - deleteReplicaLocations.addColumns(getCatalogFamily(), - getStartCodeColumn(i), now); - deleteReplicaLocations.addColumns(getCatalogFamily(), getServerNameColumn(i), now); - deleteReplicaLocations.addColumns(getCatalogFamily(), getRegionStateColumn(i), now); - } - - deleteFromMetaTable(connection, deleteReplicaLocations); - } - } - private static Put addRegionStateToPut(Put put, RegionState.State state) throws IOException { put.add(CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY) .setRow(put.getRow()) @@ -2030,7 +1999,7 @@ public class MetaTableAccessor { .build()); } - private static Put addEmptyLocation(Put p, int replicaId) throws IOException { + public static Put addEmptyLocation(Put p, int replicaId) throws IOException { CellBuilder builder = CellBuilderFactory.create(CellBuilderType.SHALLOW_COPY); return p.add(builder.clear() .setRow(p.getRow()) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java index c353161827e..3a20d19ea7f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java @@ -18,11 +18,11 @@ package org.apache.hadoop.hbase.master.assignment; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.SortedMap; import java.util.TreeMap; - import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellBuilderFactory; import org.apache.hadoop.hbase.CellBuilderType; @@ -32,9 +32,13 @@ import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.RegionLocations; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Delete; +import org.apache.hadoop.hbase.client.Mutation; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.master.MasterFileSystem; @@ -59,6 +63,7 @@ import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; @InterfaceAudience.Private public class RegionStateStore { private static final Logger LOG = LoggerFactory.getLogger(RegionStateStore.class); + private static final Logger METALOG = LoggerFactory.getLogger("org.apache.hadoop.hbase.META"); /** The delimiter for meta columns for replicaIds > 0 */ protected static final char META_REPLICA_ID_DELIMITER = '_'; @@ -220,7 +225,8 @@ public class RegionStateStore { private void updateRegionLocation(RegionInfo regionInfo, State state, Put put) throws IOException { - try (Table table = master.getConnection().getTable(TableName.META_TABLE_NAME)) { + try (Table table = getMetaTable()) { + debugLogMutation(put); table.put(put); } catch (IOException e) { // TODO: Revist!!!! Means that if a server is loaded, then we will abort our host! @@ -240,6 +246,10 @@ public class RegionStateStore { return maxSeqId > 0 ? maxSeqId + 1 : HConstants.NO_SEQNUM; } + private Table getMetaTable() throws IOException { + return master.getConnection().getTable(TableName.META_TABLE_NAME); + } + // ============================================================================================ // Update Region Splitting State helpers // ============================================================================================ @@ -280,6 +290,83 @@ public class RegionStateStore { MetaTableAccessor.deleteRegionInfos(master.getConnection(), regions); } + /** + * Update region replicas if necessary by adding new replica locations or removing unused region + * replicas + */ + public void updateRegionReplicas(TableName tableName, int oldReplicaCount, int newReplicaCount) + throws IOException { + if (newReplicaCount < oldReplicaCount) { + removeRegionReplicas(tableName, oldReplicaCount, newReplicaCount); + } else if (newReplicaCount > oldReplicaCount) { + addRegionReplicas(tableName, oldReplicaCount, newReplicaCount); + } + } + + private Scan getScanForUpdateRegionReplicas(TableName tableName) { + return MetaTableAccessor.getScanForTableName(master.getConfiguration(), tableName) + .addColumn(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER); + } + + private void removeRegionReplicas(TableName tableName, int oldReplicaCount, int newReplicaCount) + throws IOException { + Scan scan = getScanForUpdateRegionReplicas(tableName); + List deletes = new ArrayList<>(); + long now = EnvironmentEdgeManager.currentTime(); + try (Table metaTable = getMetaTable(); ResultScanner scanner = metaTable.getScanner(scan)) { + for (;;) { + Result result = scanner.next(); + if (result == null) { + break; + } + RegionInfo primaryRegionInfo = MetaTableAccessor.getRegionInfo(result); + if (primaryRegionInfo == null || primaryRegionInfo.isSplitParent()) { + continue; + } + Delete delete = new Delete(result.getRow()); + for (int i = newReplicaCount; i < oldReplicaCount; i++) { + delete.addColumns(HConstants.CATALOG_FAMILY, MetaTableAccessor.getServerColumn(i), now); + delete.addColumns(HConstants.CATALOG_FAMILY, MetaTableAccessor.getSeqNumColumn(i), now); + delete.addColumns(HConstants.CATALOG_FAMILY, MetaTableAccessor.getStartCodeColumn(i), + now); + delete.addColumns(HConstants.CATALOG_FAMILY, MetaTableAccessor.getServerNameColumn(i), + now); + delete.addColumns(HConstants.CATALOG_FAMILY, MetaTableAccessor.getRegionStateColumn(i), + now); + } + deletes.add(delete); + } + debugLogMutations(deletes); + metaTable.delete(deletes); + } + } + + private void addRegionReplicas(TableName tableName, int oldReplicaCount, int newReplicaCount) + throws IOException { + Scan scan = getScanForUpdateRegionReplicas(tableName); + List puts = new ArrayList<>(); + long now = EnvironmentEdgeManager.currentTime(); + try (Table metaTable = getMetaTable(); ResultScanner scanner = metaTable.getScanner(scan)) { + for (;;) { + Result result = scanner.next(); + if (result == null) { + break; + } + RegionInfo primaryRegionInfo = MetaTableAccessor.getRegionInfo(result); + if (primaryRegionInfo == null || primaryRegionInfo.isSplitParent()) { + continue; + } + Put put = new Put(result.getRow(), now); + for (int i = oldReplicaCount; i < newReplicaCount; i++) { + MetaTableAccessor.addEmptyLocation(put, i); + } + puts.add(put); + } + debugLogMutations(puts); + metaTable.put(puts); + } + } + // ========================================================================== // Table Descriptors helpers // ========================================================================== @@ -332,4 +419,19 @@ public class RegionStateStore { : Bytes.toBytes(HConstants.STATE_QUALIFIER_STR + META_REPLICA_ID_DELIMITER + String.format(RegionInfo.REPLICA_ID_FORMAT, replicaId)); } + + private static void debugLogMutations(List mutations) throws IOException { + if (!METALOG.isDebugEnabled()) { + return; + } + // Logging each mutation in separate line makes it easier to see diff between them visually + // because of common starting indentation. + for (Mutation mutation : mutations) { + debugLogMutation(mutation); + } + } + + private static void debugLogMutation(Mutation p) throws IOException { + METALOG.debug("{} {}", p.getClass().getSimpleName(), p.toJSON()); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java index ca6c64d30f7..791e3d7d5ac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/janitor/MetaFixer.java @@ -29,6 +29,7 @@ import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; import java.util.stream.Collectors; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; @@ -39,6 +40,7 @@ import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.exceptions.MergeRegionException; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure; +import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil; @@ -190,8 +192,8 @@ public class MetaFixer { // Add replicas if needed // we need to create regions with replicaIds starting from 1 - List newRegions = RegionReplicaUtil.addReplicas( - Collections.singletonList(regionInfo), 1, td.getRegionReplication()); + List newRegions = RegionReplicaUtil + .addReplicas(Collections.singletonList(regionInfo), 1, td.getRegionReplication()); // Add regions to META MetaTableAccessor.addRegionsToMeta(masterServices.getConnection(), newRegions, @@ -199,12 +201,13 @@ public class MetaFixer { // Setup replication for region replicas if needed if (td.getRegionReplication() > 1) { - ServerRegionReplicaUtil.setupRegionReplicaReplication( - masterServices.getConfiguration()); + ServerRegionReplicaUtil.setupRegionReplicaReplication(masterServices); } - return Either., IOException>ofLeft(newRegions); + return Either., IOException> ofLeft(newRegions); } catch (IOException e) { - return Either., IOException>ofRight(e); + return Either., IOException> ofRight(e); + } catch (ReplicationException e) { + return Either., IOException> ofRight(new HBaseIOException(e)); } }) .collect(Collectors.toList()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java index 8bef7d1389e..b4aec1eba5a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java @@ -24,6 +24,7 @@ import java.util.List; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableExistsException; import org.apache.hadoop.hbase.TableName; @@ -34,6 +35,7 @@ import org.apache.hadoop.hbase.client.TableState; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; +import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.util.CommonFSUtils; import org.apache.hadoop.hbase.util.FSTableDescriptors; import org.apache.hadoop.hbase.util.ModifyRegionUtils; @@ -343,23 +345,26 @@ public class CreateTableProcedure } protected static List addTableToMeta(final MasterProcedureEnv env, - final TableDescriptor tableDescriptor, - final List regions) throws IOException { + final TableDescriptor tableDescriptor, final List regions) throws IOException { assert (regions != null && regions.size() > 0) : "expected at least 1 region, got " + regions; ProcedureSyncWait.waitMetaRegions(env); // Add replicas if needed // we need to create regions with replicaIds starting from 1 - List newRegions = RegionReplicaUtil.addReplicas(regions, 1, - tableDescriptor.getRegionReplication()); + List newRegions = + RegionReplicaUtil.addReplicas(regions, 1, tableDescriptor.getRegionReplication()); // Add regions to META addRegionsToMeta(env, tableDescriptor, newRegions); // Setup replication for region replicas if needed if (tableDescriptor.getRegionReplication() > 1) { - ServerRegionReplicaUtil.setupRegionReplicaReplication(env.getMasterConfiguration()); + try { + ServerRegionReplicaUtil.setupRegionReplicaReplication(env.getMasterServices()); + } catch (ReplicationException e) { + throw new HBaseIOException(e); + } } return newRegions; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java index 5b118a4f37c..9cfce0ce363 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java @@ -31,7 +31,6 @@ import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.backup.HFileArchiver; -import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.Delete; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; @@ -40,12 +39,14 @@ import org.apache.hadoop.hbase.client.ResultScanner; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.favored.FavoredNodesManager; +import org.apache.hadoop.hbase.filter.KeyOnlyFilter; import org.apache.hadoop.hbase.master.MasterCoprocessorHost; import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.mob.MobConstants; import org.apache.hadoop.hbase.mob.MobUtils; import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; @@ -357,22 +358,29 @@ public class DeleteTableProcedure /** * There may be items for this table still up in hbase:meta in the case where the info:regioninfo * column was empty because of some write error. Remove ALL rows from hbase:meta that have to do - * with this table. See HBASE-12980. + * with this table. + *

+ * See HBASE-12980. */ private static void cleanRegionsInMeta(final MasterProcedureEnv env, final TableName tableName) - throws IOException { - Connection connection = env.getMasterServices().getConnection(); - Scan tableScan = MetaTableAccessor.getScanForTableName(connection, tableName); - try (Table metaTable = connection.getTable(TableName.META_TABLE_NAME)) { - List deletes = new ArrayList<>(); - try (ResultScanner resScanner = metaTable.getScanner(tableScan)) { - for (Result result : resScanner) { - deletes.add(new Delete(result.getRow())); + throws IOException { + Scan tableScan = MetaTableAccessor.getScanForTableName(env.getMasterConfiguration(), tableName) + .setFilter(new KeyOnlyFilter()); + long now = EnvironmentEdgeManager.currentTime(); + List deletes = new ArrayList<>(); + try ( + Table metaTable = env.getMasterServices().getConnection().getTable(TableName.META_TABLE_NAME); + ResultScanner scanner = metaTable.getScanner(tableScan)) { + for (;;) { + Result result = scanner.next(); + if (result == null) { + break; } + deletes.add(new Delete(result.getRow(), now)); } if (!deletes.isEmpty()) { - LOG.warn("Deleting some vestigial " + deletes.size() + " rows of " + tableName + " from " - + TableName.META_TABLE_NAME); + LOG.warn("Deleting some vestigial " + deletes.size() + " rows of " + tableName + " from " + + TableName.META_TABLE_NAME); metaTable.delete(deletes); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java index 10d1ca7a62a..4b9bd56d522 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java @@ -21,28 +21,21 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import java.util.Arrays; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Set; - import org.apache.hadoop.hbase.ConcurrentTableModificationException; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; -import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.client.Result; -import org.apache.hadoop.hbase.client.ResultScanner; -import org.apache.hadoop.hbase.client.Scan; -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.MasterCoprocessorHost; import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; +import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil; import org.apache.yetus.audience.InterfaceAudience; @@ -339,8 +332,6 @@ public class ModifyTableProcedure * Action before modifying table. * @param env MasterProcedureEnv * @param state the procedure state - * @throws IOException - * @throws InterruptedException */ private void preModify(final MasterProcedureEnv env, final ModifyTableState state) throws IOException, InterruptedException { @@ -350,7 +341,6 @@ public class ModifyTableProcedure /** * Update descriptor * @param env MasterProcedureEnv - * @throws IOException **/ private void updateTableDescriptor(final MasterProcedureEnv env) throws IOException { env.getMasterServices().getTableDescriptors().update(modifiedTableDescriptor); @@ -359,7 +349,6 @@ public class ModifyTableProcedure /** * Removes from hdfs the families that are not longer present in the new table descriptor. * @param env MasterProcedureEnv - * @throws IOException */ private void deleteFromFs(final MasterProcedureEnv env, final TableDescriptor oldTableDescriptor, final TableDescriptor newTableDescriptor) @@ -379,61 +368,28 @@ public class ModifyTableProcedure /** * update replica column families if necessary. - * @param env MasterProcedureEnv - * @throws IOException */ - private void updateReplicaColumnsIfNeeded( - final MasterProcedureEnv env, - final TableDescriptor oldTableDescriptor, - final TableDescriptor newTableDescriptor) throws IOException { + private void updateReplicaColumnsIfNeeded(MasterProcedureEnv env, + TableDescriptor oldTableDescriptor, TableDescriptor newTableDescriptor) throws IOException { final int oldReplicaCount = oldTableDescriptor.getRegionReplication(); final int newReplicaCount = newTableDescriptor.getRegionReplication(); - - if (newReplicaCount < oldReplicaCount) { - Set tableRows = new HashSet<>(); - Connection connection = env.getMasterServices().getConnection(); - Scan scan = MetaTableAccessor.getScanForTableName(connection, getTableName()); - scan.addColumn(HConstants.CATALOG_FAMILY, HConstants.REGIONINFO_QUALIFIER); - - try (Table metaTable = connection.getTable(TableName.META_TABLE_NAME)) { - ResultScanner resScanner = metaTable.getScanner(scan); - for (Result result : resScanner) { - tableRows.add(result.getRow()); - } - MetaTableAccessor.removeRegionReplicasFromMeta( - tableRows, - newReplicaCount, - oldReplicaCount - newReplicaCount, - connection); - } - } - if (newReplicaCount > oldReplicaCount) { - Connection connection = env.getMasterServices().getConnection(); - // Get the existing table regions - List existingTableRegions = - MetaTableAccessor.getTableRegions(connection, getTableName()); - // add all the new entries to the meta table - addRegionsToMeta(env, newTableDescriptor, existingTableRegions); - if (oldReplicaCount <= 1) { - // The table has been newly enabled for replica. So check if we need to setup - // region replication - ServerRegionReplicaUtil.setupRegionReplicaReplication(env.getMasterConfiguration()); + env.getAssignmentManager().getRegionStateStore().updateRegionReplicas(getTableName(), + oldReplicaCount, newReplicaCount); + if (newReplicaCount > oldReplicaCount && oldReplicaCount <= 1) { + // The table has been newly enabled for replica. So check if we need to setup + // region replication + try { + ServerRegionReplicaUtil.setupRegionReplicaReplication(env.getMasterServices()); + } catch (ReplicationException e) { + throw new HBaseIOException(e); } } } - private static void addRegionsToMeta(final MasterProcedureEnv env, - final TableDescriptor tableDescriptor, final List regionInfos) - throws IOException { - MetaTableAccessor.addRegionsToMeta(env.getMasterServices().getConnection(), regionInfos, - tableDescriptor.getRegionReplication()); - } /** * Action after modifying table. * @param env MasterProcedureEnv * @param state the procedure state - * @throws IOException - * @throws InterruptedException */ private void postModify(final MasterProcedureEnv env, final ModifyTableState state) throws IOException, InterruptedException { @@ -444,8 +400,6 @@ public class ModifyTableProcedure * Coprocessor Action. * @param env MasterProcedureEnv * @param state the procedure state - * @throws IOException - * @throws InterruptedException */ private void runCoprocessorAction(final MasterProcedureEnv env, final ModifyTableState state) throws IOException, InterruptedException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java index b83749d9c33..fbd8d30bba6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ServerRegionReplicaUtil.java @@ -22,16 +22,15 @@ import java.io.IOException; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.hbase.ReplicationPeerNotFoundException; 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.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.io.HFileLink; import org.apache.hadoop.hbase.io.Reference; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.StoreFileInfo; +import org.apache.hadoop.hbase.replication.ReplicationException; import org.apache.hadoop.hbase.replication.ReplicationPeerConfig; import org.apache.hadoop.hbase.replication.regionserver.RegionReplicaReplicationEndpoint; import org.apache.hadoop.hbase.zookeeper.ZKConfig; @@ -155,34 +154,24 @@ public class ServerRegionReplicaUtil extends RegionReplicaUtil { /** * Create replication peer for replicating to region replicas if needed. - * @param conf configuration to use - * @throws IOException + *

+ * This methods should only be called at master side. */ - public static void setupRegionReplicaReplication(Configuration conf) throws IOException { - if (!isRegionReplicaReplicationEnabled(conf)) { + public static void setupRegionReplicaReplication(MasterServices services) + throws IOException, ReplicationException { + if (!isRegionReplicaReplicationEnabled(services.getConfiguration())) { return; } - - try (Connection connection = ConnectionFactory.createConnection(conf); - Admin admin = connection.getAdmin()) { - ReplicationPeerConfig peerConfig = null; - try { - peerConfig = admin.getReplicationPeerConfig(REGION_REPLICA_REPLICATION_PEER); - } catch (ReplicationPeerNotFoundException e) { - LOG.warn( - "Region replica replication peer id=" + REGION_REPLICA_REPLICATION_PEER + " not exist", - e); - } - - if (peerConfig == null) { - LOG.info("Region replica replication peer id=" + REGION_REPLICA_REPLICATION_PEER - + " not exist. Creating..."); - peerConfig = new ReplicationPeerConfig(); - peerConfig.setClusterKey(ZKConfig.getZooKeeperClusterKey(conf)); - peerConfig.setReplicationEndpointImpl(RegionReplicaReplicationEndpoint.class.getName()); - admin.addReplicationPeer(REGION_REPLICA_REPLICATION_PEER, peerConfig); - } + if (services.getReplicationPeerManager().getPeerConfig(REGION_REPLICA_REPLICATION_PEER) + .isPresent()) { + return; } + LOG.info("Region replica replication peer id=" + REGION_REPLICA_REPLICATION_PEER + + " not exist. Creating..."); + ReplicationPeerConfig peerConfig = ReplicationPeerConfig.newBuilder() + .setClusterKey(ZKConfig.getZooKeeperClusterKey(services.getConfiguration())) + .setReplicationEndpointImpl(RegionReplicaReplicationEndpoint.class.getName()).build(); + services.addReplicationPeer(REGION_REPLICA_REPLICATION_PEER, peerConfig, true); } public static boolean isRegionReplicaReplicationEnabled(Configuration conf) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java index e42ecb62c59..3b0e00707c1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java @@ -2651,7 +2651,7 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility { .setStartKey(startKeys[i]) .setEndKey(startKeys[j]) .build(); - MetaTableAccessor.addRegionToMeta(getConnection(), hri); + MetaTableAccessor.addRegionsToMeta(getConnection(), Collections.singletonList(hri), 1); newRegions.add(hri); } 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 fae0ebc6a67..0abfb4ea065 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 @@ -32,12 +32,12 @@ import static org.mockito.Mockito.verify; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Random; import java.util.concurrent.TimeUnit; - import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.Admin; import org.apache.hadoop.hbase.client.Connection; @@ -76,7 +76,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.collect.Lists; -import org.apache.hbase.thirdparty.com.google.common.collect.Sets; /** * Test {@link org.apache.hadoop.hbase.MetaTableAccessor}. @@ -218,9 +217,11 @@ public class TestMetaTableAccessor { } }; MetaTask writer = new MetaTask(connection, "writer") { + @Override - void metaTask() throws Throwable { - MetaTableAccessor.addRegionToMeta(connection, regions.get(0)); + void metaTask() throws IOException { + MetaTableAccessor.addRegionsToMeta(connection, Collections.singletonList(regions.get(0)), + 1); LOG.info("Wrote " + regions.get(0).getEncodedName()); } }; @@ -538,44 +539,6 @@ public class TestMetaTableAccessor { assertEquals(0, startCodeCell.getValueLength()); } - @Test - public void testMetaLocationForRegionReplicasIsRemovedAtTableDeletion() throws IOException { - long regionId = System.currentTimeMillis(); - RegionInfo primary = RegionInfoBuilder.newBuilder(TableName.valueOf(name.getMethodName())) - .setStartKey(HConstants.EMPTY_START_ROW).setEndKey(HConstants.EMPTY_END_ROW).setSplit(false) - .setRegionId(regionId).setReplicaId(0).build(); - - Table meta = MetaTableAccessor.getMetaHTable(connection); - try { - List regionInfos = Lists.newArrayList(primary); - MetaTableAccessor.addRegionsToMeta(connection, regionInfos, 3); - MetaTableAccessor.removeRegionReplicasFromMeta(Sets.newHashSet(primary.getRegionName()), 1, 2, - connection); - Get get = new Get(primary.getRegionName()); - Result result = meta.get(get); - for (int replicaId = 0; replicaId < 3; replicaId++) { - Cell serverCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, - MetaTableAccessor.getServerColumn(replicaId)); - Cell startCodeCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, - MetaTableAccessor.getStartCodeColumn(replicaId)); - Cell stateCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, - MetaTableAccessor.getRegionStateColumn(replicaId)); - Cell snCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, - MetaTableAccessor.getServerNameColumn(replicaId)); - if (replicaId == 0) { - assertNotNull(stateCell); - } else { - assertNull(serverCell); - assertNull(startCodeCell); - assertNull(stateCell); - assertNull(snCell); - } - } - } finally { - meta.close(); - } - } - @Test public void testMetaLocationForRegionReplicasIsAddedAtTableCreation() throws IOException { long regionId = System.currentTimeMillis(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestEnableTable.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestEnableTable.java index 4e2d6bab722..b92ab0fb0e8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestEnableTable.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestEnableTable.java @@ -98,7 +98,7 @@ public class TestEnableTable { // content from a few of the rows. try (Table metaTable = TEST_UTIL.getConnection().getTable(TableName.META_TABLE_NAME)) { try (ResultScanner scanner = metaTable.getScanner( - MetaTableAccessor.getScanForTableName(TEST_UTIL.getConnection(), tableName))) { + MetaTableAccessor.getScanForTableName(TEST_UTIL.getConfiguration(), tableName))) { for (Result result : scanner) { // Just delete one row. Delete d = new Delete(result.getRow()); @@ -118,8 +118,8 @@ public class TestEnableTable { fail("Got an exception while deleting " + tableName); } int rowCount = 0; - try (ResultScanner scanner = metaTable - .getScanner(MetaTableAccessor.getScanForTableName(TEST_UTIL.getConnection(), tableName))) { + try (ResultScanner scanner = metaTable.getScanner( + MetaTableAccessor.getScanForTableName(TEST_UTIL.getConfiguration(), tableName))) { for (Result result : scanner) { LOG.info("Found when none expected: " + result); rowCount++; diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java index 0f4e97fd753..b7dd87b54e0 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java @@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import java.util.Collections; import java.util.concurrent.Executors; import java.util.concurrent.Future; import org.apache.hadoop.hbase.HBaseClassTestRule; @@ -299,7 +300,8 @@ public class TestAssignmentManager extends TestAssignmentManagerBase { RegionInfo hri = createRegionInfo(tableName, 1); assertNull("RegionInfo was just instantiated by the test, but " + "shouldn't be in AM regionStates yet.", am.getRegionStates().getRegionState(hri)); - MetaTableAccessor.addRegionToMeta(this.util.getConnection(), hri); + MetaTableAccessor.addRegionsToMeta(this.util.getConnection(), Collections.singletonList(hri), + 1); assertNull("RegionInfo was manually added in META, but " + "shouldn't be in AM regionStates yet.", am.getRegionStates().getRegionState(hri)); hri = am.loadRegionFromMeta(hri.getEncodedName()); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java index fea362f13f3..8015449f446 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestRegionStateStore.java @@ -19,17 +19,25 @@ package org.apache.hadoop.hbase.master.assignment; 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 java.io.IOException; import java.util.List; import java.util.concurrent.atomic.AtomicBoolean; +import org.apache.hadoop.hbase.Cell; 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.ServerName; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNameTestRule; +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.Result; import org.apache.hadoop.hbase.client.Table; import org.apache.hadoop.hbase.master.RegionState; @@ -41,10 +49,11 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.ClassRule; +import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; + +import org.apache.hbase.thirdparty.com.google.common.collect.Lists; @Category({ MasterTests.class, MediumTests.class }) @@ -54,10 +63,11 @@ public class TestRegionStateStore { public static final HBaseClassTestRule CLASS_RULE = HBaseClassTestRule.forClass(TestRegionStateStore.class); - private static final Logger LOG = LoggerFactory.getLogger(TestRegionStateStore.class); - private static HBaseTestingUtility UTIL = new HBaseTestingUtility(); + @Rule + public final TableNameTestRule name = new TableNameTestRule(); + @BeforeClass public static void beforeClass() throws Exception { UTIL.startMiniCluster(); @@ -70,7 +80,7 @@ public class TestRegionStateStore { @Test public void testVisitMetaForRegionExistingRegion() throws Exception { - final TableName tableName = TableName.valueOf("testVisitMetaForRegion"); + final TableName tableName = name.getTableName(); UTIL.createTable(tableName, "cf"); final List regions = UTIL.getHBaseCluster().getRegions(tableName); final String encodedName = regions.get(0).getRegionInfo().getEncodedName(); @@ -90,7 +100,7 @@ public class TestRegionStateStore { @Test public void testVisitMetaForBadRegionState() throws Exception { - final TableName tableName = TableName.valueOf("testVisitMetaForBadRegionState"); + final TableName tableName = name.getTableName(); UTIL.createTable(tableName, "cf"); final List regions = UTIL.getHBaseCluster().getRegions(tableName); final String encodedName = regions.get(0).getRegionInfo().getEncodedName(); @@ -136,4 +146,41 @@ public class TestRegionStateStore { }); assertFalse("Visitor has been called, but it shouldn't.", visitorCalled.get()); } + + @Test + public void testMetaLocationForRegionReplicasIsRemovedAtTableDeletion() throws IOException { + long regionId = System.currentTimeMillis(); + TableName tableName = name.getTableName(); + RegionInfo primary = RegionInfoBuilder.newBuilder(tableName) + .setStartKey(HConstants.EMPTY_START_ROW).setEndKey(HConstants.EMPTY_END_ROW).setSplit(false) + .setRegionId(regionId).setReplicaId(0).build(); + + try (Table meta = MetaTableAccessor.getMetaHTable(UTIL.getConnection())) { + List regionInfos = Lists.newArrayList(primary); + MetaTableAccessor.addRegionsToMeta(UTIL.getConnection(), regionInfos, 3); + final RegionStateStore regionStateStore = + UTIL.getHBaseCluster().getMaster().getAssignmentManager().getRegionStateStore(); + regionStateStore.updateRegionReplicas(tableName, 3, 1); + Get get = new Get(primary.getRegionName()); + Result result = meta.get(get); + for (int replicaId = 0; replicaId < 3; replicaId++) { + Cell serverCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, + MetaTableAccessor.getServerColumn(replicaId)); + Cell startCodeCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, + MetaTableAccessor.getStartCodeColumn(replicaId)); + Cell stateCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, + MetaTableAccessor.getRegionStateColumn(replicaId)); + Cell snCell = result.getColumnLatestCell(HConstants.CATALOG_FAMILY, + MetaTableAccessor.getServerNameColumn(replicaId)); + if (replicaId == 0) { + assertNotNull(stateCell); + } else { + assertNull(serverCell); + assertNull(startCodeCell); + assertNull(stateCell); + assertNull(snCell); + } + } + } + } }