TEZ-4718: Modernize Jenkins and Yetus integration to ensure full build and UT run on every PR#498
TEZ-4718: Modernize Jenkins and Yetus integration to ensure full build and UT run on every PR#498Aggarwal-Raghav wants to merge 7 commits into
Conversation
Aggarwal-Raghav
force-pushed
the
TEZ-4718
branch
4 times, most recently
from
c425d38 to
e146d9c
Compare
|
🎊 +1 overall
This message was automatically generated. |
…d and UT run on every PR
|
🎊 +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 |
|
💔 -1 overall
This message was automatically generated. |
|
@abstractdog , the UT ran without the *.java extension changes. I would appreciate a feedback on this behaviour. |
|
💔 -1 overall
This message was automatically generated. |
|
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, |
just for clarity's sake, this is not a new thing, right? |
yeah yeah. --docker flag was present earlier as well |
@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?
There was a problem hiding this comment.
Yes, will do.
| { | ||
| local filename=$1 | ||
|
|
||
| yetus_debug "Tez file filter: ${filename}" |
There was a problem hiding this comment.
I checked runs no.7 and no.8 and haven't seen "Tez file filter", am I supposed to see it?
There was a problem hiding this comment.
--debug flag is removed from Jenkinsfile as it was too much verbose. Once --debug is enabled then this Tez file filter: will be printed
There was a problem hiding this comment.
thanks, I'm fine with that, so in case of an evil pre-commit issue, --debug can be added to the jenkins pipeline script while manually Replaying the job
I'm usually against comments containing code, but in this case, a commented-out --debug option could be really helpful, what do you think?
There was a problem hiding this comment.
Sounds reasonable. Will address it by EOD.
|
Most of the tez-personality.sh was inspired from https://github.com/apache/phoenix/blob/master/dev/phoenix-personality.sh will address your comments. |
|
@abstractdog , regarding your above question on Let me share my understanding:
Let me clarify a thing:
|
|
💔 -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 |
|
@abstractdog , I'll create 2 dummy PR to test this jenkins change on TEZ-4708 and TEZ-4711. |
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.. |
yeah, and I think we should run the build workflow automatically, so it still points in the same direction |
…to strict warnings
|
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 |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
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 😔 |
|
💔 -1 overall
This message was automatically generated. |
don't be sorry, you are testing CI, that's needed to make these valuable contributions happen! 🚀 |
|
3 participants
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.