TEZ-4450: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent by ganeshashree · Pull Request #243 · apache/tez

ganeshashree · 2022-10-11T07:55:23Z

Fixed failure in fetching shuffle data when it's transferred via CompositeRoutedDataMovementEvent.
Added unit test to test the failure scenario.

Jira ticket: https://issues.apache.org/jira/browse/TEZ-4450

abstractdog · 2022-10-11T07:57:28Z

thanks for the patch @ganeshashree, could you please open a jira ticket for this?

ganeshashree · 2022-10-11T07:58:49Z

@abstractdog Jira ticket is https://issues.apache.org/jira/browse/TEZ-4450. Please review.

…ia CompositeRoutedDataMovementEvent

tez-yetus · 2022-10-11T11:54:06Z

🎊 +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 _
+1 💚	mvninstall	16m 2s	master passed
+1 💚	compile	0m 44s	master passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	compile	0m 39s	master passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	1m 12s	master passed
+1 💚	javadoc	0m 48s	master passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 35s	master passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+0 🆗	spotbugs	1m 31s	Used deprecated FindBugs config; considering switching to SpotBugs.
+1 💚	findbugs	1m 30s	master passed
		_ Patch Compile Tests _
+1 💚	mvninstall	0m 23s	the patch passed
+1 💚	compile	0m 26s	the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javac	0m 26s	the patch passed
+1 💚	compile	0m 21s	the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	javac	0m 21s	the patch passed
+1 💚	checkstyle	0m 16s	tez-runtime-library: The patch generated 0 new + 23 unchanged - 1 fixed = 23 total (was 24)
+1 💚	whitespace	0m 0s	The patch has no whitespace issues.
+1 💚	javadoc	0m 20s	the patch passed with JDK Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 19s	the patch passed with JDK Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
+1 💚	findbugs	1m 0s	the patch passed
		_ Other Tests _
+1 💚	unit	5m 32s	tez-runtime-library in the patch passed.
+1 💚	asflicense	0m 15s	The patch does not generate ASF License warnings.
		32m 17s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-243/3/artifact/out/Dockerfile
GITHUB PR	#243
JIRA Issue	TEZ-4450
Optional Tests	dupname asflicense javac javadoc unit spotbugs findbugs checkstyle compile
uname	Linux c7cd89b43789 4.15.0-191-generic #202-Ubuntu SMP Thu Aug 4 01:49:29 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	personality/tez.sh
git revision	master / `921de53`
Default Java	Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.16+8-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_342-8u342-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-243/3/testReport/
Max. process+thread count	2091 (vs. ulimit of 5500)
modules	C: tez-runtime-library U: tez-runtime-library
Console output	https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-243/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

looks good to me, only a minor question

abstractdog · 2022-10-23T07:44:56Z

tez-runtime-library/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker

@@ -0,0 +1,13 @@
+#   Licensed under the Apache License, Version 2.0 (the "License");


does this extra file have any advantages compared to adding this below as a dependency:

<dependency> <groupId>org.mockito</groupId> <artifactId>mockito-inline</artifactId> <version>xxx</version> <scope>test</scope> </dependency>

@abstractdog Both options work, the difference is just the extra file versus extra dependency. As per this doc, mockito-inline artifact likely to be discontinued once mocking of final classes and methods gets integrated into the default mock maker. Please let me know if adding mockito-inline dependency looks better.

thanks for clarifying, I'm fine with this file then

abstractdog

+1

…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor) (cherry picked from commit 8ebc4b0)

…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor) (cherry picked from commit 8ebc4b0) (cherry picked from commit 94da1f2)

…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor) (cherry picked from commit 8ebc4b0) (cherry picked from commit 94da1f2) (cherry picked from commit 238df23)

…nsferred via CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor) (#20) (cherry picked from commit 8ebc4b0) (cherry picked from commit 94da1f2) (cherry picked from commit 238df23) Co-authored-by: Ganesha Shreedhara <ganeshashree2@gmail.com>

ganeshashree changed the title ~~Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent~~ TEZ-4075: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent Oct 11, 2022

ganeshashree changed the title ~~TEZ-4075: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent~~ TEZ-4450: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent Oct 11, 2022

ganeshashree force-pushed the TEZ-4450 branch from 32fb372 to e248c03 Compare October 11, 2022 08:00

This comment was marked as outdated.

Sign in to view

TEZ-4450: Shuffle data fetch fails when shuffle data is transferred v…

c934afb

…ia CompositeRoutedDataMovementEvent

ganeshashree force-pushed the TEZ-4450 branch from e248c03 to c934afb Compare October 11, 2022 11:21

abstractdog requested changes Oct 23, 2022

View reviewed changes

abstractdog self-requested a review October 26, 2022 07:49

abstractdog approved these changes Oct 26, 2022

View reviewed changes

abstractdog merged commit 8ebc4b0 into apache:master Oct 26, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TEZ-4450: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent#243

TEZ-4450: Shuffle data fetch fails when shuffle data is transferred via CompositeRoutedDataMovementEvent#243
abstractdog merged 1 commit intoapache:masterfrom
ganeshashree:TEZ-4450

ganeshashree commented Oct 11, 2022

Uh oh!

abstractdog commented Oct 11, 2022

Uh oh!

ganeshashree commented Oct 11, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

tez-yetus commented Oct 11, 2022

Uh oh!

abstractdog left a comment

Uh oh!

abstractdog Oct 23, 2022

Uh oh!

ganeshashree Oct 26, 2022

Uh oh!

abstractdog Oct 26, 2022

Uh oh!

abstractdog left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -0,0 +1,13 @@
		# Licensed under the Apache License, Version 2.0 (the "License");

Conversation

ganeshashree commented Oct 11, 2022

Uh oh!

abstractdog commented Oct 11, 2022

Uh oh!

ganeshashree commented Oct 11, 2022

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

tez-yetus commented Oct 11, 2022

Uh oh!

abstractdog left a comment

Choose a reason for hiding this comment

Uh oh!

abstractdog Oct 23, 2022

Choose a reason for hiding this comment

Uh oh!

ganeshashree Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

abstractdog Oct 26, 2022

Choose a reason for hiding this comment

Uh oh!

abstractdog left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants