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

Add CI cache #1583

Merged
merged 10 commits into from
May 26, 2022
Merged

Add CI cache #1583

merged 10 commits into from
May 26, 2022

Conversation

ValarDragon
Copy link
Member

@ValarDragon ValarDragon commented May 25, 2022

What is the purpose of the change

Try adding a cache for go build data, to help speedup CI. Copied from mvdan here: https://github.com/mvdan/github-actions-golang/blob/master/.github/workflows/test.yml

cref #1578

Brief Changelog

  • Adds go cache to CI

Testing and Verifying

We will need to keep an eye on if there are edge cases that make this not work / give incorrect results, or things don't speedup. A bit hard to know this without it being in place though.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes? yes
  • Is a relevant changelog entry added to the Unreleased section in CHANGELOG.md? no (I don't think CI is in scope)
  • How is the feature or change documented? We need to figure out a strategy for if/how we want to document CI.

@ValarDragon ValarDragon requested a review from a team May 25, 2022 12:08
@codecov-commenter
Copy link

codecov-commenter commented May 25, 2022

Codecov Report

Merging #1583 (b1668ef) into main (31a28e3) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1583   +/-   ##
=======================================
  Coverage   19.53%   19.53%           
=======================================
  Files         242      242           
  Lines       32262    32262           
=======================================
  Hits         6303     6303           
  Misses      24804    24804           
  Partials     1155     1155           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31a28e3...b1668ef. Read the comment docs.

@ValarDragon
Copy link
Member Author

ValarDragon commented May 25, 2022

Compare the two runs

  1. (First run, cache miss) https://github.com/osmosis-labs/osmosis/runs/6591778494?check_suite_focus=true
  2. (Second run, cache hit) https://github.com/osmosis-labs/osmosis/runs/6591903758?check_suite_focus=true

This cut out over half the time! 7min -> 3min 13sec (No tests were cached here, presumably due to me doing a change in app.go which under current infra requires re-running every test! So this is just eliminating build times)

This suggests that the go build time was around 3 minutes in every CI run, perhaps getting something similar for docker tests would help as well! We'd likely want a different cache key for the docker tests though, since we don't want it to override the go test cache.

@ValarDragon
Copy link
Member Author

Last commit is trying to add a cache to test-e2e, may need a couple of go edits before we can see if it helps

@ValarDragon
Copy link
Member Author

Awesome, tests are still getting rebuilt / re-run! (Evident by last commit)

@ValarDragon
Copy link
Member Author

ValarDragon commented May 25, 2022

For Docker, Compare the two runs

(First run, cache miss) https://github.com/osmosis-labs/osmosis/runs/6592152484?check_suite_focus=true
(Second run, cache hit) https://github.com/osmosis-labs/osmosis/runs/6592518772?check_suite_focus=true

Time went from 9m31s -> 8m23s. However, concurrently it appeared github actions is having network latency problems. (The lint job and codeQL job was intermittently failing to connect to the internet for docker pull, and checkout the repo)

The e2e upgrade testing time shaved 1minute 40 seconds, however docker image build increased by 30 seconds. So we still need to separately figure out how to speedup docker image building (perhaps incremental build) and further speeding up the e2e tests.

@ValarDragon
Copy link
Member Author

Linter speed difference:

(Run with no cache) https://github.com/osmosis-labs/osmosis/runs/6591900239?check_suite_focus=true
(First run, cache miss) https://github.com/osmosis-labs/osmosis/runs/6593255081?check_suite_focus=true
(Second run, cache hit) https://github.com/osmosis-labs/osmosis/runs/6593340103?check_suite_focus=true

Time difference is 2m14s -> 36s. (The no cache run took 2m6s, so huge time saving regardless.)

@mergify mergify bot merged commit 20835e6 into main May 26, 2022
@mergify mergify bot deleted the dev/try_adding_ci_cache branch May 26, 2022 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants