diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 0f4162cd1f74..04a475684613 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -2256,21 +2256,26 @@ public long mergeRegions(final RegionInfo[] regionsToMerge, final boolean forcib final long nonce) throws IOException { checkInitialized(); + final String regionNamesToLog = RegionInfo.getShortNameToLog(regionsToMerge); + if (!isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { - String regionsStr = Arrays.deepToString(regionsToMerge); - LOG.warn("Merge switch is off! skip merge of " + regionsStr); + LOG.warn("Merge switch is off! skip merge of " + regionNamesToLog); + throw new DoNotRetryIOException( + "Merge of " + regionNamesToLog + " failed because merge switch is off"); + } + + if (!getTableDescriptors().get(regionsToMerge[0].getTable()).isMergeEnabled()) { + LOG.warn("Merge is disabled for the table! Skipping merge of {}", regionNamesToLog); throw new DoNotRetryIOException( - "Merge of " + regionsStr + " failed because merge switch is off"); + "Merge of " + regionNamesToLog + " failed as region merge is disabled for the table"); } - final String mergeRegionsStr = Arrays.stream(regionsToMerge).map(RegionInfo::getEncodedName) - .collect(Collectors.joining(", ")); return MasterProcedureUtil.submitProcedure(new NonceProcedureRunnable(this, ng, nonce) { @Override protected void run() throws IOException { getMaster().getMasterCoprocessorHost().preMergeRegions(regionsToMerge); String aid = getClientIdAuditPrefix(); - LOG.info("{} merge regions {}", aid, mergeRegionsStr); + LOG.info("{} merge regions {}", aid, regionNamesToLog); submitProcedure(new MergeTableRegionsProcedure(procedureExecutor.getEnvironment(), regionsToMerge, forcible)); getMaster().getMasterCoprocessorHost().postMergeRegions(regionsToMerge); @@ -2294,6 +2299,12 @@ public long splitRegion(final RegionInfo regionInfo, final byte[] splitRow, fina "Split region " + regionInfo.getRegionNameAsString() + " failed due to split switch off"); } + if (!getTableDescriptors().get(regionInfo.getTable()).isSplitEnabled()) { + LOG.warn("Split is disabled for the table! Skipping split of {}", regionInfo); + throw new DoNotRetryIOException("Split region " + regionInfo.getRegionNameAsString() + + " failed as region split is disabled for the table"); + } + return MasterProcedureUtil .submitProcedure(new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) { @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java index b9a3ee13361d..201fc7321759 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java @@ -443,27 +443,28 @@ protected ProcedureMetrics getProcedureMetrics(MasterProcedureEnv env) { private boolean prepareMergeRegion(final MasterProcedureEnv env) throws IOException { // Fail if we are taking snapshot for the given table TableName tn = regionsToMerge[0].getTable(); + final String regionNamesToLog = RegionInfo.getShortNameToLog(regionsToMerge); if (env.getMasterServices().getSnapshotManager().isTableTakingAnySnapshot(tn)) { - throw new MergeRegionException("Skip merging regions " - + RegionInfo.getShortNameToLog(regionsToMerge) + ", because we are snapshotting " + tn); + throw new MergeRegionException( + "Skip merging regions " + regionNamesToLog + ", because we are snapshotting " + tn); } - // Mostly this check is not used because we already check the switch before submit a merge - // procedure. Just for safe, check the switch again. This procedure can be rollbacked if - // the switch was set to false after submit. + // Mostly the below two checks are not used because we already check the switches before + // submitting the merge procedure. Just for safety, we are checking the switch again here. + // Also, in case the switch was set to false after submission, this procedure can be rollbacked, + // thanks to this double check! + // case 1: check for cluster level switch if (!env.getMasterServices().isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { - String regionsStr = Arrays.deepToString(this.regionsToMerge); - LOG.warn("Merge switch is off! skip merge of " + regionsStr); + LOG.warn("Merge switch is off! skip merge of " + regionNamesToLog); setFailure(getClass().getSimpleName(), - new IOException("Merge of " + regionsStr + " failed because merge switch is off")); + new IOException("Merge of " + regionNamesToLog + " failed because merge switch is off")); return false; } - + // case 2: check for table level switch if (!env.getMasterServices().getTableDescriptors().get(getTableName()).isMergeEnabled()) { - String regionsStr = Arrays.deepToString(regionsToMerge); - LOG.warn("Merge is disabled for the table! Skipping merge of {}", regionsStr); + LOG.warn("Merge is disabled for the table! Skipping merge of {}", regionNamesToLog); setFailure(getClass().getSimpleName(), new IOException( - "Merge of " + regionsStr + " failed as region merge is disabled for the table")); + "Merge of " + regionNamesToLog + " failed as region merge is disabled for the table")); return false; } @@ -471,8 +472,8 @@ private boolean prepareMergeRegion(final MasterProcedureEnv env) throws IOExcept RegionStateStore regionStateStore = env.getAssignmentManager().getRegionStateStore(); for (RegionInfo ri : this.regionsToMerge) { if (regionStateStore.hasMergeRegions(ri)) { - String msg = "Skip merging " + RegionInfo.getShortNameToLog(regionsToMerge) - + ", because a parent, " + RegionInfo.getShortNameToLog(ri) + ", has a merge qualifier " + String msg = "Skip merging " + regionNamesToLog + ", because a parent, " + + RegionInfo.getShortNameToLog(ri) + ", has a merge qualifier " + "(if a 'merge column' in parent, it was recently merged but still has outstanding " + "references to its parents that must be cleared before it can participate in merge -- " + "major compact it to hurry clearing of its references)"; @@ -492,8 +493,8 @@ private boolean prepareMergeRegion(final MasterProcedureEnv env) throws IOExcept try { if (!isMergeable(env, state)) { setFailure(getClass().getSimpleName(), - new MergeRegionException("Skip merging " + RegionInfo.getShortNameToLog(regionsToMerge) - + ", because a parent, " + RegionInfo.getShortNameToLog(ri) + ", is not mergeable")); + new MergeRegionException("Skip merging " + regionNamesToLog + ", because a parent, " + + RegionInfo.getShortNameToLog(ri) + ", is not mergeable")); return false; } } catch (IOException e) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java index afa0f5e42b07..01cd012ddad1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java @@ -547,16 +547,18 @@ public boolean prepareSplitRegion(final MasterProcedureEnv env) throws IOExcepti return false; } - // Mostly this check is not used because we already check the switch before submit a split - // procedure. Just for safe, check the switch again. This procedure can be rollbacked if - // the switch was set to false after submit. + // Mostly the below two checks are not used because we already check the switches before + // submitting the split procedure. Just for safety, we are checking the switch again here. + // Also, in case the switch was set to false after submission, this procedure can be rollbacked, + // thanks to this double check! + // case 1: check for cluster level switch if (!env.getMasterServices().isSplitOrMergeEnabled(MasterSwitchType.SPLIT)) { LOG.warn("pid=" + getProcId() + " split switch is off! skip split of " + parentHRI); setFailure(new IOException( "Split region " + parentHRI.getRegionNameAsString() + " failed due to split switch off")); return false; } - + // case 2: check for table level switch if (!env.getMasterServices().getTableDescriptors().get(getTableName()).isSplitEnabled()) { LOG.warn("pid={}, split is disabled for the table! Skipping split of {}", getProcId(), parentHRI); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeAtTableLevel.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeAtTableLevel.java index d535fc54356d..c5c31c1d6fd7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeAtTableLevel.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeAtTableLevel.java @@ -25,6 +25,7 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseTestingUtil; import org.apache.hadoop.hbase.TableName; @@ -32,7 +33,6 @@ import org.apache.hadoop.hbase.testclassification.ClientTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.Threads; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.ClassRule; @@ -125,7 +125,7 @@ public void testTableMergeSwitch() throws Exception { assertFalse(admin.getDescriptor(tableName).isMergeEnabled()); trySplitAndEnsureItIsSuccess(tableName); - Threads.sleep(10000); + tryMergeAndEnsureItFails(tableName); admin.disableTable(tableName); enableTableMerge(tableName); @@ -166,6 +166,7 @@ private void trySplitAndEnsureItFails(final TableName tableName) throws Exceptio // expected to reach here // check and ensure that table does not get splitted assertTrue(admin.getRegions(tableName).size() == originalCount); + assertTrue("Expected DoNotRetryIOException!", ee.getCause() instanceof DoNotRetryIOException); } } @@ -217,7 +218,7 @@ private void tryMergeAndEnsureItFails(final TableName tableName) throws Exceptio byte[] nameOfRegionB = regions.get(1).getEncodedNameAsBytes(); // check and ensure that region do not get merged - Future f = admin.mergeRegionsAsync(nameOfRegionA, nameOfRegionB, true); + Future f = admin.mergeRegionsAsync(new byte[][] { nameOfRegionA, nameOfRegionB }, true); try { f.get(10, TimeUnit.SECONDS); fail("Should not get here."); @@ -225,6 +226,7 @@ private void tryMergeAndEnsureItFails(final TableName tableName) throws Exceptio // expected to reach here // check and ensure that region do not get merged assertTrue(admin.getRegions(tableName).size() == originalCount); + assertTrue("Expected DoNotRetryIOException!", ee.getCause() instanceof DoNotRetryIOException); } } @@ -255,7 +257,7 @@ private void tryMergeAndEnsureItIsSuccess(final TableName tableName) throws Exce byte[] nameOfRegionB = regions.get(1).getEncodedNameAsBytes(); // merge the table regions and wait until region count decreases - admin.mergeRegionsAsync(nameOfRegionA, nameOfRegionB, true); + admin.mergeRegionsAsync(new byte[][] { nameOfRegionA, nameOfRegionB }, true); TEST_UTIL.waitFor(30000, new ExplainingPredicate() { @Override