Skip to content

TEZ-4270: HeartbeatHandlerBase Does Not Need Stop Condition - #93

Merged
abstractdog merged 2 commits into
apache:masterfrom
belugabehr:TEZ-4270
Dec 26, 2021
Merged

TEZ-4270: HeartbeatHandlerBase Does Not Need Stop Condition#93
abstractdog merged 2 commits into
apache:masterfrom
belugabehr:TEZ-4270

Conversation

@belugabehr

Copy link
Copy Markdown
Contributor

No description provided.

@belugabehr

Copy link
Copy Markdown
ContributorAuthor

Tests failed because no unit tests added, but since this is an optimization, there is no change in behavior, no tests required. Please consider for merge.

@belugabehr

Copy link
Copy Markdown
ContributorAuthor

@abstractdog Can you please take a look? :)

@abstractdog
abstractdog self-requested a review December 25, 2021 16:18

@abstractdogabstractdog left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this patch makes sense to me, I cannot see any particular reason why the stopped flag is used (e.g. added as a bug fix, or any kind of specific logic using it as AsyncDispatcher), it's present since an initial commit

+1, pending tests

@belugabehr: could you please rebase (=retrigger) this PR to get test coverage

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec12m 35sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall12m 50smaster passed
+1 💚compile0m 42smaster passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚compile0m 38smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚checkstyle1m 10smaster passed
+1 💚javadoc0m 49smaster passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javadoc0m 35smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗spotbugs1m 38sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs1m 36smaster passed
_ Patch Compile Tests _
+1 💚mvninstall0m 25sthe patch passed
+1 💚compile0m 27sthe patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javac0m 27sthe patch passed
+1 💚compile0m 24sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚javac0m 24sthe patch passed
+1 💚checkstyle0m 17sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 21sthe patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javadoc0m 21sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚findbugs1m 7sthe patch passed
_ Other Tests _
+1 💚unit4m 22stez-dag in the patch passed.
+1 💚asflicense0m 14sThe patch does not generate ASF License warnings.
39m 49s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/3/artifact/out/Dockerfile
GITHUB PR#93
JIRA IssueTEZ-4270
Optional Testsdupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
unameLinux 3c16830fb73b 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / bf616b2
Default JavaPrivate Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/3/testReport/
Max. process+thread count188 (vs. ulimit of 5500)
modulesC: tez-dag U: tez-dag
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/3/console
versionsgit=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeComment
+0 🆗reexec16m 57sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+1 💚@author0m 1sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall13m 54smaster passed
+1 💚compile0m 45smaster passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚compile0m 39smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚checkstyle1m 8smaster passed
+1 💚javadoc0m 50smaster passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javadoc0m 33smaster passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗spotbugs1m 50sUsed deprecated FindBugs config; considering switching to SpotBugs.
+1 💚findbugs1m 48smaster passed
_ Patch Compile Tests _
+1 💚mvninstall0m 30sthe patch passed
+1 💚compile0m 35sthe patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javac0m 35sthe patch passed
+1 💚compile0m 27sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚javac0m 27sthe patch passed
+1 💚checkstyle0m 20sthe patch passed
+1 💚whitespace0m 0sThe patch has no whitespace issues.
+1 💚javadoc0m 26sthe patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚javadoc0m 25sthe patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚findbugs1m 25sthe patch passed
_ Other Tests _
+1 💚unit4m 34stez-dag in the patch passed.
+1 💚asflicense0m 14sThe patch does not generate ASF License warnings.
46m 21s
SubsystemReport/Notes
DockerClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/2/artifact/out/Dockerfile
GITHUB PR#93
JIRA IssueTEZ-4270
Optional Testsdupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
unameLinux 5f4dd0ba58e9 4.15.0-162-generic #170-Ubuntu SMP Mon Oct 18 11:38:05 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personalitypersonality/tez.sh
git revisionmaster / bf616b2
Default JavaPrivate Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/2/testReport/
Max. process+thread count178 (vs. ulimit of 5500)
modulesC: tez-dag U: tez-dag
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-93/2/console
versionsgit=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered byApache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

@abstractdog
abstractdog merged commit c9b8e90 into apache:masterDec 26, 2021
@abstractdog

Copy link
Copy Markdown
Contributor

merged to master, thanks @belugabehr for the patch!

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@belugabehr@tez-yetus@abstractdog