HBASE-23055 Alter hbase:meta (#667)

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
     For table state, go to master rather than go to meta direct. Going
     to meta won't work for hbase;meta state. Puts load on Master.

    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.

 Signed-off-by: Ramkrishna <[email protected]>

hbase-client/src/main/java/org/apache/hadoop/hbase/AsyncMetaTableAccessor.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
@@ -616,4 +616,5 @@ private static int parseReplicaIdFromServerColumn(byte[] serverColumn) {
     }
     return -1;
   }
+
 }

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

@@ -303,11 +303,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 */
@@ -1077,7 +1084,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());
@@ -1105,7 +1112,8 @@ public static Map<TableName, TableState> 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
    */

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

@@ -90,6 +90,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.snapshot.ClientSnapshotDescriptionUtils;
 import org.apache.hadoop.hbase.snapshot.RestoreSnapshotException;
 import org.apache.hadoop.hbase.snapshot.SnapshotCreationException;
@@ -192,6 +193,8 @@
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetTableDescriptorsResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetTableNamesRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetTableNamesResponse;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetTableStateRequest;
+import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.GetTableStateResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsBalancerEnabledRequest;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsBalancerEnabledResponse;
 import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos.IsCatalogJanitorEnabledRequest;
@@ -666,42 +669,25 @@ public CompletableFuture<Void> disableTable(TableName tableName) {
 
   @Override
   public CompletableFuture<Boolean> isTableEnabled(TableName tableName) {
-    if (TableName.isMetaTableName(tableName)) {
-      return CompletableFuture.completedFuture(true);
-    }
-    CompletableFuture<Boolean> future = new CompletableFuture<>();
-    addListener(AsyncMetaTableAccessor.getTableState(metaTable, tableName), (state, error) -> {
-      if (error != null) {
-        future.completeExceptionally(error);
-        return;
-      }
-      if (state.isPresent()) {
-        future.complete(state.get().inStates(TableState.State.ENABLED));
-      } else {
-        future.completeExceptionally(new TableNotFoundException(tableName));
-      }
-    });
-    return future;
+    return isTableState(tableName, TableState.State.ENABLED);
   }
 
   @Override
   public CompletableFuture<Boolean> isTableDisabled(TableName tableName) {
-    if (TableName.isMetaTableName(tableName)) {
-      return CompletableFuture.completedFuture(false);
-    }
-    CompletableFuture<Boolean> future = new CompletableFuture<>();
-    addListener(AsyncMetaTableAccessor.getTableState(metaTable, tableName), (state, error) -> {
-      if (error != null) {
-        future.completeExceptionally(error);
-        return;
-      }
-      if (state.isPresent()) {
-        future.complete(state.get().inStates(TableState.State.DISABLED));
-      } else {
-        future.completeExceptionally(new TableNotFoundException(tableName));
-      }
-    });
-    return future;
+    return isTableState(tableName, TableState.State.DISABLED);
+  }
+
+  /**
+   * @return Future that calls Master getTableState and compares to <code>state</code>
+   */
+  private CompletableFuture<Boolean> isTableState(TableName tableName, TableState.State state) {
+    return this.<Boolean> newMasterCaller().
+        action((controller, stub) ->
+      this.<GetTableStateRequest, GetTableStateResponse, Boolean> call(controller, stub,
+          GetTableStateRequest.newBuilder().
+              setTableName(ProtobufUtil.toProtoTableName(tableName)).build(),
+        (s, c, req, done) -> s.getTableState(c, req, done),
+        resp -> resp.getTableState().getState().toString().equals(state.toString()))).call();
   }
 
   @Override

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<RegionLocations> future,
           }
           Pair<RegionState.State, ServerName> 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<RegionLocations> 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<RegionState.State, ServerName> stateAndServerName = getStateAndServerName(proto);
@@ -197,9 +198,8 @@ private void getMetaRegionLocation(CompletableFuture<RegionLocations> future,
   public CompletableFuture<RegionLocations> getMetaRegionLocation() {
     CompletableFuture<RegionLocations> 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);

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<Integer, String> 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<Integer, String> 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<String> 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);
   }
 
   /**

hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java

@@ -1175,8 +1175,7 @@ public enum OperationStatusCode {
   /** Directories that are not HBase user table directories */
   public static final List<String> HBASE_NON_USER_TABLE_DIRS =
     Collections.unmodifiableList(Arrays.asList((String[])ArrayUtils.addAll(
-      new String[] { TableName.META_TABLE_NAME.getNameAsString() },
-      HBASE_NON_TABLE_DIRS.toArray())));
+       HBASE_NON_TABLE_DIRS.toArray())));
 
   /** Health script related settings. */
   public static final String HEALTH_SCRIPT_LOC = "hbase.node.health.script.location";

hbase-server/src/main/java/org/apache/hadoop/hbase/TableDescriptors.java

@@ -25,51 +25,39 @@
 
 /**
  * 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<String, TableDescriptor> getByNamespace(String name)
-  throws IOException;
+  Map<String, TableDescriptor> getByNamespace(String name) throws IOException;
 
   /**
    * Get Map of all TableDescriptors. Populates the descriptor cache as a
    * side effect.
    * 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<String, TableDescriptor> 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

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

@@ -49,7 +49,6 @@
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.function.Function;
 import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import javax.servlet.ServletException;
@@ -670,10 +669,6 @@ private int putUpJettyServer() throws IOException {
     return connector.getLocalPort();
   }
 
-  @Override
-  protected Function<TableDescriptorBuilder, TableDescriptorBuilder> getMetaTableObserver() {
-    return builder -> builder.setRegionReplication(conf.getInt(HConstants.META_REPLICAS_NUM, HConstants.DEFAULT_META_REPLICA_NUM));
-  }
   /**
    * For compatibility, if failed with regionserver credentials, try the master one
    */
@@ -1035,7 +1030,7 @@ private void finishActiveMasterInitialization(MonitoredTask status) throws IOExc
     RegionState rs = this.assignmentManager.getRegionStates().
         getRegionState(RegionInfoBuilder.FIRST_META_REGIONINFO);
     LOG.info("hbase:meta {}", rs);
-    if (rs.isOffline()) {
+    if (rs != null && rs.isOffline()) {
       Optional<InitMetaProcedure> optProc = procedureExecutor.getProcedures().stream()
         .filter(p -> p instanceof InitMetaProcedure).map(o -> (InitMetaProcedure) o).findAny();
       initMetaProc = optProc.orElseGet(() -> {