Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Enable parallel build for pre-merge job to reduce overall duration [skip ci] #3028

Merged
merged 4 commits into from
Jul 28, 2021
Merged

Enable parallel build for pre-merge job to reduce overall duration [skip ci] #3028

merged 4 commits into from
Jul 28, 2021

Conversation

zhanga5
Copy link
Contributor

@zhanga5 zhanga5 commented Jul 27, 2021 •
edited
Loading

and upload logs to github for failed stage only. It will run mvn verify and unit tests stages in parallel. If any one failed will abort another if not finished yet.

@zhanga5
Enable parallel build for pre-merge job to reduce overall duration
e318bb4 
and upload logs to github for failed stage only

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@zhanga5 zhanga5 linked an issue Jul 27, 2021 that may be closed by this pull request
Premerge build takes too long (3 hours) #2731
Closed
@zhanga5
Copy link
Contributor Author

zhanga5 commented Jul 27, 2021

build

pxLi
pxLi reviewed Jul 27, 2021
View reviewed changes
jenkins/spark-premerge-build-ut.sh Outdated Show resolved Hide resolved
jenkins/Jenkinsfile-blossom.premerge Outdated Show resolved Hide resolved
jenkins/Jenkinsfile-blossom.premerge Outdated Show resolved Hide resolved
@pxLi pxLi added build Related to CI / CD or cleanly building test Only impacts tests labels Jul 27, 2021
@zhanga5 zhanga5 force-pushed the premerge_in_parallel branch 2 times, most recently from 00220fc to f15e247 Compare July 27, 2021 03:49
@pxLi pxLi mentioned this pull request Jul 27, 2021
Merged
pxLi
pxLi reviewed Jul 27, 2021
View reviewed changes
jenkins/Jenkinsfile-blossom.premerge Outdated Show resolved Hide resolved
jenkins/Jenkinsfile-blossom.premerge Outdated Show resolved Hide resolved
@pxLi pxLi marked this pull request as draft July 27, 2021 06:14
@pxLi pxLi changed the title Enable parallel build for pre-merge job to reduce overall duration Enable parallel build for pre-merge job to reduce overall duration [skip ci] Jul 27, 2021
@zhanga5
Parameterize premerge build script to support run unit test independe…
ac0c5b6 
…ntly and other cases

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@pxLi
Copy link
Collaborator

pxLi commented Jul 27, 2021 •
edited
Loading

convert to draft to avoid accidental merge. We will instantly do following tests against this repo after merge. Please help review but let us to merge this PR, thanks!

tgravescs
tgravescs reviewed Jul 27, 2021
View reviewed changes
Copy link
Collaborator

@tgravescs tgravescs left a comment

Choose a reason for hiding this comment

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

so to be clear, this is running mvn verify in parallel with the unit tests, correct. So basically build and integration tests run in one and unit tests run in another? can you update description to clarify.

jenkins/Jenkinsfile-blossom.premerge Outdated Show resolved Hide resolved
@zhanga5
Refine some comments per review comments
a67b30d 
Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@zhanga5
Copy link
Contributor Author

zhanga5 commented Jul 28, 2021

so to be clear, this is running mvn verify in parallel with the unit tests, correct. So basically build and integration tests run in one and unit tests run in another? can you update description to clarify.

yes, that's right. Updated description and added more comments to Jenkinsfile

@pxLi pxLi marked this pull request as ready for review July 28, 2021 01:20
pxLi
pxLi approved these changes Jul 28, 2021
View reviewed changes
Copy link
Collaborator

@pxLi pxLi left a comment

Choose a reason for hiding this comment

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

LGTM.
Alex has verified in forked repo, we are going trigger a PR build right after the merge.

@pxLi
Copy link
Collaborator

pxLi commented Jul 28, 2021

build

@pxLi pxLi merged commit d795552 into NVIDIA:branch-21.10 Jul 28, 2021
@zhanga5 zhanga5 deleted the premerge_in_parallel branch July 28, 2021 01:53
pxLi pushed a commit to pxLi/spark-rapids that referenced this pull request Jul 28, 2021
@zhanga5 @pxLi
Enable parallel build for pre-merge job to reduce overall duration [s…
5d8c0c3 
…kip ci] (NVIDIA#3028)

* Enable parallel build for pre-merge job to reduce overall duration

and upload logs to github for failed stage only

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>

* Refined based on review comments

* Parameterize premerge build script to support run unit test independently and other cases

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>

* Refine some comments per review comments

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
pxLi added a commit that referenced this pull request Jul 28, 2021
@pxLi
Merge pull request #3053 from pxLi/update-parallel-in-21.08
da72e5f 
cherry-pick #3028 [skip ci]
zhanga5 added a commit to zhanga5/spark-rapids that referenced this pull request Aug 9, 2021
@zhanga5
Enable parallel build for pre-merge job to reduce overall duration [s…
5eee291 
…kip ci] (NVIDIA#3028)

* Enable parallel build for pre-merge job to reduce overall duration

and upload logs to github for failed stage only

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>

* Refined based on review comments

* Parameterize premerge build script to support run unit test independently and other cases

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>

* Refine some comments per review comments

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tgravescs tgravescs tgravescs left review comments

@pxLi pxLi pxLi approved these changes

@GaryShen2008 GaryShen2008 Awaiting requested review from GaryShen2008 GaryShen2008 is a code owner

@jlowe jlowe Awaiting requested review from jlowe jlowe is a code owner

@NvTimLiu NvTimLiu Awaiting requested review from NvTimLiu NvTimLiu is a code owner

@revans2 revans2 Awaiting requested review from revans2 revans2 is a code owner

Assignees
No one assigned
Labels
build Related to CI / CD or cleanly building test Only impacts tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Premerge build takes too long (3 hours)
3 participants
@zhanga5 @pxLi @tgravescs