Uh oh!
There was an error while loading. Please reload this page.
TEZ-4718: Modernize Jenkins and Yetus integration to ensure full build and UT run on every PR - #498
Conversation
c425d38 to
e146d9cComparetez-yetus
commented
May 15, 2026
🎊 +1 overall
This message was automatically generated. |
…d and UT run on every PR
tez-yetus
commented
May 15, 2026
🎊 +1 overall
This message was automatically generated. |
Phew! Now, it is running build and UT. The changes are inspired from apache phoenix project. Still few things needs to be iron out. Need to add |
tez-yetus
commented
May 15, 2026
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
commented
May 16, 2026
@abstractdog , the UT ran without the *.java extension changes. I would appreciate a feedback on this behaviour. |
tez-yetus
commented
May 16, 2026
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
commented
May 16, 2026
So, between Run7 and Run8 the the build time has reduced from:
Point to Ponder:
Here is breakdown of each
Question, I think keeping the compile stage makes more sense as it takes less time and check the project for sytax error. Also, |
abstractdog
commented
May 17, 2026
just for clarity's sake, this is not a new thing, right? |
Aggarwal-Raghav
commented
May 17, 2026
yeah yeah. --docker flag was present earlier as well |
abstractdog
commented
May 17, 2026
@Aggarwal-Raghav : left some syntax comments, also thought about your questions here: what about shooting for all with an "install" command, so having 2 stages (master + patch): and taking one more look, what value does the Lines 1236 to 1256 in 383ffd9 is it just for having custom compiler arguments, other than what we have in case of a simple mvn clean install command?also, I guess the |
| ## @stability evolving | ||
| ## @replaceable no | ||
| function personality_globals | ||
| { |
There was a problem hiding this comment.
I got used to not having a line break before {, looks strange, would you consider doing function personality_globals {? if you agree, please take care of the other methods too
There was a problem hiding this comment.
ACK. will handle it
| # shellcheck disable=SC2034 | ||
| PROJECT_NAME=tez | ||
| # shellcheck disable=SC2034 | ||
| BUILDTOOL=maven | ||
| # shellcheck disable=SC2034 | ||
| PATCH_BRANCH_DEFAULT=master | ||
| # shellcheck disable=SC2034 | ||
| JIRA_ISSUE_RE='^TEZ-[0-9]+$' | ||
| # shellcheck disable=SC2034 | ||
| GITHUB_REPO="apache/tez" | ||
| # Increase memory for maven | ||
| # shellcheck disable=SC2034 | ||
| MAVEN_OPTS="${MAVEN_OPTS:-"-Xmx4g -XX:+UseG1GC"}" | ||
| # Default Yetus settings | ||
| # shellcheck disable=SC2034 | ||
| DOCKER_MEMORY="20g" | ||
| # shellcheck disable=SC2034 | ||
| PROC_LIMIT=5500 |
There was a problem hiding this comment.
why not simply exporting these and getting rid of SC2034?
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Aggarwal-Raghav
commented
May 17, 2026
Most of the tez-personality.sh was inspired from https://github.com/apache/phoenix/blob/master/dev/phoenix-personality.sh will address your comments. |
Aggarwal-Raghav
commented
May 17, 2026
@abstractdog , regarding your above question on Let me share my understanding:
Let me clarify a thing:
|
tez-yetus
commented
May 17, 2026
💔 -1 overall
This message was automatically generated. |
UPDATE: this comment might be outdated, check the next one first: #498 (comment) I would go for "Option 1 (Maximum Strictness)", even if I like the optimization in the "Option 2 (The Sweet Spot)":
regarding discontinuing Yetus: this points too far and needs community consensus: I mean that's okay, but the PR hits the local optima, I'm really satisfied with short-/mid-term |
Aggarwal-Raghav
commented
May 18, 2026
@abstractdog , I'll create 2 dummy PR to test this jenkins change on TEZ-4708 and TEZ-4711. |
abstractdog
commented
May 18, 2026
hm, wait, I just realized again that github actions cover the tez/.github/workflows/build.yml Lines 49 to 50 in d0fa411 which is even better, as it runs on multiple platforms considering the minimal time difference between these options (on my laptop): so what about running full mvn install in github actions, and going for "Option 2 (The Sweet Spot)" in yetus? |
To clarify, you'll need to configure GitHub Actions to run automatically on pull requests. I assume you're currently initiating them manually for each pull request.. |
abstractdog
commented
May 18, 2026
yeah, and I think we should run the build workflow automatically, so it still points in the same direction |
Aggarwal-Raghav
commented
May 19, 2026
Run failed because of infra issue, will re-trigger |
In the latest run i.e 11, it is doing From tez-personality I can change the behaviour but I think that will be too hacky . So |
tez-yetus
commented
May 19, 2026
💔 -1 overall
This message was automatically generated. |
tez-yetus
commented
May 19, 2026
💔 -1 overall
This message was automatically generated. |
Aggarwal-Raghav
commented
May 19, 2026
Because of https://github.com/apache/yetus/blob/dda07307f7eac00257a33ec61cac86dbde8d1e6d/precommit/src/main/shell/plugins.d/checkstyle.sh#L196-L200 the Sorry for running too many CI 😔 |
tez-yetus
commented
May 19, 2026
💔 -1 overall
This message was automatically generated. |
abstractdog
commented
May 19, 2026
don't be sorry, you are testing CI, that's needed to make these valuable contributions happen! 🚀 |
Aggarwal-Raghav
commented
May 19, 2026
|
Aggarwal-Raghav
commented
May 20, 2026
@abstractdog , the PR is ready from my side. I think the improvement is good.
All in All we are adding approx 70 min of UT in every PR . spotbugs, linter, javadoc time will be dependent on PR changes as they are retaining the behaviour of running on submodule level. |
abstractdog
left a comment
There was a problem hiding this comment.
very nice @Aggarwal-Raghav , this looks good to me!
abstractdog
commented
May 20, 2026
it's fine, I'm glad we achieved full coverage |
Aggarwal-Raghav
commented
May 20, 2026
Thanks, once https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-498/14/ completes we can get this merged 👍🏻 |
Uh oh!
There was an error while loading. Please reload this page.
abstractdog
commented
May 20, 2026
oops, already merged...let's follow that run in case of any problems :) |
Aggarwal-Raghav
commented
May 20, 2026
yes. |
tez-yetus
commented
May 20, 2026
💔 -1 overall
This message was automatically generated. |
build-tools/docker/Dockerfileensuring a reproducible build environment.dev-support/tez-personality.shto centralize and strictly control Tez-specific test execution logic, moving intelligence out of the Jenkinsfile.spotbugs, checkstyle, javac, and javadocto run "smartly" (only on changed modules) to improve build times, while forcing full-project checks if critical files (like .proto or pom.xml) are modified.Flags REMOVED:
dev-support: Removed to allow Yetus to properly analyze and lint changes made to the CI infrastructure scripts themselves./dev-support/bin/shelldocs: Removed because the path doesn't exists; Use shellcheck bundled in the Docker image.PROC_LIMITinsidetez-personality.sh).DOCKER_MEMORYinsidetez-personality.sh).Flags ADDED:
.../tez-personality.sh: Added to load the newly created Tez-specific build and filtering logic.