Skip to content

TEZ-4543: Throw a special exception to DagClient when there is no current DAG - #338

Merged
ayushtkn merged 2 commits into
apache:masterfrom
abstractdog:TEZ-4543
Feb 27, 2024
Merged

TEZ-4543: Throw a special exception to DagClient when there is no current DAG#338
ayushtkn merged 2 commits into
apache:masterfrom
abstractdog:TEZ-4543

Conversation

@abstractdog

@abstractdogabstractdog commented Feb 27, 2024•
edited
Loading

Copy link
Copy Markdown
Contributor

This is an AM + client-side change to make DagClient realize much faster when its asking for the status of a DAG which is not present.

@tez-yetus

This comment was marked as outdated.

Sign in to view
@tez-yetus

tez-yetus commented Feb 27, 2024

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec26m 57sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 1sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 2 new or modified test files.
_ master Compile Tests _
+0 🆗mvndep6m 3sMaven dependency ordering for branch
+1 💚mvninstall13m 34smaster passed
+1 💚compile1m 20smaster passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚compile1m 13smaster passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚checkstyle1m 27smaster passed
+1 💚javadoc1m 13smaster passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javadoc1m 9smaster passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+0 🆗spotbugs2m 2sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs4m 7smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall0m 51sthe patch passed
+1 💚compile1m 2sthe patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javac1m 2sthe patch passed
+1 💚compile0m 51sthe patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚javac0m 51sthe patch passed
+1 💚checkstyle0m 13stez-api: The patch generated 0 new + 73 unchanged - 1 fixed = 73 total (was 74)
+1 💚checkstyle0m 20sThe patch passed checkstyle in tez-dag
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 35sthe patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javadoc0m 35sthe patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚findbugs2m 24sthe patch passed
_ Other Tests _
+1 💚unit2m 14stez-api in the patch passed.
+1 💚unit5m 13stez-dag in the patch passed.
+1 💚asflicense0m 21sThe patch does not generate ASF License warnings.
73m 11s
SubsystemReport/Notes
DockerClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/1/artifact/out/Dockerfile
GITHUB PR#338
JIRA IssueTEZ-4543
Optional Testsdupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
unameLinux 11c00730d383 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / f8c2e11
Default JavaPrivate Build-1.8.0_392-8u392-ga-1~22.04-b08
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~22.04-b08
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/1/testReport/
Max. process+thread count486 (vs. ulimit of 5500)
modulesC: tez-api tez-dag U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/1/console
versionsgit=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ayushtkn
ayushtkn reviewed Feb 27, 2024
View reviewed changes

@ayushtknayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor comments, rest looks good

Comment threadtez-api/src/main/java/org/apache/tez/dag/api/NoCurrentDAGException.java Outdated
Comment on lines +24 to +25
* Fatal exception: thrown by the AM if there is no DAG running when
* when a DAG's status is queried. This is different from {@link org.apache.tez.dag.api.DAGNotRunningException}

@ayushtknayushtknFeb 27, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit
two times when, once in first line & then in second

@abstractdogabstractdogFeb 27, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:) fixing it

Comment threadtez-api/src/main/java/org/apache/tez/dag/api/client/DAGClientImpl.java Outdated
LOG.info("DAG is no longer running - application not found by YARN", e);
dagCompleted = true;
} catch (NoCurrentDAGException e) {
return dagLost(e);

@ayushtknayushtknFeb 27, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the exception e isn't used in the method dagLost, we can remove that param., moreover I think we can explore having a info log with the exception before returning

@abstractdogabstractdogFeb 27, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed

@abstractdog

abstractdog commented Feb 27, 2024

Copy link
Copy Markdown
ContributorAuthor

minor comments, rest looks good

thanks, fixed in ba93717

@tez-yetus

tez-yetus commented Feb 27, 2024

Copy link
Copy Markdown

🎊 +1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec0m 30sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
+1 💚test4tests0m 0sThe patch appears to include 2 new or modified test files.
_ master Compile Tests _
+0 🆗mvndep6m 1sMaven dependency ordering for branch
+1 💚mvninstall11m 7smaster passed
+1 💚compile1m 18smaster passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚compile1m 9smaster passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚checkstyle1m 21smaster passed
+1 💚javadoc1m 10smaster passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javadoc0m 57smaster passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+0 🆗spotbugs1m 19sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs2m 55smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall0m 48sthe patch passed
+1 💚compile0m 57sthe patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javac0m 57sthe patch passed
+1 💚compile0m 46sthe patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚javac0m 46sthe patch passed
+1 💚checkstyle0m 13stez-api: The patch generated 0 new + 73 unchanged - 1 fixed = 73 total (was 74)
+1 💚checkstyle0m 20sThe patch passed checkstyle in tez-dag
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 34sthe patch passed with JDK Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1
+1 💚javadoc0m 34sthe patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~22.04-b08
+1 💚findbugs2m 15sthe patch passed
_ Other Tests _
+1 💚unit2m 13stez-api in the patch passed.
+1 💚unit4m 59stez-dag in the patch passed.
+1 💚asflicense0m 24sThe patch does not generate ASF License warnings.
42m 1s
SubsystemReport/Notes
DockerClientAPI=1.44 ServerAPI=1.44 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/3/artifact/out/Dockerfile
GITHUB PR#338
JIRA IssueTEZ-4543
Optional Testsdupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
unameLinux 1de697df0d9f 5.15.0-94-generic #104-Ubuntu SMP Tue Jan 9 15:25:40 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / f8c2e11
Default JavaPrivate Build-1.8.0_392-8u392-ga-1~22.04-b08
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.22+7-post-Ubuntu-0ubuntu222.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~22.04-b08
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/3/testReport/
Max. process+thread count473 (vs. ulimit of 5500)
modulesC: tez-api tez-dag U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-338/3/console
versionsgit=2.34.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

ayushtkn
ayushtkn approved these changes Feb 27, 2024
View reviewed changes

@ayushtknayushtkn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ayushtkn
ayushtkn merged commit 34bb628 into apache:masterFeb 27, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ayushtknayushtknayushtkn approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@abstractdog@tez-yetus@ayushtkn