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.

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

ci pipeline improvements #3901

Merged
merged 2 commits into from
Apr 2, 2022
Merged

Conversation

mbien
Copy link
Member

@mbien mbien commented Mar 31, 2022

PR tests some optimizations / improvements.

use Zstandard compression for pipeline artifacts.

  • faster compression (multithreaded) and extraction compared to gz
  • smaller artifact footprint (with -9 setting, seems to be the sweet spot in my experience from other deployments)
  • all github nodes support it out of the box
  • cumulative performance improvement due to the nature of the pipeline

Local test using 2 threads to simulate a gh node. Tested in a ramdisk. The actual code uses -T0 to be forward compatible.

# compression
time tar -czf /tmp/build.tar.gz --exclude ".git" .
real    1m12,641s
user    1m18,657s
sys     0m3,167s

# uses two threads to simmulate CI node
time tar -I 'zstd -9 -T2' -cf /tmp/build.tar.zst --exclude ".git" .
real	0m22,993s
user	0m50,221s
sys	0m3,018s


# archive sizes
ls -l /tmp/build*
-rw-r--r-- 1 mbien mbien 1575854860 31. Mär 01:21 /tmp/build.tar.gz
-rw-r--r-- 1 mbien mbien 1431884800 31. Mär 01:25 /tmp/build.tar.zst


# extraction
time tar -xzf /tmp/build.tar.gz
real    0m14,681s
user    0m15,051s
sys     0m3,438s

time tar --zstd -xf /tmp/build.tar.zst
real    0m3,747s
user    0m2,399s
sys     0m3,006s

outside of gh actions I would have combined compression + upload in one step (same for download + extraction). But this isn't possible while using the default upload action.

second commit: don't cleanup on failure

  • don't trigger on failure since someone might want to restart a job
  • updated trigger is success() || cancelled()
  • retention period remains set to 1 day for missed cleanups
  • there is also the option to manually remove uploads if needed

@mbien mbien added enhancement CI Continuous Integration labels Mar 31, 2022
@mbien
Copy link
Member Author

mbien commented Mar 31, 2022

MacOS, please do your job, look at windows :)

Run tar --zstd -xf build.tar.zst
tar: Option --zstd is not supported
Usage:
  List:    tar -tf <archive-filename>
  Extract: tar -xf <archive-filename>
  Create:  tar -cf <archive-filename> [filenames...]
  Help:    tar --help
Error: Process completed with exit code 1.

results:

  • compression down from 1m 42s to 53s
  • extraction down from 34s to 16s.
  • mac has to use unzsd | tar as workaround

 - faster compression (multithreaded) and extraction compared to gz
 - smaller artifact footprint (with -9 setting)
 - all github nodes support it out of the box
 - tar on mac isn't aware of zstd, so it needs to be piped to tar after unzstd
 - don't trigger on failure since someone might want to restart a job
 - updated trigger is success() || cancelled()
 - retention period remains set to 1 day for missed cleanups
 - there is also the option to manually remove uploads if needed
@mbien mbien added this to the NB14 milestone Mar 31, 2022
@mbien mbien marked this pull request as ready for review March 31, 2022 07:23
Copy link
Contributor

@matthiasblaesing matthiasblaesing left a comment

Choose a reason for hiding this comment

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

Thank you - the final part (not deleting artifacts on failure) is IMHO the most important I had to restart the full pipeline to fix two jobs, which kind of defeats the purpose.

@mbien
Copy link
Member Author

mbien commented Apr 2, 2022

Thank you - the final part (not deleting artifacts on failure) is IMHO the most important I had to restart the full pipeline to fix two jobs, which kind of defeats the purpose.

Agreed. Thanks for the review. Lets get this merged so that job restarts can be used as advertised in the original PR.

@mbien mbien merged commit 5812c4c into apache:master Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous Integration enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants