TEZ-4129: Delete intermediate attempt data for failed attempts for Shuffle Handler #72

shameersss1 · 2020-07-23T07:19:56Z

Delete intermediate attempt data for failed task attempts for Tez Shuffle Handler

abstractdog · 2021-12-27T12:08:47Z

@shameersss1: could you please rebase this PR and squash your commits? I believe ShuffleHandler changes should be reviewed and committed next year, we'll find a way to review those
I think Github PRs are going to make the conversation much easier

shameersss1 · 2021-12-29T05:55:30Z

@shameersss1: could you please rebase this PR and squash your commits? I believe ShuffleHandler changes should be reviewed and committed next year, we'll find a way to review those I think Github PRs are going to make the conversation much easier

@abstractdog Thanks for looking into this. I have rebased and squash merged the commit. Please review the changes.

tez-yetus · 2021-12-30T12:19:32Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	0m 29s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 1s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 3 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	4m 22s	Maven dependency ordering for branch
+1 💚	mvninstall	9m 8s	master passed
+1 💚	compile	2m 50s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	compile	2m 39s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	2m 34s	master passed
+1 💚	javadoc	2m 48s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	2m 31s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	0m 42s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 49s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 9s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 45s	the patch passed
+1 💚	compile	1m 40s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javac	1m 40s	the patch passed
+1 💚	compile	1m 30s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	1m 30s	the patch passed
-0 ⚠️	checkstyle	0m 11s	tez-plugins/tez-aux-services: The patch generated 2 new + 63 unchanged - 0 fixed = 65 total (was 63)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	1m 29s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	1m 23s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	findbugs	4m 5s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 0s	tez-api in the patch passed.
+1 💚	unit	0m 27s	tez-common in the patch passed.
+1 💚	unit	5m 19s	tez-runtime-library in the patch passed.
+1 💚	unit	4m 16s	tez-dag in the patch passed.
+1 💚	unit	2m 38s	tez-aux-services in the patch passed.
+1 💚	asflicense	0m 48s	The patch does not generate ASF License warnings.
		62m 51s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/2/artifact/out/Dockerfile
GITHUB PR	#72
JIRA Issue	TEZ-4129
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux a5b2e48fce9c 4.15.0-156-generic #163-Ubuntu SMP Thu Aug 19 23:31:58 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `c9b8e90`
Default Java	Private 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
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/2/artifact/out/diff-checkstyle-tez-plugins_tez-aux-services.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/2/testReport/
Max. process+thread count	1479 (vs. ulimit of 5500)
modules	C: tez-api tez-common tez-runtime-library tez-dag tez-plugins/tez-aux-services U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/2/console
versions	git=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

tez-dag/src/test/java/org/apache/tez/dag/app/dag/impl/TestTaskImpl.java

tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java

tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java

tez-plugins/tez-aux-services/src/main/java/org/apache/tez/auxservices/ShuffleHandler.java

shameersss1 · 2022-01-24T14:56:35Z

@abstractdog - Thanks for the review. I have addressed your comments. Please re-review the changes

tez-yetus · 2022-01-24T16:00:22Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 3s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 2 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	5m 4s	Maven dependency ordering for branch
+1 💚	mvninstall	9m 21s	master passed
+1 💚	compile	2m 52s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	compile	2m 41s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	2m 36s	master passed
+1 💚	javadoc	2m 49s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	2m 31s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	0m 43s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 54s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 46s	the patch passed
+1 💚	compile	1m 43s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javac	1m 43s	the patch passed
+1 💚	compile	1m 34s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	1m 34s	the patch passed
-0 ⚠️	checkstyle	0m 10s	tez-plugins/tez-aux-services: The patch generated 2 new + 63 unchanged - 0 fixed = 65 total (was 63)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	1m 31s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	1m 24s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	findbugs	4m 5s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 6s	tez-api in the patch passed.
+1 💚	unit	0m 28s	tez-common in the patch passed.
+1 💚	unit	4m 34s	tez-runtime-library in the patch passed.
+1 💚	unit	4m 25s	tez-dag in the patch passed.
+1 💚	unit	2m 43s	tez-aux-services in the patch passed.
+1 💚	asflicense	0m 48s	The patch does not generate ASF License warnings.
		64m 19s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/3/artifact/out/Dockerfile
GITHUB PR	#72
JIRA Issue	TEZ-4129
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux e77a7a965b00 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `1176386`
Default Java	Private 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
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/3/artifact/out/diff-checkstyle-tez-plugins_tez-aux-services.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/3/testReport/
Max. process+thread count	1485 (vs. ulimit of 5500)
modules	C: tez-api tez-common tez-runtime-library tez-dag tez-plugins/tez-aux-services U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/3/console
versions	git=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog · 2022-01-25T09:47:07Z

tez-plugins/tez-aux-services/src/test/java/org/apache/tez/auxservices/TestShuffleHandler.java

+                  ShuffleHandler.USERCACHE, user,
+                  ShuffleHandler.APPCACHE, appId.toString(), "dag_1/output/", appAttemptId});
+      File taskAttemptDir = new File(taskAttemptDirStr);
+      Assert.assertTrue("Task Attempt Directory does not exist!", taskAttemptDir.exists());


for clarity's sake, please create an Assert.assertFalse(..., taskAttemptDir.exists()) before you expect the file to appear, this way the unit test also verifies that the task attempt file wasn't present (it's not the point of the unit test, but it is worth doing)

abstractdog · 2022-01-25T10:00:34Z

@abstractdog - Thanks for the review. I have addressed your comments. Please re-review the changes

thanks @shameersss1 , 1 more very minor note

…uffle Handler

shameersss1 · 2022-01-26T02:19:57Z

@abstractdog - Thanks for the review. I have addressed your comments. Please re-review the changes

thanks @shameersss1 , 1 more very minor note

@abstractdog I have addressed that. Please re-review the changes

tez-yetus · 2022-01-26T03:23:27Z

🎊 +1 overall

Vote	Subsystem	Runtime	Comment
+0 🆗	reexec	1m 10s	Docker mode activated.
		_ Prechecks _
+1 💚	dupname	0m 0s	No case conflicting files found.
+1 💚	@author	0m 0s	The patch does not contain any @author tags.
+1 💚	test4tests	0m 0s	The patch appears to include 2 new or modified test files.
		_ master Compile Tests _
+0 🆗	mvndep	4m 59s	Maven dependency ordering for branch
+1 💚	mvninstall	9m 18s	master passed
+1 💚	compile	2m 52s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	compile	2m 39s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	2m 36s	master passed
+1 💚	javadoc	2m 48s	master passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	2m 34s	master passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	0m 42s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	4m 55s	master passed
		_ Patch Compile Tests _
+0 🆗	mvndep	0m 10s	Maven dependency ordering for patch
+1 💚	mvninstall	1m 48s	the patch passed
+1 💚	compile	1m 43s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javac	1m 43s	the patch passed
+1 💚	compile	1m 32s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	1m 32s	the patch passed
-0 ⚠️	checkstyle	0m 11s	tez-plugins/tez-aux-services: The patch generated 4 new + 63 unchanged - 0 fixed = 67 total (was 63)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	1m 29s	the patch passed with JDK Ubuntu-11.0.13+8-Ubuntu-0ubuntu1.20.04
+1 💚	javadoc	1m 23s	the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	findbugs	4m 7s	the patch passed
		_ Other Tests _
+1 💚	unit	2m 4s	tez-api in the patch passed.
+1 💚	unit	0m 27s	tez-common in the patch passed.
+1 💚	unit	4m 35s	tez-runtime-library in the patch passed.
+1 💚	unit	4m 26s	tez-dag in the patch passed.
+1 💚	unit	2m 44s	tez-aux-services in the patch passed.
+1 💚	asflicense	0m 49s	The patch does not generate ASF License warnings.
		64m 20s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/4/artifact/out/Dockerfile
GITHUB PR	#72
JIRA Issue	TEZ-4129
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux ccdbac750454 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `1176386`
Default Java	Private 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
checkstyle	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/4/artifact/out/diff-checkstyle-tez-plugins_tez-aux-services.txt
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/4/testReport/
Max. process+thread count	2100 (vs. ulimit of 5500)
modules	C: tez-api tez-common tez-runtime-library tez-dag tez-plugins/tez-aux-services U: .
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-72/4/console
versions	git=2.25.1 maven=3.6.3 findbugs=3.0.1
Powered by	Apache Yetus 0.12.0 https://yetus.apache.org

This message was automatically generated.

abstractdog

LGTM +1
I've included this patch to a cluster using tez shuffle handler, haven't found any regression
code looks straightforward and clean, I'm approving

letting this PR open for 24h if any other comments come in
@jteagles

…uffle Handler (apache#72) (Syed Shameerur Rahman reviewed by Laszlo Bodor)

shameersss1 force-pushed the TEZ-4129 branch from c2bc541 to fd976f3 Compare December 29, 2021 05:54

This comment has been minimized.

Sign in to view

shameersss1 force-pushed the TEZ-4129 branch from fd976f3 to 931618d Compare December 30, 2021 11:16