Skip to content

Conversation

@ganeshashree
Copy link
Contributor

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
Copy link
Contributor

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

@ganeshashree 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
Copy link
Contributor Author

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

@ganeshashree 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
@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@tez-yetus
Copy link

🎊 +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.

Copy link
Contributor

@abstractdog abstractdog left a comment

Choose a reason for hiding this comment

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

looks good to me, only a minor question

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

Choose a reason for hiding this comment

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

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>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

@abstractdog abstractdog self-requested a review October 26, 2022 07:49
Copy link
Contributor

@abstractdog abstractdog left a comment

Choose a reason for hiding this comment

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

+1

@abstractdog abstractdog merged commit 8ebc4b0 into apache:master Oct 26, 2022
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 11, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
(cherry picked from commit 94da1f2)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
(cherry picked from commit 94da1f2)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
(cherry picked from commit 94da1f2)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
(cherry picked from commit 94da1f2)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 12, 2024
…ia CompositeRoutedDataMovementEvent (apache#243) (Ganesha Shreedhara reviewed by Laszlo Bodor)

(cherry picked from commit 8ebc4b0)
(cherry picked from commit 94da1f2)
prabhjyotsingh pushed a commit to acceldata-io/tez that referenced this pull request Nov 20, 2024
…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)
prabhjyotsingh added a commit to acceldata-io/tez that referenced this pull request Nov 20, 2024
…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 <[email protected]>
shubhluck pushed a commit to acceldata-io/tez that referenced this pull request Nov 21, 2024
…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 <[email protected]>
Sign up for free to 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