From 614ab028d47a5de21f64f51d2c86a6b254c41ead Mon Sep 17 00:00:00 2001 From: Guanghao Zhang Date: Sat, 22 Feb 2020 17:56:23 +0800 Subject: [PATCH] HBASE-23864 No need to submit SplitTableRegionProcedure/MergeTableRegionsProcedure when split/merge is disabled (#1185) Signed-off-by: binlijin Signed-off-by: stack --- .../org/apache/hadoop/hbase/master/HMaster.java | 14 ++++++++++++++ .../hbase/master/assignment/AssignmentManager.java | 14 ++++++++++++++ .../assignment/MergeTableRegionsProcedure.java | 6 +++++- .../assignment/SplitTableRegionProcedure.java | 7 +++++-- .../hbase/client/TestSplitOrMergeStatus.java | 8 +++++++- 5 files changed, 45 insertions(+), 4 deletions(-) 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 698fad4f7bd..a20a745bb12 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 @@ -1899,6 +1899,13 @@ public class HMaster extends HRegionServer implements MasterServices { final long nonce) throws IOException { checkInitialized(); + if (!isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { + String regionsStr = Arrays.deepToString(regionsToMerge); + LOG.warn("Merge switch is off! skip merge of " + regionsStr); + throw new DoNotRetryIOException("Merge of " + regionsStr + + " failed because merge switch is off"); + } + final String mergeRegionsStr = Arrays.stream(regionsToMerge). map(r -> RegionInfo.getShortNameToLog(r)).collect(Collectors.joining(", ")); return MasterProcedureUtil.submitProcedure(new NonceProcedureRunnable(this, ng, nonce) { @@ -1924,6 +1931,13 @@ public class HMaster extends HRegionServer implements MasterServices { final long nonceGroup, final long nonce) throws IOException { checkInitialized(); + + if (!isSplitOrMergeEnabled(MasterSwitchType.SPLIT)) { + LOG.warn("Split switch is off! skip split of " + regionInfo); + throw new DoNotRetryIOException("Split region " + regionInfo.getRegionNameAsString() + + " failed due to split switch off"); + } + return MasterProcedureUtil.submitProcedure( new MasterProcedureUtil.NonceProcedureRunnable(this, nonceGroup, nonce) { @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index ee59457f720..4eca5109d3c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -32,6 +32,7 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; + import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseIOException; @@ -42,6 +43,7 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.UnknownRegionException; import org.apache.hadoop.hbase.client.DoNotRetryRegionException; +import org.apache.hadoop.hbase.client.MasterSwitchType; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.client.RegionStatesCount; @@ -1007,6 +1009,12 @@ public class AssignmentManager { " hriA=" + hriA + " hriB=" + hriB); } + if (!master.isSplitOrMergeEnabled(MasterSwitchType.SPLIT)) { + LOG.warn("Split switch is off! skip split of " + parent); + throw new DoNotRetryIOException("Split region " + parent.getRegionNameAsString() + + " failed due to split switch off"); + } + // Submit the Split procedure final byte[] splitKey = hriB.getStartKey(); if (LOG.isDebugEnabled()) { @@ -1032,6 +1040,12 @@ public class AssignmentManager { " maybe an old RS (< 2.0) had the operation in progress"); } + if (!master.isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { + LOG.warn("Merge switch is off! skip merge of regionA=" + hriA + " regionB=" + hriB); + throw new DoNotRetryIOException("Merge of regionA=" + hriA + " regionB=" + hriB + + " failed because merge switch is off"); + } + // Submit the Merge procedure if (LOG.isDebugEnabled()) { LOG.debug("Handling merge request from RS=" + merged + ", merged=" + merged); 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 0c4fa4657e9..f8d59ad221f 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 @@ -453,6 +453,10 @@ public class MergeTableRegionsProcedure throw new MergeRegionException("Skip merging regions " + RegionInfo.getShortNameToLog(regionsToMerge) + ", 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. if (!env.getMasterServices().isSplitOrMergeEnabled(MasterSwitchType.MERGE)) { String regionsStr = Arrays.deepToString(this.regionsToMerge); LOG.warn("Merge switch is off! skip merge of " + regionsStr); @@ -753,4 +757,4 @@ public class MergeTableRegionsProcedure // range of steps; what do we do for these should an operator want to cancel them? HBASE-20022. return isRollbackSupported(getCurrentState()) && super.abort(env); } -} \ No newline at end of file +} 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 1752c4685d7..0d628283c32 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 @@ -523,8 +523,9 @@ public class SplitTableRegionProcedure return false; } - // Since we have the lock and the master is coordinating the operation - // we are always able to split the region + // 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. 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() + @@ -543,6 +544,8 @@ public class SplitTableRegionProcedure // set node state as SPLITTING node.setState(State.SPLITTING); + // Since we have the lock and the master is coordinating the operation + // we are always able to split the region return true; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeStatus.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeStatus.java index f9749741bba..a46a58d4b92 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeStatus.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestSplitOrMergeStatus.java @@ -28,6 +28,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.HBaseTestingUtility; import org.apache.hadoop.hbase.HRegionInfo; @@ -96,7 +97,12 @@ public class TestSplitOrMergeStatus { boolean[] results = admin.setSplitOrMergeEnabled(false, false, MasterSwitchType.SPLIT); assertEquals(1, results.length); assertTrue(results[0]); - admin.split(t.getName()); + try { + admin.split(t.getName()); + fail("Should not get here."); + } catch (DoNotRetryIOException e) { + // Expected. + } int count = admin.getTableRegions(tableName).size(); assertTrue(originalCount == count); results = admin.setSplitOrMergeEnabled(true, false, MasterSwitchType.SPLIT);