diff --git a/hadoop-mapreduce-project/CHANGES.txt b/hadoop-mapreduce-project/CHANGES.txt index 6354e8d24ad..74bee76e132 100644 --- a/hadoop-mapreduce-project/CHANGES.txt +++ b/hadoop-mapreduce-project/CHANGES.txt @@ -120,6 +120,9 @@ Release 0.23.3 - UNRELEASED MAPREDUCE-3773. Add queue metrics with buckets for job run times. (omalley via acmurthy) + MAPREDUCE-3970 Add ServiceOperations class to aid working with Services + (stevel) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/service/AbstractService.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/service/AbstractService.java index 317a5289b20..eeea1e1705c 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/service/AbstractService.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/service/AbstractService.java @@ -145,10 +145,7 @@ public abstract class AbstractService implements Service { * the desired state */ private void ensureCurrentState(STATE currentState) { - if (state != currentState) { - throw new IllegalStateException("For this operation, current State must " + - "be " + currentState + " instead of " + state); - } + ServiceOperations.ensureCurrentState(state, currentState); } /** diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/ServiceAssert.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/ServiceAssert.java index 7a45aa3985b..cfa79482cb4 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/ServiceAssert.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/ServiceAssert.java @@ -46,4 +46,34 @@ public class ServiceAssert extends Assert { assertEquals("Service in wrong state: " + service, state, service.getServiceState()); } + + /** + * Assert that the breakable service has entered a state exactly the number + * of time asserted. + * @param service service -if null an assertion is raised. + * @param state state to check. + * @param expected expected count. + */ + public static void assertStateCount(BreakableService service, + Service.STATE state, + int expected) { + assertNotNull("Null service", service); + int actual = service.getCount(state); + if (expected != actual) { + fail("Expected entry count for state [" + state +"] of " + service + + " to be " + expected + " but was " + actual); + } + } + + /** + * Assert that a service configuration contains a specific key; the value + * is ignored. + * @param service service to check + * @param key key to look for + */ + public static void assertServiceConfigurationContains(Service service, + String key) { + assertNotNull("No option "+ key + " in service configuration", + service.getConfig().get(key)); + } } diff --git a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/TestServiceLifecycle.java b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/TestServiceLifecycle.java index 7c9655e5163..c69b7b7c8ee 100644 --- a/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/TestServiceLifecycle.java +++ b/hadoop-mapreduce-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/service/TestServiceLifecycle.java @@ -24,17 +24,11 @@ import org.junit.Test; public class TestServiceLifecycle extends ServiceAssert { - void assertStateCount(BreakableService service, - Service.STATE state, - int expected) { - int actual = service.getCount(state); - if (expected != actual) { - fail("Expected entry count for state [" + state +"] of " + service - + " to be " + expected + " but was " + actual); - } - } - - + /** + * Walk the {@link BreakableService} through it's lifecycle, + * more to verify that service's counters work than anything else + * @throws Throwable if necessary + */ @Test public void testWalkthrough() throws Throwable { @@ -57,12 +51,14 @@ public class TestServiceLifecycle extends ServiceAssert { /** * call init twice - * @throws Throwable + * @throws Throwable if necessary */ @Test public void testInitTwice() throws Throwable { BreakableService svc = new BreakableService(); - svc.init(new Configuration()); + Configuration conf = new Configuration(); + conf.set("test.init","t"); + svc.init(conf); try { svc.init(new Configuration()); fail("Expected a failure, got " + svc); @@ -70,11 +66,12 @@ public class TestServiceLifecycle extends ServiceAssert { //expected } assertStateCount(svc, Service.STATE.INITED, 2); + assertServiceConfigurationContains(svc, "test.init"); } /** - * call start twice - * @throws Throwable + * Call start twice + * @throws Throwable if necessary */ @Test public void testStartTwice() throws Throwable { @@ -92,11 +89,11 @@ public class TestServiceLifecycle extends ServiceAssert { /** - * verify that when a service is stopped more than once, no exception - * is thrown, and the counter is incremented - * this is because the state change operations happen after the counter in + * Verify that when a service is stopped more than once, no exception + * is thrown, and the counter is incremented. + * This is because the state change operations happen after the counter in * the subclass is incremented, even though stop is meant to be a no-op - * @throws Throwable + * @throws Throwable if necessary */ @Test public void testStopTwice() throws Throwable { @@ -113,7 +110,7 @@ public class TestServiceLifecycle extends ServiceAssert { /** * Show that if the service failed during an init * operation, it stays in the created state, even after stopping it - * @throws Throwable + * @throws Throwable if necessary */ @Test @@ -139,7 +136,7 @@ public class TestServiceLifecycle extends ServiceAssert { /** * Show that if the service failed during an init * operation, it stays in the created state, even after stopping it - * @throws Throwable + * @throws Throwable if necessary */ @Test @@ -163,11 +160,10 @@ public class TestServiceLifecycle extends ServiceAssert { } /** - * verify that when a service is stopped more than once, no exception - * is thrown, and the counter is incremented - * this is because the state change operations happen after the counter in - * the subclass is incremented, even though stop is meant to be a no-op - * @throws Throwable + * verify that when a service fails during its stop operation, + * its state does not change, and the subclass invocation counter + * increments. + * @throws Throwable if necessary */ @Test public void testFailingStop() throws Throwable { @@ -181,6 +177,7 @@ public class TestServiceLifecycle extends ServiceAssert { //expected } assertStateCount(svc, Service.STATE.STOPPED, 1); + assertServiceStateStarted(svc); //now try again, and expect it to happen again try { svc.stop(); @@ -191,4 +188,31 @@ public class TestServiceLifecycle extends ServiceAssert { assertStateCount(svc, Service.STATE.STOPPED, 2); } + /** + * verify that when a service that is not started is stopped, its counter + * of stop calls is still incremented-and the service remains in its + * original state.. + * @throws Throwable on a failure + */ + @Test + public void testStopUnstarted() throws Throwable { + BreakableService svc = new BreakableService(); + svc.stop(); + assertServiceStateCreated(svc); + assertStateCount(svc, Service.STATE.STOPPED, 1); + + //stop failed, now it can be initialised + svc.init(new Configuration()); + + //and try to stop again, with no state change but an increment + svc.stop(); + assertServiceStateInited(svc); + assertStateCount(svc, Service.STATE.STOPPED, 2); + + //once started, the service can be stopped reliably + svc.start(); + ServiceOperations.stop(svc); + assertServiceStateStopped(svc); + assertStateCount(svc, Service.STATE.STOPPED, 3); + } }