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

Split pytest by 'slow_test' tag and run from different k8s pods to reduce premerge job duration #3241

Merged
merged 3 commits into from
Aug 18, 2021
Merged

Split pytest by 'slow_test' tag and run from different k8s pods to reduce premerge job duration #3241

merged 3 commits into from
Aug 18, 2021

Conversation

zhanga5
Copy link
Contributor

@zhanga5 zhanga5 commented Aug 17, 2021 •
edited by pxLi
Loading

part of #2731

split pytests by 'slow_test' tag in order to balance test duration and memory consumption from two running pods in parallel after identify two .py files that required long duration or big memory. Now premerge overall duration could be reduced to ~1:30h from ~2:30h

@zhanga5 zhanga5 linked an issue Aug 17, 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 Aug 17, 2021

build

revans2
revans2 reviewed Aug 17, 2021
View reviewed changes
Copy link
Collaborator

@revans2 revans2 left a comment

Choose a reason for hiding this comment

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

From Jenkins it still looks like the job took 2:17h to complete. From the log it appears that both jobs were running in parallel, so I really would like to understand why this not any faster.

jenkins/spark-premerge-build.sh Outdated Show resolved Hide resolved
@sameerz sameerz added the test Only impacts tests label Aug 17, 2021
@zhanga5
Copy link
Contributor Author

zhanga5 commented Aug 18, 2021

From Jenkins it still looks like the job took 2:17h to complete. From the log it appears that both jobs were running in parallel, so I really would like to understand why this not any faster.

Any change to Jenkinsfile will take effect after merge due to security reason. Please refer to Jenkins job pxLi-rapids_premerge-github from internal Blossom about the testing.

@zhanga5
Add slow_test tag for pytest files that required long duration or big…
9951675 
… memory

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@zhanga5
Split pytest by 'slow test' tag and run from different k8s pods to re…
c110be8 
…duce premerge job duration

and keep one Spark version of the unit test for premerge job

Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@zhanga5 zhanga5 changed the title Run slow test of pytest from separated (UT) pod to minimize premerge overall duration Split pytest by 'slow_test' tag and run from different k8s pods to reduce premerge job duration Aug 18, 2021
@pxLi
Copy link
Collaborator

pxLi commented Aug 18, 2021

build

@zhanga5
Simplify description of 'slow_test' mark
389add3 
Signed-off-by: Alex Zhang <alex4zhang@gmail.com>
@pxLi
Copy link
Collaborator

pxLi commented Aug 18, 2021

build

pxLi
pxLi approved these changes Aug 18, 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, thx~
We will need to deal w/ TODO later

TODO: this function should be named as 'integration_test()' but it would break backward compatibility

@pxLi pxLi merged commit a014c2c into NVIDIA:branch-21.10 Aug 18, 2021
@zhanga5 zhanga5 deleted the tag_slow_test branch August 18, 2021 09:19
@revans2
Copy link
Collaborator

revans2 commented Aug 18, 2021

This commented out 3 of the 4 sets of scala tests that were being run before. I asked about it in this PR and it was not addressed.

@zhanga5
Copy link
Contributor Author

zhanga5 commented Aug 19, 2021

This commented out 3 of the 4 sets of scala tests that were being run before. I asked about it in this PR and it was not addressed.

it's already replied in #3241 (comment) . Please let me know if you have any other comments

@pxLi pxLi mentioned this pull request Aug 19, 2021
Closed
@jlowe jlowe mentioned this pull request Aug 20, 2021
Merged
razajafri pushed a commit to razajafri/spark-rapids that referenced this pull request Aug 23, 2021
@zhanga5 @razajafri
Split pytest by 'slow_test' tag and run from different k8s pods to re…
5dbdbf2 
…duce premerge job duration (NVIDIA#3241)

* Add slow_test tag for pytest files that required long duration or big memory

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

* Split pytest by 'slow test' tag and run from different k8s pods to reduce premerge job duration

and keep one Spark version of the unit test for premerge job

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

* Simplify description of 'slow_test' mark

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

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

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

Assignees
No one assigned
Labels
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)
4 participants
@zhanga5 @pxLi @revans2 @sameerz