Apache9 commented on a change in pull request #2569: URL: https://github.com/apache/hbase/pull/2569#discussion_r508577459
########## File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DeleteTableProcedure.java ########## @@ -100,6 +101,12 @@ protected Flow executeFromState(final MasterProcedureEnv env, DeleteTableState s // TODO: Move out... in the acquireLock() LOG.debug("Waiting for RIT for {}", this); regions = env.getAssignmentManager().getRegionStates().getRegionsOfTable(getTableName()); + // We need to archive SPLIT regions as well, so add the SPLIT regions to the region list + List<RegionInfo> splitRegions = env.getAssignmentManager().getRegionStates() Review comment: I think here we could just get all the regions of the table. And look at the code of getRegionsOfTable(TableName, boolean), I'm afraid the implementation of the include method has something wrong? ``` if (node.isInState(State.SPLIT) || hri.isSplit()) { return false; } if ((node.isInState(State.OFFLINE) || hri.isOffline()) && !offline) { return false; } return (!hri.isOffline() && !hri.isSplit()) || ((hri.isOffline() || hri.isSplit()) && offline); ``` I do not think we need the two extra if conditions? Especially the first one, it will make us always exclude split parent? ---------------------------------------------------------------- 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: us...@infra.apache.org