[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…

[GitBox <gi...@apache.org>]

ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r453024239



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
     }
   }
 
-  private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+  private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+    throws IOException, ProcedureSuspendedException {
     ServerName loc = regionNode.getRegionLocation();
     if (loc == null) {
       LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
         RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
       setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
       return;
     }
+    final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+    final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+    if (!isMeta && !isMetaAvailable) {
+      // meta is not assigned yet, so yield
+      throw new ProcedureSuspendedException();

Review comment:
       Hmm maybe. This part of the code is not very chatty, and this is kind of an expected scenario. Maybe DEBUG level.

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
     }
   }
 
-  private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+  private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+    throws IOException, ProcedureSuspendedException {
     ServerName loc = regionNode.getRegionLocation();
     if (loc == null) {
       LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
         RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
       setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
       return;
     }
+    final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+    final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+    if (!isMeta && !isMetaAvailable) {

Review comment:
       After more reading, I think this approach is wrong. There's already a facility for waiting on meta to be available. My ITBLL experience indicates it doesn't go far enough. I found:
   
   ```
     @Override
     protected boolean waitInitialized(MasterProcedureEnv env) {
       if (TableName.isMetaTableName(getTableName())) {
         return false;
       }
       // First we need meta to be loaded, and second, if meta is not online then we will likely to
       // fail when updating meta so we wait until it is assigned.
       AssignmentManager am = env.getAssignmentManager();
       return am.waitMetaLoaded(this) || am.waitMetaAssigned(this, getRegion());
     }
   ```
   
   https://github.com/apache/hbase/blob/809a623/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java#L168-L176
   
   Meta has already been assigned by the time this code executes, it's simply not coming online. Let me poke around some more on the region server side, see if I can make sense from that angle.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org