From 487c72499ff146218b0b04439f58ed4c6d5a4a04 Mon Sep 17 00:00:00 2001 From: stack Date: Thu, 19 Sep 2019 16:01:34 -0700 Subject: [PATCH] HBASE-23055 Alter hbase:meta Make it so hbase:meta can be altered. TableState for hbase:meta is kept in Master. State is in-memory transient so if Master fails, hbase:meta is ENABLED again. hbase:meta schema will be bootstrapped from the filesystem. Changes to filesystem schema are atomic so we should be ok if Master fails mid-edit (TBD) Undoes a bunch of guards that prevented our being able to edit hbase:meta. At minimmum, need to add in a bunch of WARNING. TODO: Tests, more clarity around hbase:meta table state, and undoing references to hard-coded hbase:meta regioninfo. M hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java Throw illegal access exception if you try to use MetaTableAccessor getting state of the hbase:meta table. M hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java TODO: getTableState needs work in Connection implemetnations. Presumes state is in meta table for all tables. Uses MetaTableAccessor. TODO: More cleanup in here and more cleanup in async versions. M hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java Change isTableDisabled/Enabled implementation to ask the Master instead. This will give the Master's TableStateManager's opinion rather than client figuring it for themselves reading meta table direct. M hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java TODO: Cleanup in here. Go to master for state, not to meta. M hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java Logging cleanup. M hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java Shutdown access. M hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java Just cleanup. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java Add state holder for hbase:meta. Removed unused methods. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java Shut down access. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java Allow hbase:meta to be disabled. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java Allow hbase:meta to be enabled. --- .../hadoop/hbase/MetaTableAccessor.java | 20 +++- .../client/ConnectionImplementation.java | 6 +- .../hadoop/hbase/client/HBaseAdmin.java | 31 +++-- .../client/MasterKeepAliveConnection.java | 6 +- .../hbase/client/RawAsyncHBaseAdmin.java | 6 +- .../hadoop/hbase/client/ZKAsyncRegistry.java | 12 +- .../hadoop/hbase/zookeeper/ZNodePaths.java | 59 +++++----- .../org/apache/hadoop/hbase/HConstants.java | 6 - .../apache/hadoop/hbase/TableDescriptors.java | 20 +--- .../hbase/master/TableStateManager.java | 105 +++++------------ .../master/assignment/RegionStateStore.java | 3 +- .../procedure/DisableTableProcedure.java | 9 +- .../procedure/EnableTableProcedure.java | 109 +++++++++--------- .../master/zksyncer/MetaLocationSyncer.java | 6 +- .../hadoop/hbase/util/FSTableDescriptors.java | 14 --- .../hbase/zookeeper/MetaTableLocator.java | 18 +-- .../apache/hadoop/hbase/zookeeper/ZKUtil.java | 2 +- 17 files changed, 187 insertions(+), 245 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 8d61f9903887..794504865a27 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 @@ -304,11 +304,18 @@ public static HRegionLocation getRegionLocation(Connection connection, byte[] re */ public static HRegionLocation getRegionLocation(Connection connection, RegionInfo regionInfo) throws IOException { - byte[] row = getMetaKeyForRegion(regionInfo); - Get get = new Get(row); + return getRegionLocation(getCatalogFamilyRow(connection, regionInfo), + regionInfo, regionInfo.getReplicaId()); + } + + /** + * @return Return the {@link HConstants#CATALOG_FAMILY} row from hbase:meta. + */ + public static Result getCatalogFamilyRow(Connection connection, RegionInfo ri) + throws IOException { + Get get = new Get(getMetaKeyForRegion(ri)); get.addFamily(HConstants.CATALOG_FAMILY); - Result r = get(getMetaHTable(connection), get); - return getRegionLocation(r, regionInfo, regionInfo.getReplicaId()); + return get(getMetaHTable(connection), get); } /** Returns the row key to use for this regionInfo */ @@ -1110,7 +1117,7 @@ public static RegionInfo getRegionInfo(final Result r, byte [] qualifier) { public static TableState getTableState(Connection conn, TableName tableName) throws IOException { if (tableName.equals(TableName.META_TABLE_NAME)) { - return new TableState(tableName, TableState.State.ENABLED); + throw new IllegalAccessError("Go to the Master to find hbase:meta table state, not here"); } Table metaHTable = getMetaHTable(conn); Get get = new Get(tableName.getName()).addColumn(getTableFamily(), getTableStateColumn()); @@ -1138,7 +1145,8 @@ public static Map getTableStates(Connection conn) } /** - * Updates state in META + * Updates state in META. + * Do not use. For internal use only. * @param conn connection to use * @param tableName table to look for */ diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java index 39f7abfdac53..e058b4fca44a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ConnectionImplementation.java @@ -1,4 +1,4 @@ -/** +/* * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -45,6 +45,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.ReentrantLock; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.AuthUtil; import org.apache.hadoop.hbase.CallQueueTooBigException; @@ -2057,6 +2058,9 @@ public NonceGenerator getNonceGenerator() { @Override public TableState getTableState(TableName tableName) throws IOException { + // TODO: This doesn't work if tablename is hbase:meta. Need to ask Master. + // Other problems with this implementation are that it presumes state is + // available in Master. Would be good to hide how state is kept. checkClosed(); TableState tableState = MetaTableAccessor.getTableState(this, tableName); if (tableState == null) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java index 0b2be1955815..925a4bf78132 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java @@ -91,6 +91,7 @@ import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.security.access.ShadedAccessControlUtil; import org.apache.hadoop.hbase.security.access.UserPermission; +import org.apache.hadoop.hbase.shaded.protobuf.ResponseConverter; import org.apache.hadoop.hbase.snapshot.ClientSnapshotDescriptionUtils; import org.apache.hadoop.hbase.snapshot.HBaseSnapshotException; import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException; @@ -948,14 +949,18 @@ public HTableDescriptor[] disableTables(Pattern pattern) throws IOException { @Override public boolean isTableEnabled(final TableName tableName) throws IOException { checkTableExists(tableName); - return executeCallable(new RpcRetryingCallable() { + // Go to the Master. It knows state of all tables. + return executeCallable(new MasterCallable(getConnection(), + getRpcControllerFactory()) { @Override - protected Boolean rpcCall(int callTimeout) throws Exception { - TableState tableState = MetaTableAccessor.getTableState(getConnection(), tableName); - if (tableState == null) { + protected Boolean rpcCall() throws Exception { + setPriority(tableName); + MasterProtos.GetTableStateRequest req = RequestConverter.buildGetTableStateRequest(tableName); + MasterProtos.GetTableStateResponse ret = master.getTableState(getRpcController(), req); + if (!ret.hasTableState() || ret.getTableState() == null) { throw new TableNotFoundException(tableName); } - return tableState.inStates(TableState.State.ENABLED); + return ret.getTableState().getState() == HBaseProtos.TableState.State.ENABLED; } }); } @@ -963,7 +968,20 @@ protected Boolean rpcCall(int callTimeout) throws Exception { @Override public boolean isTableDisabled(TableName tableName) throws IOException { checkTableExists(tableName); - return connection.isTableDisabled(tableName); + // Go to the Master. It knows state of all tables. + return executeCallable(new MasterCallable(getConnection(), + getRpcControllerFactory()) { + @Override + protected Boolean rpcCall() throws Exception { + setPriority(tableName); + MasterProtos.GetTableStateRequest req = RequestConverter.buildGetTableStateRequest(tableName); + MasterProtos.GetTableStateResponse ret = master.getTableState(getRpcController(), req); + if (!ret.hasTableState() || ret.getTableState() == null) { + throw new TableNotFoundException(tableName); + } + return ret.getTableState().getState() == HBaseProtos.TableState.State.DISABLED; + } + }); } @Override @@ -4357,5 +4375,4 @@ protected Boolean rpcCall() throws Exception { }); } - } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterKeepAliveConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterKeepAliveConnection.java index b1c37776f9e4..a5427c7f0fd1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterKeepAliveConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MasterKeepAliveConnection.java @@ -1,4 +1,4 @@ -/** +/* * Copyright The Apache Software Foundation * * Licensed to the Apache Software Foundation (ASF) under one @@ -23,6 +23,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos; import org.apache.yetus.audience.InterfaceAudience; +import java.io.Closeable; + /** * A KeepAlive connection is not physically closed immediately after the close, * but rather kept alive for a few minutes. It makes sense only if it is shared. @@ -35,7 +37,7 @@ * final user code. Hence it's package protected. */ @InterfaceAudience.Private -interface MasterKeepAliveConnection extends MasterProtos.MasterService.BlockingInterface { +interface MasterKeepAliveConnection extends MasterProtos.MasterService.BlockingInterface, Closeable { // Do this instead of implement Closeable because closeable returning IOE is PITA. void close(); } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index 6a0099f9e2e4..7950fed47955 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java @@ -663,9 +663,9 @@ public CompletableFuture disableTable(TableName tableName) { @Override public CompletableFuture isTableEnabled(TableName tableName) { - if (TableName.isMetaTableName(tableName)) { - return CompletableFuture.completedFuture(true); - } + // TODO: This doesn't work if tablename is hbase:meta. Need to ask Master. + // Other problems with this implementation are that it presumes state is + // available in Master. Would be good to hide how state is kept. CompletableFuture future = new CompletableFuture<>(); addListener(AsyncMetaTableAccessor.getTableState(metaTable, tableName), (state, error) -> { if (error != null) { diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java index 36fa6bba7544..0a021aa0f720 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/ZKAsyncRegistry.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 @@ -158,7 +158,8 @@ private void getMetaRegionLocation(CompletableFuture future, } Pair stateAndServerName = getStateAndServerName(proto); if (stateAndServerName.getFirst() != RegionState.State.OPEN) { - LOG.warn("Meta region is in state " + stateAndServerName.getFirst()); + LOG.warn("hbase:meta region (replicaId={}) is in state {}", replicaId, + stateAndServerName.getFirst()); } locs[DEFAULT_REPLICA_ID] = new HRegionLocation( getRegionInfoForDefaultReplica(FIRST_META_REGIONINFO), stateAndServerName.getSecond()); @@ -173,7 +174,7 @@ private void getMetaRegionLocation(CompletableFuture future, LOG.warn("Failed to fetch " + path, error); locs[replicaId] = null; } else if (proto == null) { - LOG.warn("Meta znode for replica " + replicaId + " is null"); + LOG.warn("hbase:meta znode for replica " + replicaId + " is null"); locs[replicaId] = null; } else { Pair stateAndServerName = getStateAndServerName(proto); @@ -197,9 +198,8 @@ private void getMetaRegionLocation(CompletableFuture future, public CompletableFuture getMetaRegionLocation() { CompletableFuture future = new CompletableFuture<>(); addListener( - zk.list(znodePaths.baseZNode) - .thenApply(children -> children.stream() - .filter(c -> c.startsWith(znodePaths.metaZNodePrefix)).collect(Collectors.toList())), + zk.list(znodePaths.baseZNode).thenApply(children -> children.stream(). + filter(c -> znodePaths.isMetaZNodePrefix(c)).collect(Collectors.toList())), (metaReplicaZNodes, error) -> { if (error != null) { future.completeExceptionally(error); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java index c5e510fe4b9c..cfa0e82ea0e4 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZNodePaths.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 @@ -24,6 +24,7 @@ import static org.apache.hadoop.hbase.HConstants.ZOOKEEPER_ZNODE_PARENT; import static org.apache.hadoop.hbase.client.RegionInfo.DEFAULT_REPLICA_ID; +import java.util.Collection; import java.util.Optional; import java.util.stream.IntStream; import org.apache.hadoop.conf.Configuration; @@ -40,15 +41,24 @@ public class ZNodePaths { // TODO: Replace this with ZooKeeper constant when ZOOKEEPER-277 is resolved. public static final char ZNODE_PATH_SEPARATOR = '/'; - public final static String META_ZNODE_PREFIX = "meta-region-server"; + private static final String META_ZNODE_PREFIX = "meta-region-server"; private static final String DEFAULT_SNAPSHOT_CLEANUP_ZNODE = "snapshot-cleanup"; // base znode for this cluster public final String baseZNode; - // the prefix of meta znode, does not include baseZNode. - public final String metaZNodePrefix; - // znodes containing the locations of the servers hosting the meta replicas - public final ImmutableMap metaReplicaZNodes; + + /** + * The prefix of meta znode. Does not include baseZNode. + * Its a 'prefix' because meta replica id integer can be tagged on the end (if + * no number present, it is 'default' replica). + */ + private final String metaZNodePrefix; + + /** + * znodes containing the locations of the servers hosting the meta replicas + */ + private final ImmutableMap metaReplicaZNodes; + // znode containing ephemeral nodes of the regionservers public final String rsZNode; // znode containing ephemeral nodes of the draining regionservers @@ -158,21 +168,21 @@ public String toString() { } /** - * Is the znode of any meta replica - * @param node - * @return true or false + * @return true if the znode is a meta region replica */ public boolean isAnyMetaReplicaZNode(String node) { - if (metaReplicaZNodes.containsValue(node)) { - return true; - } - return false; + return this.metaReplicaZNodes.containsValue(node); + } + + /** + * @return Meta Replica ZNodes + */ + public Collection getMetaReplicaZNodes() { + return this.metaReplicaZNodes.values(); } /** - * Get the znode string corresponding to a replicaId - * @param replicaId - * @return znode + * @return the znode string corresponding to a replicaId */ public String getZNodeForReplica(int replicaId) { // return a newly created path but don't update the cache of paths @@ -183,24 +193,21 @@ public String getZNodeForReplica(int replicaId) { } /** - * Parse the meta replicaId from the passed znode + * Parse the meta replicaId from the passed znode name. * @param znode the name of the znode, does not include baseZNode * @return replicaId */ public int getMetaReplicaIdFromZnode(String znode) { - if (znode.equals(metaZNodePrefix)) { - return RegionInfo.DEFAULT_REPLICA_ID; - } - return Integer.parseInt(znode.substring(metaZNodePrefix.length() + 1)); + return znode.equals(metaZNodePrefix)? + RegionInfo.DEFAULT_REPLICA_ID: + Integer.parseInt(znode.substring(metaZNodePrefix.length() + 1)); } /** - * Is it the default meta replica's znode - * @param znode the name of the znode, does not include baseZNode - * @return true or false + * @return True if meta znode. */ - public boolean isDefaultMetaReplicaZnode(String znode) { - return metaReplicaZNodes.get(DEFAULT_REPLICA_ID).equals(znode); + public boolean isMetaZNodePrefix(String znode) { + return znode != null && znode.startsWith(this.metaZNodePrefix); } /** 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 209dec411247..97b63ddb6bed 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 @@ -1208,12 +1208,6 @@ public enum OperationStatusCode { HBCK_SIDELINEDIR_NAME, HBASE_TEMP_DIRECTORY, MIGRATION_NAME })); - /** Directories that are not HBase user table directories */ - public static final List HBASE_NON_USER_TABLE_DIRS = - Collections.unmodifiableList(Arrays.asList((String[])ArrayUtils.addAll( - new String[] { TableName.META_TABLE_NAME.getNameAsString() }, - HBASE_NON_TABLE_DIRS.toArray()))); - /** Health script related settings. */ public static final String HEALTH_SCRIPT_LOC = "hbase.node.health.script.location"; public static final String HEALTH_SCRIPT_TIMEOUT = "hbase.node.health.script.timeout"; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java index 2537e7f83e79..e0a9eabfa629 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java @@ -25,25 +25,19 @@ /** * Get, remove and modify table descriptors. - * Used by servers to host descriptors. */ @InterfaceAudience.Private public interface TableDescriptors { /** - * @param tableName * @return TableDescriptor for tablename - * @throws IOException */ - TableDescriptor get(final TableName tableName) - throws IOException; + TableDescriptor get(final TableName tableName) throws IOException; /** * Get Map of all NamespaceDescriptors for a given namespace. * @return Map of all descriptors. - * @throws IOException */ - Map getByNamespace(String name) - throws IOException; + Map getByNamespace(String name) throws IOException; /** * Get Map of all TableDescriptors. Populates the descriptor cache as a @@ -51,25 +45,19 @@ Map getByNamespace(String name) * Notice: the key of map is the table name which contains namespace. It was generated by * {@link TableName#getNameWithNamespaceInclAsString()}. * @return Map of all descriptors. - * @throws IOException */ Map getAll() throws IOException; /** * Add or update descriptor * @param htd Descriptor to set into TableDescriptors - * @throws IOException */ - void add(final TableDescriptor htd) - throws IOException; + void add(final TableDescriptor htd) throws IOException; /** - * @param tablename * @return Instance of table descriptor or null if none found. - * @throws IOException */ - TableDescriptor remove(final TableName tablename) - throws IOException; + TableDescriptor remove(final TableName tablename) throws IOException; /** * Enables the tabledescriptor cache diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java index 1eb041692082..b6ba805c16a5 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableStateManager.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 @@ -34,7 +34,6 @@ import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.client.TableState; -import org.apache.hadoop.hbase.exceptions.IllegalArgumentIOException; import org.apache.hadoop.hbase.util.IdReadWriteLock; import org.apache.hadoop.hbase.util.ZKDataMigrator; import org.apache.hadoop.hbase.zookeeper.ZKUtil; @@ -53,8 +52,20 @@ // TODO: Make this a guava Service @InterfaceAudience.Private public class TableStateManager { - private static final Logger LOG = LoggerFactory.getLogger(TableStateManager.class); + + /** + * All table state is kept in hbase:meta except that of hbase:meta itself. + * hbase:meta state is kept here locally in this in-memory variable. State + * for hbase:meta is not persistent. If this process dies, the hbase:meta + * state reverts to enabled. State is used so we can edit hbase:meta as we + * would any other table by disabling, altering, and then re-enabling. If this + * process dies in the midst of an edit, the table reverts to enabled. Schema + * is read from the filesystem. It is changed atomically so if we die midway + * through an edit we should be good. + */ + private TableState.State metaTableState = TableState.State.ENABLED; + /** * Set this key to false in Configuration to disable migrating table state from zookeeper so * hbase:meta table. @@ -68,7 +79,7 @@ public class TableStateManager { private final ConcurrentMap tableName2State = new ConcurrentHashMap<>(); - public TableStateManager(MasterServices master) { + TableStateManager(MasterServices master) { this.master = master; } @@ -87,61 +98,6 @@ public void setTableState(TableName tableName, TableState.State newState) throws } } - /** - * Set table state to provided but only if table in specified states Caller should lock table on - * write. - * @param tableName table to change state for - * @param newState new state - * @param states states to check against - * @return null if succeed or table state if failed - */ - public TableState setTableStateIfInStates(TableName tableName, TableState.State newState, - TableState.State... states) throws IOException { - ReadWriteLock lock = tnLock.getLock(tableName); - lock.writeLock().lock(); - try { - TableState currentState = readMetaState(tableName); - if (currentState == null) { - throw new TableNotFoundException(tableName); - } - if (currentState.inStates(states)) { - updateMetaState(tableName, newState); - return null; - } else { - return currentState; - } - } finally { - lock.writeLock().unlock(); - } - } - - /** - * Set table state to provided but only if table not in specified states Caller should lock table - * on write. - * @param tableName table to change state for - * @param newState new state - * @param states states to check against - */ - public boolean setTableStateIfNotInStates(TableName tableName, TableState.State newState, - TableState.State... states) throws IOException { - ReadWriteLock lock = tnLock.getLock(tableName); - lock.writeLock().lock(); - try { - TableState currentState = readMetaState(tableName); - if (currentState == null) { - throw new TableNotFoundException(tableName); - } - if (!currentState.inStates(states)) { - updateMetaState(tableName, newState); - return true; - } else { - return false; - } - } finally { - lock.writeLock().unlock(); - } - } - public boolean isTableState(TableName tableName, TableState.State... states) { try { TableState tableState = getTableState(tableName); @@ -155,6 +111,7 @@ public boolean isTableState(TableName tableName, TableState.State... states) { public void setDeletedTable(TableName tableName) throws IOException { if (tableName.equals(TableName.META_TABLE_NAME)) { + // Can't delete the hbase:meta table. return; } ReadWriteLock lock = tnLock.getLock(tableName); @@ -183,7 +140,7 @@ public boolean isTablePresent(TableName tableName) throws IOException { * @param states filter by states * @return tables in given states */ - public Set getTablesInStates(TableState.State... states) throws IOException { + Set getTablesInStates(TableState.State... states) throws IOException { // Only be called in region normalizer, will not use cache. final Set rv = Sets.newHashSet(); MetaTableAccessor.fullScanTables(master.getConnection(), new MetaTableAccessor.Visitor() { @@ -221,22 +178,18 @@ public TableState getTableState(TableName tableName) throws IOException { } private void updateMetaState(TableName tableName, TableState.State newState) throws IOException { - if (tableName.equals(TableName.META_TABLE_NAME)) { - if (TableState.State.DISABLING.equals(newState) || - TableState.State.DISABLED.equals(newState)) { - throw new IllegalArgumentIOException("Cannot disable the meta table; " + newState); - } - // Otherwise, just return; no need to set ENABLED on meta -- it is always ENABLED. - return; - } boolean succ = false; try { - MetaTableAccessor.updateTableState(master.getConnection(), tableName, newState); - tableName2State.put(tableName, newState); + if (tableName.equals(TableName.META_TABLE_NAME)) { + this.metaTableState = newState; + } else { + MetaTableAccessor.updateTableState(master.getConnection(), tableName, newState); + } + this.tableName2State.put(tableName, newState); succ = true; } finally { if (!succ) { - tableName2State.remove(tableName); + this.tableName2State.remove(tableName); } } metaStateUpdated(tableName, newState); @@ -255,7 +208,9 @@ private TableState readMetaState(TableName tableName) throws IOException { if (state != null) { return new TableState(tableName, state); } - TableState tableState = MetaTableAccessor.getTableState(master.getConnection(), tableName); + TableState tableState = tableName.equals(TableName.META_TABLE_NAME)? + new TableState(TableName.META_TABLE_NAME, this.metaTableState): + MetaTableAccessor.getTableState(master.getConnection(), tableName); if (tableState != null) { tableName2State.putIfAbsent(tableName, tableState.getState()); } @@ -263,10 +218,8 @@ private TableState readMetaState(TableName tableName) throws IOException { } public void start() throws IOException { - TableDescriptors tableDescriptors = master.getTableDescriptors(); migrateZooKeeper(); - Connection connection = master.getConnection(); - fixTableStates(tableDescriptors, connection); + fixTableStates(master.getTableDescriptors(), master.getConnection()); } private void fixTableStates(TableDescriptors tableDescriptors, Connection connection) @@ -377,4 +330,4 @@ protected void deleteZooKeeper(TableName tableName) { LOG.warn("Failed deleting table state from zookeeper", e); } } -} +} \ No newline at end of file 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 5a8e7dd9381f..5cd38cfc5a2b 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 @@ -147,8 +147,7 @@ private void visitMetaEntry(final RegionStateVisitor visitor, final Result resul } } - public void updateRegionLocation(RegionStateNode regionStateNode) - throws IOException { + void updateRegionLocation(RegionStateNode regionStateNode) throws IOException { if (regionStateNode.getRegionInfo().isMetaRegion()) { updateMetaLocation(regionStateNode.getRegionInfo(), regionStateNode.getRegionLocation(), regionStateNode.getState()); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java index 18c194f3287a..3cceb19a037e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java @@ -109,8 +109,8 @@ protected Flow executeFromState(final MasterProcedureEnv env, final DisableTable setNextState(DisableTableState.DISABLE_TABLE_ADD_REPLICATION_BARRIER); break; case DISABLE_TABLE_ADD_REPLICATION_BARRIER: - if (env.getMasterServices().getTableDescriptors().get(tableName) - .hasGlobalReplicationScope()) { + if (env.getMasterServices().getTableDescriptors().get(tableName). + hasGlobalReplicationScope()) { MasterFileSystem fs = env.getMasterFileSystem(); try (BufferedMutator mutator = env.getMasterServices().getConnection() .getBufferedMutator(TableName.META_TABLE_NAME)) { @@ -242,10 +242,7 @@ public TableOperationType getTableOperationType() { */ private boolean prepareDisable(final MasterProcedureEnv env) throws IOException { boolean canTableBeDisabled = true; - if (tableName.equals(TableName.META_TABLE_NAME)) { - setFailure("master-disable-table", new ConstraintException("Cannot disable catalog table")); - canTableBeDisabled = false; - } else if (!MetaTableAccessor.tableExists(env.getMasterServices().getConnection(), tableName)) { + if (!MetaTableAccessor.tableExists(env.getMasterServices().getConnection(), tableName)) { setFailure("master-disable-table", new TableNotFoundException(tableName)); canTableBeDisabled = false; } else if (!skipTableStateCheck) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java index 06d6a2cfc525..6e421b6d381d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.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 @@ -27,11 +27,9 @@ 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.Get; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Result; -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; @@ -99,66 +97,55 @@ protected Flow executeFromState(final MasterProcedureEnv env, final EnableTableS setNextState(EnableTableState.ENABLE_TABLE_MARK_REGIONS_ONLINE); break; case ENABLE_TABLE_MARK_REGIONS_ONLINE: + // Get the region replica count. If changed since disable, need to do + // more work assigning. Connection connection = env.getMasterServices().getConnection(); - // we will need to get the tableDescriptor here to see if there is a change in the replica - // count - TableDescriptor hTableDescriptor = + TableDescriptor tableDescriptor = env.getMasterServices().getTableDescriptors().get(tableName); - - // Get the replica count - int regionReplicaCount = hTableDescriptor.getRegionReplication(); - - // Get the regions for the table from memory; get both online and offline regions - // ('true'). + int configuredReplicaCount = tableDescriptor.getRegionReplication(); + // Get regions for the table from memory; get both online and offline regions ('true'). List regionsOfTable = env.getAssignmentManager().getRegionStates().getRegionsOfTable(tableName, true); - int currentMaxReplica = 0; - // Check if the regions in memory have replica regions as marked in META table - for (RegionInfo regionInfo : regionsOfTable) { - if (regionInfo.getReplicaId() > currentMaxReplica) { - // Iterating through all the list to identify the highest replicaID region. - // We can stop after checking with the first set of regions?? - currentMaxReplica = regionInfo.getReplicaId(); - } - } - - // read the META table to know the actual number of replicas for the table - if there - // was a table modification on region replica then this will reflect the new entries also - int replicasFound = - getNumberOfReplicasFromMeta(connection, regionReplicaCount, regionsOfTable); - assert regionReplicaCount - 1 == replicasFound; - LOG.info(replicasFound + " META entries added for the given regionReplicaCount " - + regionReplicaCount + " for the table " + tableName.getNameAsString()); - if (currentMaxReplica == (regionReplicaCount - 1)) { + // How many replicas do we currently have? Check regions returned from + // in-memory state. + int currentMaxReplica = getMaxReplicaId(regionsOfTable); + + // Read the META table to know the number of replicas the table currently has. + // If there was a table modification on region replica count then need to + // adjust replica counts here. + int replicasFound = TableName.isMetaTableName(this.tableName)? + 0: // TODO: Figure better what to do here for hbase:meta replica. + getReplicaCountInMeta(connection, configuredReplicaCount, regionsOfTable); + LOG.info("replicasFound={} (configuredReplicaCount={} for {}", replicasFound, + configuredReplicaCount, tableName.getNameAsString()); + if (currentMaxReplica == (configuredReplicaCount - 1)) { if (LOG.isDebugEnabled()) { - LOG.debug("There is no change to the number of region replicas." - + " Assigning the available regions." + " Current and previous" - + "replica count is " + regionReplicaCount); + LOG.debug("No change in number of region replicas (configuredReplicaCount={});" + + " assigning.", configuredReplicaCount); } - } else if (currentMaxReplica > (regionReplicaCount - 1)) { - // we have additional regions as the replica count has been decreased. Delete + } else if (currentMaxReplica > (configuredReplicaCount - 1)) { + // We have additional regions as the replica count has been decreased. Delete // those regions because already the table is in the unassigned state LOG.info("The number of replicas " + (currentMaxReplica + 1) - + " is more than the region replica count " + regionReplicaCount); + + " is more than the region replica count " + configuredReplicaCount); List copyOfRegions = new ArrayList(regionsOfTable); for (RegionInfo regionInfo : copyOfRegions) { - if (regionInfo.getReplicaId() > (regionReplicaCount - 1)) { + if (regionInfo.getReplicaId() > (configuredReplicaCount - 1)) { // delete the region from the regionStates env.getAssignmentManager().getRegionStates().deleteRegion(regionInfo); // remove it from the list of regions of the table - LOG.info("The regioninfo being removed is " + regionInfo + " " - + regionInfo.getReplicaId()); + LOG.info("Removed replica={} of {}", regionInfo.getRegionId(), regionInfo); regionsOfTable.remove(regionInfo); } } } else { // the replicasFound is less than the regionReplication - LOG.info("The number of replicas has been changed(increased)." - + " Lets assign the new region replicas. The previous replica count was " - + (currentMaxReplica + 1) + ". The current replica count is " + regionReplicaCount); - regionsOfTable = RegionReplicaUtil.addReplicas(hTableDescriptor, regionsOfTable, - currentMaxReplica + 1, regionReplicaCount); + LOG.info("Number of replicas has increased. Assigning new region replicas." + + "The previous replica count was {}. The current replica count is {}.", + (currentMaxReplica + 1), configuredReplicaCount); + regionsOfTable = RegionReplicaUtil.addReplicas(tableDescriptor, regionsOfTable, + currentMaxReplica + 1, configuredReplicaCount); } // Assign all the table regions. (including region replicas if added). // createAssignProcedure will try to retain old assignments if possible. @@ -186,9 +173,13 @@ protected Flow executeFromState(final MasterProcedureEnv env, final EnableTableS return Flow.HAS_MORE_STATE; } - private int getNumberOfReplicasFromMeta(Connection connection, int regionReplicaCount, + /** + * @return Count of replicas found reading hbase:meta Region row or zk if + * asking about the hbase:meta table itself.. + */ + private int getReplicaCountInMeta(Connection connection, int regionReplicaCount, List regionsOfTable) throws IOException { - Result r = getRegionFromMeta(connection, regionsOfTable); + Result r = MetaTableAccessor.getCatalogFamilyRow(connection, regionsOfTable.get(0)); int replicasFound = 0; for (int i = 1; i < regionReplicaCount; i++) { // Since we have already added the entries to the META we will be getting only that here @@ -201,16 +192,6 @@ private int getNumberOfReplicasFromMeta(Connection connection, int regionReplica return replicasFound; } - private Result getRegionFromMeta(Connection connection, List regionsOfTable) - throws IOException { - byte[] metaKeyForRegion = MetaTableAccessor.getMetaKeyForRegion(regionsOfTable.get(0)); - Get get = new Get(metaKeyForRegion); - get.addFamily(HConstants.CATALOG_FAMILY); - Table metaTable = MetaTableAccessor.getMetaHTable(connection); - Result r = metaTable.get(get); - return r; - } - @Override protected void rollbackState(final MasterProcedureEnv env, final EnableTableState state) throws IOException { @@ -408,4 +389,20 @@ private void runCoprocessorAction(final MasterProcedureEnv env, final EnableTabl } } } + + /** + * @return Maximum region replica id found in passed list of regions. + */ + private static int getMaxReplicaId(List regions) { + int max = 0; + for (RegionInfo regionInfo: regions) { + if (regionInfo.getReplicaId() > max) { + // Iterating through all the list to identify the highest replicaID region. + // We can stop after checking with the first set of regions?? + max = regionInfo.getReplicaId(); + } + } + return max; + + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/zksyncer/MetaLocationSyncer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/zksyncer/MetaLocationSyncer.java index eb80a2a232b2..98d73224ce9b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/zksyncer/MetaLocationSyncer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/zksyncer/MetaLocationSyncer.java @@ -1,4 +1,4 @@ -/** +/* * * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -41,6 +41,6 @@ boolean validate(String path) { @Override Collection getNodesToWatch() { - return watcher.getZNodePaths().metaReplicaZNodes.values(); + return watcher.getZNodePaths().getMetaReplicaZNodes(); } -} +} \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java index 004195028da0..dc6899334f0e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSTableDescriptors.java @@ -222,11 +222,6 @@ public TableDescriptor get(final TableName tablename) cachehits++; return metaTableDescriptor; } - // hbase:meta is already handled. If some one tries to get the descriptor for - // .logs, .oldlogs or .corrupt throw an exception. - if (HConstants.HBASE_NON_USER_TABLE_DIRS.contains(tablename.getNameAsString())) { - throw new IOException("No descriptor found for non table = " + tablename); - } if (usecache) { // Look in cache of descriptors. @@ -326,15 +321,6 @@ public void add(TableDescriptor htd) throws IOException { if (fsreadonly) { throw new NotImplementedException("Cannot add a table descriptor - in read only mode"); } - TableName tableName = htd.getTableName(); - if (TableName.META_TABLE_NAME.equals(tableName)) { - throw new NotImplementedException(HConstants.NOT_IMPLEMENTED); - } - if (HConstants.HBASE_NON_USER_TABLE_DIRS.contains(tableName.getNameAsString())) { - throw new NotImplementedException( - "Cannot add a table descriptor for a reserved subdirectory name: " - + htd.getTableName().getNameAsString()); - } updateTableDescriptor(htd); } diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MetaTableLocator.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MetaTableLocator.java index 0cebc762fd73..1183cd675893 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MetaTableLocator.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/MetaTableLocator.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 @@ -61,14 +61,6 @@ public final class MetaTableLocator { private MetaTableLocator() { } - /** - * Checks if the meta region location is available. - * @return true if meta region location is available, false if not - */ - public static boolean isLocationAvailable(ZKWatcher zkw) { - return getMetaRegionLocation(zkw) != null; - } - /** * @param zkw ZooKeeper watcher to be used * @return meta table regions and their locations. @@ -266,7 +258,7 @@ public static RegionState getMetaRegionState(ZKWatcher zkw) throws KeeperExcepti } /** - * Load the meta region state from the meta server ZNode. + * Load the meta region state from the meta region server ZNode. * * @param zkw reference to the {@link ZKWatcher} which also contains configuration and operation * @param replicaId the ID of the replica @@ -306,10 +298,8 @@ public static RegionState getMetaRegionState(ZKWatcher zkw, int replicaId) if (serverName == null) { state = RegionState.State.OFFLINE; } - return new RegionState( - RegionReplicaUtil.getRegionInfoForReplica( - RegionInfoBuilder.FIRST_META_REGIONINFO, replicaId), - state, serverName); + return new RegionState(RegionReplicaUtil.getRegionInfoForReplica( + RegionInfoBuilder.FIRST_META_REGIONINFO, replicaId), state, serverName); } /** diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index 878f3ca6e777..2e3e4b03b441 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -2057,7 +2057,7 @@ private static void logRetrievedMsg(final ZKWatcher zkw, " byte(s) of data from znode " + znode + (watcherSet? " and set watcher; ": "; data=") + (data == null? "null": data.length == 0? "empty": ( - znode.startsWith(zkw.getZNodePaths().metaZNodePrefix)? + zkw.getZNodePaths().isMetaZNodePrefix(znode)? getServerNameOrEmptyString(data): znode.startsWith(zkw.getZNodePaths().backupMasterAddressesZNode)? getServerNameOrEmptyString(data):