From 8cd23c5b4845a242bb3a0e220444f48dff285c34 Mon Sep 17 00:00:00 2001 From: Jian He Date: Wed, 5 Mar 2014 19:04:58 +0000 Subject: [PATCH] YARN-1752. Fixed ApplicationMasterService to reject unregister request if AM did not register before. Contributed by Rohith Sharma. git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1574623 13f79535-47bb-0310-9956-ffa450edef68 --- hadoop-yarn-project/CHANGES.txt | 3 ++ ...alidApplicationMasterRequestException.java | 8 ++-- .../ApplicationMasterService.java | 13 +++++++ .../server/resourcemanager/RMAuditLogger.java | 1 + .../yarn/server/resourcemanager/MockAM.java | 12 +++--- .../yarn/server/resourcemanager/MockRM.java | 2 +- .../TestApplicationMasterService.java | 37 +++++++++++++++++++ .../server/resourcemanager/TestRMRestart.java | 2 +- 8 files changed, 68 insertions(+), 10 deletions(-) diff --git a/hadoop-yarn-project/CHANGES.txt b/hadoop-yarn-project/CHANGES.txt index b94bd2ea87..25ca96aa89 100644 --- a/hadoop-yarn-project/CHANGES.txt +++ b/hadoop-yarn-project/CHANGES.txt @@ -259,6 +259,9 @@ Release 2.4.0 - UNRELEASED YARN-1785. FairScheduler treats app lookup failures as ERRORs. (bc Wong via kasha) + YARN-1752. Fixed ApplicationMasterService to reject unregister request if + AM did not register before. (Rohith Sharma via jianhe) + OPTIMIZATIONS BUG FIXES diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/exceptions/InvalidApplicationMasterRequestException.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/exceptions/InvalidApplicationMasterRequestException.java index 386f299017..6b422b3bf0 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/exceptions/InvalidApplicationMasterRequestException.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/exceptions/InvalidApplicationMasterRequestException.java @@ -24,10 +24,12 @@ /** * This exception is thrown when an ApplicationMaster asks for resources by - * calling {@link ApplicationMasterProtocol#allocate(AllocateRequest)} API - * without first registering by calling + * calling {@link ApplicationMasterProtocol#allocate(AllocateRequest)} or tries + * to unregister by calling + * {@link ApplicationMasterProtocol#finishApplicationMaster(FinishApplicationMasterRequest)} + * API without first registering by calling * {@link ApplicationMasterProtocol#registerApplicationMaster(RegisterApplicationMasterRequest)} - * or if it tries to register more then once. + * or if it tries to register more than once. */ public class InvalidApplicationMasterRequestException extends YarnException { diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java index 17ec32aaa3..75a57dd569 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/ApplicationMasterService.java @@ -332,6 +332,19 @@ public FinishApplicationMasterResponse finishApplicationMaster( // Allow only one thread in AM to do finishApp at a time. synchronized (lock) { + if (!hasApplicationMasterRegistered(applicationAttemptId)) { + String message = + "Application Master is trying to unregister before registering for: " + + applicationAttemptId.getApplicationId(); + LOG.error(message); + RMAuditLogger.logFailure( + this.rmContext.getRMApps() + .get(applicationAttemptId.getApplicationId()).getUser(), + AuditConstants.UNREGISTER_AM, "", "ApplicationMasterService", + message, applicationAttemptId.getApplicationId(), + applicationAttemptId); + throw new InvalidApplicationMasterRequestException(message); + } this.amLivelinessMonitor.receivedPing(applicationAttemptId); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java index f7d12136f0..9ae09a4db7 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/RMAuditLogger.java @@ -50,6 +50,7 @@ public static class AuditConstants { public static final String FINISH_FAILED_APP = "Application Finished - Failed"; public static final String FINISH_KILLED_APP = "Application Finished - Killed"; public static final String REGISTER_AM = "Register App Master"; + public static final String UNREGISTER_AM = "Unregister App Master"; public static final String ALLOC_CONTAINER = "AM Allocated Container"; public static final String RELEASE_CONTAINER = "AM Released Container"; diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockAM.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockAM.java index 3b8d69f795..6ebdc68047 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockAM.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockAM.java @@ -225,17 +225,19 @@ public void unregisterAppAttempt() throws Exception { final FinishApplicationMasterRequest req = FinishApplicationMasterRequest.newInstance( FinalApplicationStatus.SUCCEEDED, "", ""); - unregisterAppAttempt(req); + unregisterAppAttempt(req,true); } - public void unregisterAppAttempt(final FinishApplicationMasterRequest req) - throws Exception { - waitForState(RMAppAttemptState.RUNNING); + public void unregisterAppAttempt(final FinishApplicationMasterRequest req, + boolean waitForStateRunning) throws Exception { + if (waitForStateRunning) { + waitForState(RMAppAttemptState.RUNNING); + } UserGroupInformation ugi = UserGroupInformation.createRemoteUser(attemptId.toString()); Token token = context.getRMApps().get(attemptId.getApplicationId()) - .getRMAppAttempt(attemptId).getAMRMToken(); + .getRMAppAttempt(attemptId).getAMRMToken(); ugi.addTokenIdentifier(token.decodeIdentifier()); ugi.doAs(new PrivilegedExceptionAction() { @Override diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java index c91ce35015..2efdfe1d24 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/MockRM.java @@ -477,7 +477,7 @@ public static void finishApplicationMaster(RMApp rmApp, MockRM rm, MockNM nm, FinishApplicationMasterRequest req = FinishApplicationMasterRequest.newInstance( FinalApplicationStatus.SUCCEEDED, "", ""); - am.unregisterAppAttempt(req); + am.unregisterAppAttempt(req,true); am.waitForState(RMAppAttemptState.FINISHING); nm.nodeHeartbeat(am.getApplicationAttemptId(), 1, ContainerState.COMPLETE); am.waitForState(RMAppAttemptState.FINISHED); diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationMasterService.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationMasterService.java index 76c999d849..7d71452593 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationMasterService.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestApplicationMasterService.java @@ -26,6 +26,7 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.security.UserGroupInformation; import org.apache.hadoop.yarn.api.protocolrecords.AllocateResponse; +import org.apache.hadoop.yarn.api.protocolrecords.FinishApplicationMasterRequest; import org.apache.hadoop.yarn.api.protocolrecords.RegisterApplicationMasterRequest; import org.apache.hadoop.yarn.api.protocolrecords.RegisterApplicationMasterResponse; import org.apache.hadoop.yarn.api.protocolrecords.impl.pb.AllocateRequestPBImpl; @@ -34,6 +35,7 @@ import org.apache.hadoop.yarn.event.Dispatcher; import org.apache.hadoop.yarn.event.EventHandler; import org.apache.hadoop.yarn.event.InlineDispatcher; +import org.apache.hadoop.yarn.exceptions.InvalidApplicationMasterRequestException; import org.apache.hadoop.yarn.exceptions.InvalidContainerReleaseException; import org.apache.hadoop.yarn.exceptions.YarnException; import org.apache.hadoop.yarn.security.AMRMTokenIdentifier; @@ -246,4 +248,39 @@ public void testProgressFilter() throws Exception{ sleep(100); } } + + @Test(timeout=1200000) + public void testFinishApplicationMasterBeforeRegistering() throws Exception { + MockRM rm = new MockRM(conf); + try { + rm.start(); + // Register node1 + MockNM nm1 = rm.registerNode("127.0.0.1:1234", 6 * GB); + // Submit an application + RMApp app1 = rm.submitApp(2048); + MockAM am1 = MockRM.launchAM(app1, rm, nm1); + FinishApplicationMasterRequest req = + FinishApplicationMasterRequest.newInstance( + FinalApplicationStatus.FAILED, "", ""); + Throwable cause = null; + try { + am1.unregisterAppAttempt(req, false); + } catch (Exception e) { + cause = e.getCause(); + } + Assert.assertNotNull(cause); + Assert + .assertTrue(cause instanceof InvalidApplicationMasterRequestException); + Assert.assertNotNull(cause.getMessage()); + Assert + .assertTrue(cause + .getMessage() + .contains( + "Application Master is trying to unregister before registering for:")); + } finally { + if (rm != null) { + rm.stop(); + } + } + } } diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java index dff9019589..ad2e17f730 100644 --- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java +++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/test/java/org/apache/hadoop/yarn/server/resourcemanager/TestRMRestart.java @@ -928,7 +928,7 @@ private void finishApplicationMaster(RMApp rmApp, MockRM rm, MockNM nm, ((MemoryRMStateStore) rm.getRMContext().getStateStore()).getState(); Map rmAppState = rmState.getApplicationState(); - am.unregisterAppAttempt(req); + am.unregisterAppAttempt(req,true); am.waitForState(RMAppAttemptState.FINISHING); nm.nodeHeartbeat(am.getApplicationAttemptId(), 1, ContainerState.COMPLETE); am.waitForState(RMAppAttemptState.FINISHED);