From 00a4dfa3b1f92fc4c2e28bd33dfd735f2dafefe7 Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Fri, 16 Aug 2024 13:54:15 +0530 Subject: [PATCH 1/6] TEZ-4561: Improve reported exception when DAGAppMaster is shutting down. --- .../org/apache/tez/dag/app/DAGAppMaster.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index 263ac76b4c..563c5bcb34 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -36,6 +36,7 @@ import java.util.Arrays; import java.util.Calendar; import java.util.Collections; +import java.util.Date; import java.util.EnumSet; import java.util.HashMap; import java.util.HashSet; @@ -936,7 +937,7 @@ public void handle(DAGAppMasterEvent event) { protected class DAGAppMasterShutdownHandler { private AtomicBoolean shutdownHandled = new AtomicBoolean(false); private long sleepTimeBeforeExit = TezConstants.TEZ_DAG_SLEEP_TIME_BEFORE_EXIT; - + Date shutdownTime; void setSleepTimeBeforeExit(long sleepTimeBeforeExit) { this.sleepTimeBeforeExit = sleepTimeBeforeExit; } @@ -954,6 +955,7 @@ public void shutdown(boolean now) { synchronized (shutdownHandlerRunning) { shutdownHandlerRunning.set(true); + shutdownTime = new Date(System.currentTimeMillis()); } LOG.info("Handling DAGAppMaster shutdown"); @@ -1680,9 +1682,11 @@ public HadoopShim getHadoopShim() { @Override public Map getApplicationACLs() { - if (getServiceState() != STATE.STARTED) { + STATE serviceState = getServiceState(); + if (serviceState != STATE.STARTED) { throw new TezUncheckedException( - "Cannot get ApplicationACLs before all services have started"); + "Cannot get ApplicationACLs before all services have started, The current service state is " + serviceState + + getShutdownTimeString()); } return taskSchedulerManager.getApplicationAcls(); } @@ -1743,6 +1747,13 @@ public void setQueueName(String queueName) { } } + private String getShutdownTimeString() { + if (shutdownHandler != null && shutdownHandler.shutdownTime != null) { + return " The shutdown hook started at " + shutdownHandler.shutdownTime; + } + return ""; + } + private static class ServiceWithDependency implements ServiceStateChangeListener { ServiceWithDependency(Service service) { this.service = service; From 115fc4b9267adb6c8afb74c502983cf3d314adec Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Fri, 16 Aug 2024 16:12:16 +0530 Subject: [PATCH 2/6] Add Test Case. --- .../org/apache/tez/dag/app/DAGAppMaster.java | 2 +- .../apache/tez/dag/app/TestDAGAppMaster.java | 27 ++++++++++++++++--- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index 563c5bcb34..d2bc81f469 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -1749,7 +1749,7 @@ public void setQueueName(String queueName) { private String getShutdownTimeString() { if (shutdownHandler != null && shutdownHandler.shutdownTime != null) { - return " The shutdown hook started at " + shutdownHandler.shutdownTime; + return ". The shutdown hook started at " + shutdownHandler.shutdownTime; } return ""; } diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java index bb1e6de505..3401592304 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java @@ -32,7 +32,8 @@ import org.apache.hadoop.security.token.SecretManager; import org.apache.hadoop.security.token.Token; import org.apache.hadoop.security.token.TokenIdentifier; -import org.apache.hadoop.util.Progressable; +import org.apache.hadoop.test.LambdaTestUtils; +import org.apache.hadoop.util.Time; import org.apache.hadoop.yarn.api.records.ApplicationAttemptId; import org.apache.hadoop.yarn.api.records.ApplicationId; import org.apache.hadoop.yarn.api.records.ContainerId; @@ -49,6 +50,7 @@ import org.apache.tez.dag.api.NamedEntityDescriptor; import org.apache.tez.dag.api.TezConfiguration; import org.apache.tez.dag.api.TezConstants; +import org.apache.tez.dag.api.TezUncheckedException; import org.apache.tez.dag.api.UserPayload; import org.apache.tez.dag.api.records.DAGProtos; import org.apache.tez.dag.api.records.DAGProtos.AMPluginDescriptorProto; @@ -73,12 +75,12 @@ import java.io.DataInputStream; import java.io.DataOutput; import java.io.File; -import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.IOException; import java.lang.reflect.Field; -import java.net.URI; import java.nio.ByteBuffer; +import java.time.Instant; +import java.util.Date; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -493,6 +495,25 @@ public void testDagCredentialsWithMerge() throws Exception { testDagCredentials(true); } + @Test + public void testGetACLFailure() throws Exception { + ApplicationId appId = ApplicationId.newInstance(1, 1); + ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId, 2); + DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true); + TezConfiguration conf = new TezConfiguration(false); + conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, false); + dam.init(conf); + LambdaTestUtils.intercept(TezUncheckedException.class, + "Cannot get ApplicationACLs before all services have started, The current service state is INITED", + () -> dam.getContext().getApplicationACLs()); + dam.start(); + dam.stop(); + dam.mockShutdown.shutdownTime = Date.from(Instant.ofEpochMilli(Time.now())); + LambdaTestUtils.intercept(TezUncheckedException.class, + " Cannot get ApplicationACLs before all services have started, The current service state is STOPPED. The shutdown hook started at " + + dam.mockShutdown.shutdownTime, () -> dam.getContext().getApplicationACLs()); + } + @Test public void testBadProgress() throws Exception { TezConfiguration conf = new TezConfiguration(); From 616981db8b1ee4282b28ac258d7d37024ecb16cd Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Fri, 16 Aug 2024 20:04:49 +0530 Subject: [PATCH 3/6] Fix Checkstyle. --- .../src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java index 3401592304..5b27118222 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java @@ -510,7 +510,8 @@ public void testGetACLFailure() throws Exception { dam.stop(); dam.mockShutdown.shutdownTime = Date.from(Instant.ofEpochMilli(Time.now())); LambdaTestUtils.intercept(TezUncheckedException.class, - " Cannot get ApplicationACLs before all services have started, The current service state is STOPPED. The shutdown hook started at " + " Cannot get ApplicationACLs before all services have started, " + + "The current service state is STOPPED. The shutdown hook started at " + dam.mockShutdown.shutdownTime, () -> dam.getContext().getApplicationACLs()); } From 9dc1b51eae886c0fe3a1494efef6e9ad076ec3ea Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Wed, 28 Aug 2024 12:56:44 +0530 Subject: [PATCH 4/6] Move FullStop Above. --- .../src/main/java/org/apache/tez/dag/app/DAGAppMaster.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index d2bc81f469..3ae81c555c 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -1686,7 +1686,7 @@ public Map getApplicationACLs() { if (serviceState != STATE.STARTED) { throw new TezUncheckedException( "Cannot get ApplicationACLs before all services have started, The current service state is " + serviceState - + getShutdownTimeString()); + + "." + getShutdownTimeString()); } return taskSchedulerManager.getApplicationAcls(); } @@ -1749,7 +1749,7 @@ public void setQueueName(String queueName) { private String getShutdownTimeString() { if (shutdownHandler != null && shutdownHandler.shutdownTime != null) { - return ". The shutdown hook started at " + shutdownHandler.shutdownTime; + return " The shutdown hook started at " + shutdownHandler.shutdownTime; } return ""; } From 1790462b6ca297cd1b52fb3222809e5555971ce7 Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Wed, 28 Aug 2024 20:34:36 +0530 Subject: [PATCH 5/6] Use setter & getter. --- .../org/apache/tez/dag/app/DAGAppMaster.java | 17 +++++++++++++---- .../apache/tez/dag/app/TestDAGAppMaster.java | 5 +++-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index 3ae81c555c..aeeebc96ef 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -937,7 +937,16 @@ public void handle(DAGAppMasterEvent event) { protected class DAGAppMasterShutdownHandler { private AtomicBoolean shutdownHandled = new AtomicBoolean(false); private long sleepTimeBeforeExit = TezConstants.TEZ_DAG_SLEEP_TIME_BEFORE_EXIT; - Date shutdownTime; + private Date shutdownTime; + + public Date getShutdownTime() { + return shutdownTime; + } + + public void setShutdownTime(Date shutdownTime) { + this.shutdownTime = shutdownTime; + } + void setSleepTimeBeforeExit(long sleepTimeBeforeExit) { this.sleepTimeBeforeExit = sleepTimeBeforeExit; } @@ -955,7 +964,7 @@ public void shutdown(boolean now) { synchronized (shutdownHandlerRunning) { shutdownHandlerRunning.set(true); - shutdownTime = new Date(System.currentTimeMillis()); + setShutdownTime(new Date(System.currentTimeMillis())); } LOG.info("Handling DAGAppMaster shutdown"); @@ -1748,8 +1757,8 @@ public void setQueueName(String queueName) { } private String getShutdownTimeString() { - if (shutdownHandler != null && shutdownHandler.shutdownTime != null) { - return " The shutdown hook started at " + shutdownHandler.shutdownTime; + if (shutdownHandler != null && shutdownHandler.getShutdownTime() != null) { + return " The shutdown hook started at " + shutdownHandler.getShutdownTime(); } return ""; } diff --git a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java index 5b27118222..46e8c98510 100644 --- a/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java +++ b/tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java @@ -69,6 +69,7 @@ import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import java.io.ByteArrayInputStream; import java.io.DataInput; @@ -508,11 +509,11 @@ public void testGetACLFailure() throws Exception { () -> dam.getContext().getApplicationACLs()); dam.start(); dam.stop(); - dam.mockShutdown.shutdownTime = Date.from(Instant.ofEpochMilli(Time.now())); + Mockito.when(dam.mockShutdown.getShutdownTime()).thenReturn(Date.from(Instant.ofEpochMilli(Time.now()))); LambdaTestUtils.intercept(TezUncheckedException.class, " Cannot get ApplicationACLs before all services have started, " + "The current service state is STOPPED. The shutdown hook started at " - + dam.mockShutdown.shutdownTime, () -> dam.getContext().getApplicationACLs()); + + dam.mockShutdown.getShutdownTime(), () -> dam.getContext().getApplicationACLs()); } @Test From fa43d377edc132d017c8c6f23028e417bee80fe7 Mon Sep 17 00:00:00 2001 From: Ayush Saxena Date: Wed, 28 Aug 2024 21:25:29 +0530 Subject: [PATCH 6/6] FindBugs --- .../main/java/org/apache/tez/dag/app/DAGAppMaster.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index aeeebc96ef..9c7cc18b60 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -937,13 +937,13 @@ public void handle(DAGAppMasterEvent event) { protected class DAGAppMasterShutdownHandler { private AtomicBoolean shutdownHandled = new AtomicBoolean(false); private long sleepTimeBeforeExit = TezConstants.TEZ_DAG_SLEEP_TIME_BEFORE_EXIT; - private Date shutdownTime; + private long shutdownTime; public Date getShutdownTime() { - return shutdownTime; + return new Date(shutdownTime); } - public void setShutdownTime(Date shutdownTime) { + public void setShutdownTime(long shutdownTime) { this.shutdownTime = shutdownTime; } @@ -964,7 +964,7 @@ public void shutdown(boolean now) { synchronized (shutdownHandlerRunning) { shutdownHandlerRunning.set(true); - setShutdownTime(new Date(System.currentTimeMillis())); + setShutdownTime(System.currentTimeMillis()); } LOG.info("Handling DAGAppMaster shutdown");