From dd796bd916e8c53995e7a18916ca6f67f5db031a Mon Sep 17 00:00:00 2001 From: Umesh Agashe Date: Tue, 8 Aug 2017 17:01:00 -0700 Subject: [PATCH] HBASE-18525 [AMv2] Fixed test TestAssignmentManager#testSocketTimeout on master branch --- .../assignment/RegionTransitionProcedure.java | 28 ++++++++++++++++++++++ .../master/assignment/TestAssignmentManager.java | 7 +++++- 2 files changed, 34 insertions(+), 1 deletion(-) 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 49124ea04d5486a219f2d94648b8a66ac8357640..7a257c2c278f988937205382920442fe1c2a84b8 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. + * + *

XXX: Currently {@link AssignProcedure} and {@link UnassignProcedure} are asymmetric in some + * respect. + *

+ * + *

TODO: Considering a priority to 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 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 bb72b9030b330e5cb59c0d8f2c8cc1e7717c6f4f..d18c12adc4909ff57e368191b45b8b81c0eb142f 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 -- 2.10.1 (Apple Git-78)