HBASE-18105 [AMv2] Split/Merge need cleanup; currently they diverge and do not fully embrace AMv2 world (Yi Liang)
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/38eaf47f Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/38eaf47f Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/38eaf47f Branch: refs/heads/HBASE-18467 Commit: 38eaf47fa7dd4b0b3b822e73c9e644370a5cacb6 Parents: d35d837 Author: Michael Stack <st...@apache.org> Authored: Mon Oct 2 11:38:11 2017 -0700 Committer: Michael Stack <st...@apache.org> Committed: Mon Oct 2 11:38:11 2017 -0700 ---------------------------------------------------------------------- .../src/main/protobuf/MasterProcedure.proto | 4 +-- .../src/main/protobuf/RegionServerStatus.proto | 6 ++-- .../hbase/coprocessor/MasterObserver.java | 12 +++---- .../hbase/master/MasterCoprocessorHost.java | 12 +++---- .../master/assignment/AssignmentManager.java | 2 -- .../assignment/SplitTableRegionProcedure.java | 29 ++++++++--------- .../hbase/coprocessor/TestMasterObserver.java | 4 +-- .../TestMergeTableRegionsProcedure.java | 29 +++++++++++++++++ .../TestSplitTableRegionProcedure.java | 33 ++++++++++++++++++++ .../MasterProcedureTestingUtility.java | 2 +- .../TestSplitTransactionOnCluster.java | 2 +- 11 files changed, 99 insertions(+), 36 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto ---------------------------------------------------------------------- diff --git a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto index 2cdebb1..626530f 100644 --- a/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto +++ b/hbase-protocol-shaded/src/main/protobuf/MasterProcedure.proto @@ -286,9 +286,9 @@ enum SplitTableRegionState { SPLIT_TABLE_REGION_PRE_OPERATION = 2; SPLIT_TABLE_REGION_CLOSE_PARENT_REGION = 3; SPLIT_TABLE_REGION_CREATE_DAUGHTER_REGIONS = 4; - SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_PONR = 5; + SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_META = 5; SPLIT_TABLE_REGION_UPDATE_META = 6; - SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_PONR = 7; + SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_META = 7; SPLIT_TABLE_REGION_OPEN_CHILD_REGIONS = 8; SPLIT_TABLE_REGION_POST_OPERATION = 9; } http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-protocol-shaded/src/main/protobuf/RegionServerStatus.proto ---------------------------------------------------------------------- diff --git a/hbase-protocol-shaded/src/main/protobuf/RegionServerStatus.proto b/hbase-protocol-shaded/src/main/protobuf/RegionServerStatus.proto index 1cd4376..f83bb20 100644 --- a/hbase-protocol-shaded/src/main/protobuf/RegionServerStatus.proto +++ b/hbase-protocol-shaded/src/main/protobuf/RegionServerStatus.proto @@ -105,11 +105,13 @@ message RegionStateTransition { READY_TO_SPLIT = 3; READY_TO_MERGE = 4; - SPLIT_PONR = 5; - MERGE_PONR = 6; + /** We used to have PONR enums for split and merge in here occupying + positions 5 and 6 but they have since been removed. Do not reuse these + indices */ SPLIT = 7; MERGED = 8; + SPLIT_REVERTED = 9; MERGE_REVERTED = 10; } http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java index bfa88e6..85da610 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java @@ -696,25 +696,25 @@ public interface MasterObserver { final RegionInfo regionInfoB) throws IOException {} /** - * This will be called before PONR step as part of split transaction. Calling + * This will be called before update META step as part of split transaction. Calling * {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} rollback the split * @param ctx the environment to interact with the framework and master * @param splitKey * @param metaEntries */ - default void preSplitRegionBeforePONRAction( + default void preSplitRegionBeforeMETAAction( final ObserverContext<MasterCoprocessorEnvironment> ctx, final byte[] splitKey, final List<Mutation> metaEntries) throws IOException {} /** - * This will be called after PONR step as part of split transaction + * This will be called after update META step as part of split transaction * Calling {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} has no * effect in this hook. * @param ctx the environment to interact with the framework and master */ - default void preSplitRegionAfterPONRAction( + default void preSplitRegionAfterMETAAction( final ObserverContext<MasterCoprocessorEnvironment> ctx) throws IOException {} @@ -745,7 +745,7 @@ public interface MasterObserver { final RegionInfo mergedRegion) throws IOException {} /** - * This will be called before PONR step as part of regions merge transaction. Calling + * This will be called before update META step as part of regions merge transaction. Calling * {@link org.apache.hadoop.hbase.coprocessor.ObserverContext#bypass()} rollback the merge * @param ctx the environment to interact with the framework and master * @param metaEntries mutations to execute on hbase:meta atomically with regions merge updates. @@ -757,7 +757,7 @@ public interface MasterObserver { @MetaMutationAnnotation List<Mutation> metaEntries) throws IOException {} /** - * This will be called after PONR step as part of regions merge transaction. + * This will be called after META step as part of regions merge transaction. * @param ctx the environment to interact with the framework and master */ default void postMergeRegionsCommitAction( http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java index 5431ece..72ba5ae 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java @@ -874,34 +874,34 @@ public class MasterCoprocessorHost } /** - * This will be called before PONR step as part of split table region procedure. + * This will be called before update META step as part of split table region procedure. * @param splitKey * @param metaEntries * @param user the user * @throws IOException */ - public boolean preSplitBeforePONRAction( + public boolean preSplitBeforeMETAAction( final byte[] splitKey, final List<Mutation> metaEntries, final User user) throws IOException { return execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { @Override public void call(MasterObserver observer) throws IOException { - observer.preSplitRegionBeforePONRAction(this, splitKey, metaEntries); + observer.preSplitRegionBeforeMETAAction(this, splitKey, metaEntries); } }); } /** - * This will be called after PONR step as part of split table region procedure. + * This will be called after update META step as part of split table region procedure. * @param user the user * @throws IOException */ - public void preSplitAfterPONRAction(final User user) throws IOException { + public void preSplitAfterMETAAction(final User user) throws IOException { execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation(user) { @Override public void call(MasterObserver observer) throws IOException { - observer.preSplitRegionAfterPONRAction(this); + observer.preSplitRegionAfterMETAAction(this); } }); } http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java ---------------------------------------------------------------------- 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 021d411..22f734c 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 @@ -754,7 +754,6 @@ public class AssignmentManager implements ServerListener { transition.hasOpenSeqNum() ? transition.getOpenSeqNum() : HConstants.NO_SEQNUM); break; case READY_TO_SPLIT: - case SPLIT_PONR: case SPLIT: case SPLIT_REVERTED: assert transition.getRegionInfoCount() == 3 : transition; @@ -765,7 +764,6 @@ public class AssignmentManager implements ServerListener { parent, splitA, splitB); break; case READY_TO_MERGE: - case MERGE_PONR: case MERGED: case MERGE_REVERTED: assert transition.getRegionInfoCount() == 3 : transition; http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java ---------------------------------------------------------------------- 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 cbd334e..78ed7b4 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 @@ -164,7 +164,8 @@ public class SplitTableRegionProcedure } if(bestSplitRow == null || bestSplitRow.length == 0) { - throw new DoNotRetryIOException("Region not splittable because bestSplitPoint = null"); + throw new DoNotRetryIOException("Region not splittable because bestSplitPoint = null, " + + "maybe table is too small for auto split. For force split, try specifying split row"); } if (Bytes.equals(regionToSplit.getStartKey(), bestSplitRow)) { @@ -223,18 +224,18 @@ public class SplitTableRegionProcedure break; case SPLIT_TABLE_REGION_CREATE_DAUGHTER_REGIONS: createDaughterRegions(env); - setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_PONR); + setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_META); break; - case SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_PONR: - preSplitRegionBeforePONR(env); + case SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_META: + preSplitRegionBeforeMETA(env); setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_UPDATE_META); break; case SPLIT_TABLE_REGION_UPDATE_META: updateMetaForDaughterRegions(env); - setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_PONR); + setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_META); break; - case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_PONR: - preSplitRegionAfterPONR(env); + case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_META: + preSplitRegionAfterMETA(env); setNextState(SplitTableRegionState.SPLIT_TABLE_REGION_OPEN_CHILD_REGIONS); break; case SPLIT_TABLE_REGION_OPEN_CHILD_REGIONS: @@ -273,11 +274,11 @@ public class SplitTableRegionProcedure switch (state) { case SPLIT_TABLE_REGION_POST_OPERATION: case SPLIT_TABLE_REGION_OPEN_CHILD_REGIONS: - case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_PONR: + case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_META: case SPLIT_TABLE_REGION_UPDATE_META: // PONR throw new UnsupportedOperationException(this + " unhandled state=" + state); - case SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_PONR: + case SPLIT_TABLE_REGION_PRE_OPERATION_BEFORE_META: break; case SPLIT_TABLE_REGION_CREATE_DAUGHTER_REGIONS: // Doing nothing, as re-open parent region would clean up daughter region directories. @@ -311,7 +312,7 @@ public class SplitTableRegionProcedure switch (state) { case SPLIT_TABLE_REGION_POST_OPERATION: case SPLIT_TABLE_REGION_OPEN_CHILD_REGIONS: - case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_PONR: + case SPLIT_TABLE_REGION_PRE_OPERATION_AFTER_META: case SPLIT_TABLE_REGION_UPDATE_META: // It is not safe to rollback if we reach to these states. return false; @@ -703,12 +704,12 @@ public class SplitTableRegionProcedure * Post split region actions before the Point-of-No-Return step * @param env MasterProcedureEnv **/ - private void preSplitRegionBeforePONR(final MasterProcedureEnv env) + private void preSplitRegionBeforeMETA(final MasterProcedureEnv env) throws IOException, InterruptedException { final List<Mutation> metaEntries = new ArrayList<Mutation>(); final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); if (cpHost != null) { - if (cpHost.preSplitBeforePONRAction(getSplitRow(), metaEntries, getUser())) { + if (cpHost.preSplitBeforeMETAAction(getSplitRow(), metaEntries, getUser())) { throw new IOException("Coprocessor bypassing region " + getParentRegion().getRegionNameAsString() + " split."); } @@ -739,11 +740,11 @@ public class SplitTableRegionProcedure * Pre split region actions after the Point-of-No-Return step * @param env MasterProcedureEnv **/ - private void preSplitRegionAfterPONR(final MasterProcedureEnv env) + private void preSplitRegionAfterMETA(final MasterProcedureEnv env) throws IOException, InterruptedException { final MasterCoprocessorHost cpHost = env.getMasterCoprocessorHost(); if (cpHost != null) { - cpHost.preSplitAfterPONRAction(getUser()); + cpHost.preSplitAfterMETAAction(getUser()); } } http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java index 2759a68..4f8f264 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java @@ -1430,14 +1430,14 @@ public class TestMasterObserver { } @Override - public void preSplitRegionBeforePONRAction( + public void preSplitRegionBeforeMETAAction( final ObserverContext<MasterCoprocessorEnvironment> ctx, final byte[] splitKey, final List<Mutation> metaEntries) throws IOException { } @Override - public void preSplitRegionAfterPONRAction( + public void preSplitRegionAfterMETAAction( final ObserverContext<MasterCoprocessorEnvironment> ctx) throws IOException { } http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java index 28d07aa..beb53ec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestMergeTableRegionsProcedure.java @@ -272,6 +272,35 @@ public class TestMergeTableRegionsProcedure { MasterProcedureTestingUtility.testRollbackAndDoubleExecution(procExec, procId, numberOfSteps); } + @Test + public void testMergeWithoutPONR() throws Exception { + final TableName tableName = TableName.valueOf("testRecoveryAndDoubleExecution"); + final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); + + List<RegionInfo> tableRegions = createTable(tableName); + + ProcedureTestingUtility.waitNoProcedureRunning(procExec); + ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true); + + RegionInfo[] regionsToMerge = new RegionInfo[2]; + regionsToMerge[0] = tableRegions.get(0); + regionsToMerge[1] = tableRegions.get(1); + + long procId = procExec.submitProcedure( + new MergeTableRegionsProcedure(procExec.getEnvironment(), regionsToMerge, true)); + + // Execute until step 9 of split procedure + // NOTE: step 9 is after step MERGE_TABLE_REGIONS_UPDATE_META + MasterProcedureTestingUtility.testRecoveryAndDoubleExecution(procExec, procId, 9, false); + + // Unset Toggle Kill and make ProcExec work correctly + ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false); + MasterProcedureTestingUtility.restartMasterProcedureExecutor(procExec); + ProcedureTestingUtility.waitProcedure(procExec, procId); + + assertRegionCount(tableName, initialRegionCount - 1); + } + private List<RegionInfo> createTable(final TableName tableName) throws Exception { HTableDescriptor desc = new HTableDescriptor(tableName); http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java index 7b6e977..4b63f0e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestSplitTableRegionProcedure.java @@ -415,6 +415,39 @@ public class TestSplitTableRegionProcedure { assertEquals(splitFailedCount, splitProcMetrics.getFailedCounter().getCount()); } + @Test + public void testSplitWithoutPONR() throws Exception { + final TableName tableName = TableName.valueOf(name.getMethodName()); + final ProcedureExecutor<MasterProcedureEnv> procExec = getMasterProcedureExecutor(); + + RegionInfo [] regions = MasterProcedureTestingUtility.createTable( + procExec, tableName, null, ColumnFamilyName1, ColumnFamilyName2); + insertData(tableName); + int splitRowNum = startRowNum + rowCount / 2; + byte[] splitKey = Bytes.toBytes("" + splitRowNum); + + assertTrue("not able to find a splittable region", regions != null); + assertTrue("not able to find a splittable region", regions.length == 1); + ProcedureTestingUtility.waitNoProcedureRunning(procExec); + ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, true); + + // Split region of the table + long procId = procExec.submitProcedure( + new SplitTableRegionProcedure(procExec.getEnvironment(), regions[0], splitKey)); + + // Execute until step 7 of split procedure + // NOTE: the 7 (number after SPLIT_TABLE_REGION_UPDATE_META step) + MasterProcedureTestingUtility.testRecoveryAndDoubleExecution(procExec, procId, 7, false); + + // Unset Toggle Kill and make ProcExec work correctly + ProcedureTestingUtility.setKillAndToggleBeforeStoreUpdate(procExec, false); + MasterProcedureTestingUtility.restartMasterProcedureExecutor(procExec); + ProcedureTestingUtility.waitProcedure(procExec, procId); + + // Even split failed after step 4, it should still works fine + verify(tableName, splitRowNum); + } + private void insertData(final TableName tableName) throws IOException, InterruptedException { Table t = UTIL.getConnection().getTable(tableName); Put p; http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java index b87c343..1485966 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/MasterProcedureTestingUtility.java @@ -366,7 +366,7 @@ public class MasterProcedureTestingUtility { * finish. * @see #testRecoveryAndDoubleExecution(ProcedureExecutor, long) */ - private static void testRecoveryAndDoubleExecution( + public static void testRecoveryAndDoubleExecution( final ProcedureExecutor<MasterProcedureEnv> procExec, final long procId, final int numSteps, final boolean expectExecRunning) throws Exception { ProcedureTestingUtility.waitProcedure(procExec, procId); http://git-wip-us.apache.org/repos/asf/hbase/blob/38eaf47f/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index cee4caf..1965d5a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -283,7 +283,7 @@ public class TestSplitTransactionOnCluster { } @Override - public void preSplitRegionBeforePONRAction( + public void preSplitRegionBeforeMETAAction( final ObserverContext<MasterCoprocessorEnvironment> ctx, final byte[] splitKey, final List<Mutation> metaEntries) throws IOException {