Skip to content

Conversation

@abstractdog
Copy link
Contributor

@abstractdog abstractdog commented Sep 12, 2025

From Jira:

The proposal is to create a PluginManager to offload much of the plugin management code from the already overloaded DAGAppMaster class (including its static methods).
This issue was discovered while working on TEZ-4007, which only exacerbated the situation with the addition of AMExtensions (not AMExtensions themselves, but rather the original code in DAGAppMaster). Let’s clean this up first before proceeding with TEZ-4007.

Testing done with affected unit tests
mvn install -Dtest=TestTaskSchedulerManager,TestVertexImpl2,TestDAGAppMaster -pl ./tez-dag

@abstractdog abstractdog force-pushed the TEZ-4647 branch 4 times, most recently from 997d249 to a38ec96 Compare September 12, 2025 09:00
@tez-yetus

This comment was marked as outdated.

@tez-yetus

This comment was marked as outdated.

@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 Logfile Comment
+0 🆗 reexec 25m 31s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 detsecrets 0m 0s detect-secrets was not available.
+0 🆗 xmllint 0m 0s xmllint was not available.
+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 _
+1 💚 mvninstall 11m 49s master passed
+1 💚 compile 0m 46s master passed
+1 💚 checkstyle 1m 5s master passed
+1 💚 javadoc 0m 34s master passed
+0 🆗 spotbugs 2m 33s tez-dag in master has 785 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 33s the patch passed
+1 💚 codespell 0m 32s No new issues.
+1 💚 compile 0m 31s the patch passed
+1 💚 javac 0m 31s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 17s tez-dag: The patch generated 0 new + 105 unchanged - 1 fixed = 105 total (was 106)
+1 💚 javadoc 0m 13s the patch passed
+1 💚 spotbugs 1m 52s the patch passed
_ Other Tests _
+1 💚 unit 5m 7s tez-dag in the patch passed.
+1 💚 asflicense 0m 13s The patch does not generate ASF License warnings.
52m 55s
Subsystem Report/Notes
Docker ClientAPI=1.51 ServerAPI=1.51 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/9/artifact/out/Dockerfile
GITHUB PR #428
Optional Tests dupname asflicense codespell detsecrets xmllint javac javadoc unit spotbugs checkstyle compile
uname Linux 6eaece3b1b08 5.15.0-143-generic #153-Ubuntu SMP Fri Jun 13 19:10:45 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-home/workspace/tez-multibranch_PR-428/src/.yetus/personality.sh
git revision master / f0c4020
Default Java Ubuntu-21.0.8+9-Ubuntu-0ubuntu124.04.1
Test Results https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/9/testReport/
Max. process+thread count 243 (vs. ulimit of 5500)
modules C: tez-dag U: tez-dag
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/9/console
versions git=2.43.0 maven=3.8.7 spotbugs=4.9.3 codespell=2.0.0
Powered by Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

Copy link
Member

@ayushtkn ayushtkn left a comment

Choose a reason for hiding this comment

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

LGTM

public static void addDescriptor(List<NamedEntityDescriptor> list, BiMap<String, Integer> pluginMap,
NamedEntityDescriptor namedEntityDescriptor) {
list.add(namedEntityDescriptor);
pluginMap.put(list.get(list.size() - 1).getEntityName(), list.size() - 1);
Copy link
Member

Choose a reason for hiding this comment

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

can do getLast instead of size -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, absolutely

@tez-yetus
Copy link

🎊 +1 overall

Vote Subsystem Runtime Logfile Comment
+0 🆗 reexec 0m 32s Docker mode activated.
_ Prechecks _
+1 💚 dupname 0m 0s No case conflicting files found.
+0 🆗 detsecrets 0m 1s detect-secrets was not available.
+0 🆗 xmllint 0m 1s xmllint was not available.
+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 _
+1 💚 mvninstall 11m 38s master passed
+1 💚 compile 0m 45s master passed
+1 💚 checkstyle 1m 5s master passed
+1 💚 javadoc 0m 34s master passed
+0 🆗 spotbugs 2m 35s tez-dag in master has 785 extant spotbugs warnings.
_ Patch Compile Tests _
+1 💚 mvninstall 0m 32s the patch passed
+1 💚 codespell 0m 32s No new issues.
+1 💚 compile 0m 32s the patch passed
+1 💚 javac 0m 32s the patch passed
+1 💚 blanks 0m 0s The patch has no blanks issues.
+1 💚 checkstyle 0m 17s tez-dag: The patch generated 0 new + 105 unchanged - 1 fixed = 105 total (was 106)
+1 💚 javadoc 0m 12s the patch passed
+1 💚 spotbugs 1m 51s the patch passed
_ Other Tests _
+1 💚 unit 5m 5s tez-dag in the patch passed.
+1 💚 asflicense 0m 13s The patch does not generate ASF License warnings.
27m 46s
Subsystem Report/Notes
Docker ClientAPI=1.51 ServerAPI=1.51 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/10/artifact/out/Dockerfile
GITHUB PR #428
Optional Tests dupname asflicense codespell detsecrets xmllint javac javadoc unit spotbugs checkstyle compile
uname Linux b36734937aa2 5.15.0-143-generic #153-Ubuntu SMP Fri Jun 13 19:10:45 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Build tool maven
Personality /home/jenkins/jenkins-home/workspace/tez-multibranch_PR-428/src/.yetus/personality.sh
git revision master / 250a0e2
Default Java Ubuntu-21.0.8+9-Ubuntu-0ubuntu124.04.1
Test Results https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/10/testReport/
Max. process+thread count 243 (vs. ulimit of 5500)
modules C: tez-dag U: tez-dag
Console output https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-428/10/console
versions git=2.43.0 maven=3.8.7 spotbugs=4.9.3 codespell=2.0.0
Powered by Apache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog
Copy link
Contributor Author

thanks @ayushtkn for the review, I'm merging this soon after addressing your comment

@abstractdog abstractdog merged commit 261362e into apache:master Sep 15, 2025
4 checks passed
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