Repository: hbase Updated Branches: refs/heads/master f314b5911 -> 67eddf587
HBASE-18525 [AMv2] Fixed test TestAssignmentManager#testSocketTimeout on master branch Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/67eddf58 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/67eddf58 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/67eddf58 Branch: refs/heads/master Commit: 67eddf5874f6a008fe341fa6ce6b34e42a945118 Parents: f314b59 Author: Umesh Agashe <[email protected]> Authored: Tue Aug 8 17:01:00 2017 -0700 Committer: Michael Stack <[email protected]> Committed: Wed Aug 9 10:15:37 2017 -0700 ---------------------------------------------------------------------- .../assignment/RegionTransitionProcedure.java | 30 +++++++++++++++++++- .../assignment/TestAssignmentManager.java | 7 ++++- pom.xml | 1 - 3 files changed, 35 insertions(+), 3 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/67eddf58/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionTransitionProcedure.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionTransitionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionTransitionProcedure.java index 49124ea..dd8dedc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionTransitionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionTransitionProcedure.java @@ -53,6 +53,34 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.RegionServerStatusProto * to remote server is sent and the Procedure is suspended waiting on external * event to be woken again. Once the external event is triggered, Procedure * moves to the REGION_TRANSITION_FINISH state. + * + * <p>NOTE: {@link AssignProcedure} and {@link UnassignProcedure} should not be thought of + * as being asymmetric, at least currently. + * <ul> + * <li>{@link AssignProcedure} moves through all the above described states and implements methods + * associated with each while {@link UnassignProcedure} starts at state + * REGION_TRANSITION_DISPATCH and state REGION_TRANSITION_QUEUE is not supported.</li> + * + * <li>When any step in {@link AssignProcedure} fails, failure handler + * AssignProcedure#handleFailure(MasterProcedureEnv, RegionStateNode) re-attempts the + * assignment by setting the procedure state to REGION_TRANSITION_QUEUE and forces + * assignment to a different target server by setting {@link AssignProcedure#forceNewPlan}. When + * the number of attempts reach hreshold configuration 'hbase.assignment.maximum.attempts', + * the procedure is aborted. For {@link UnassignProcedure}, similar re-attempts are + * intentionally not implemented. It is a 'one shot' procedure. + * </li> + * </ul> + * + * <p>TODO: Considering it is a priority doing all we can to get make a region available as soon as possible, + * re-attempting with any target makes sense if specified target fails in case of + * {@link AssignProcedure}. For {@link UnassignProcedure}, if communication with RS fails, + * similar re-attempt makes little sense (what should be different from previous attempt?). Also it + * could be complex with current implementation of + * {@link RegionTransitionProcedure#execute(MasterProcedureEnv)} and {@link UnassignProcedure}. + * We have made a choice of keeping {@link UnassignProcedure} simple, where the procedure either + * succeeds or fails depending on communication with RS. As parent will have broader context, parent + * can better handle the failed instance of {@link UnassignProcedure}. Similar simplicity for + * {@link AssignProcedure} is desired and should be explored/ discussed further. */ @InterfaceAudience.Private public abstract class RegionTransitionProcedure @@ -378,4 +406,4 @@ public abstract class RegionTransitionProcedure * @return ServerName the Assign or Unassign is going against. */ public abstract ServerName getServer(final MasterProcedureEnv env); -} \ No newline at end of file +} http://git-wip-us.apache.org/repos/asf/hbase/blob/67eddf58/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java index bb72b90..d18c12a 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/TestAssignmentManager.java @@ -56,6 +56,7 @@ import org.apache.hadoop.hbase.master.procedure.MasterProcedureConstants; import org.apache.hadoop.hbase.master.procedure.MasterProcedureScheduler; import org.apache.hadoop.hbase.master.procedure.ProcedureSyncWait; import org.apache.hadoop.hbase.master.procedure.RSProcedureDispatcher; +import org.apache.hadoop.hbase.master.procedure.ServerCrashException; import org.apache.hadoop.hbase.procedure2.Procedure; import org.apache.hadoop.hbase.procedure2.ProcedureMetrics; import org.apache.hadoop.hbase.procedure2.ProcedureTestingUtility; @@ -89,6 +90,7 @@ import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; import org.junit.rules.TestName; import org.junit.rules.TestRule; @@ -102,6 +104,7 @@ public class TestAssignmentManager { @Rule public final TestRule timeout = CategoryBasedTimeout.builder().withTimeout(this.getClass()). withLookingForStuckThread(true).build(); + @Rule public final ExpectedException exception = ExpectedException.none(); private static final int PROC_NTHREADS = 64; private static final int NREGIONS = 1 * 1000; @@ -252,12 +255,14 @@ public class TestAssignmentManager { waitOnFuture(submitProcedure(am.createAssignProcedure(hri, false))); rsDispatcher.setMockRsExecutor(new SocketTimeoutRsExecutor(20, 3)); + + exception.expect(ServerCrashException.class); waitOnFuture(submitProcedure(am.createUnassignProcedure(hri, null, false))); assertEquals(assignSubmittedCount + 1, assignProcMetrics.getSubmittedCounter().getCount()); assertEquals(assignFailedCount, assignProcMetrics.getFailedCounter().getCount()); assertEquals(unassignSubmittedCount + 1, unassignProcMetrics.getSubmittedCounter().getCount()); - assertEquals(unassignFailedCount, unassignProcMetrics.getFailedCounter().getCount()); + assertEquals(unassignFailedCount + 1, unassignProcMetrics.getFailedCounter().getCount()); } @Test http://git-wip-us.apache.org/repos/asf/hbase/blob/67eddf58/pom.xml ---------------------------------------------------------------------- diff --git a/pom.xml b/pom.xml index 4026d81..7648e8e 100644 --- a/pom.xml +++ b/pom.xml @@ -1342,7 +1342,6 @@ </maven.build.timestamp.format> <buildDate>${maven.build.timestamp}</buildDate> <compileSource>1.8</compileSource> - <hbase-thirdparty.version>1.0.0</hbase-thirdparty.version> <!-- Build dependencies --> <maven.min.version>3.0.4</maven.min.version> <java.min.version>${compileSource}</java.min.version>
