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

CI: fix rust caching #134

Closed
ggwpez opened this issue Jan 4, 2024 · 8 comments · Fixed by #135
Closed

CI: fix rust caching #134

ggwpez opened this issue Jan 4, 2024 · 8 comments · Fixed by #135

Comments

@ggwpez
Copy link
Member

ggwpez commented Jan 4, 2024 •
edited
Loading

Eg here it prints No cache found although there should be some?!
Also seems to happen on other branches. Maybe @alvicsam knows how to fix it? Can we maybe even use this new Parity caching tool?

The check-migrations job could also need some caching for its build 😅
@bkchr bkchr mentioned this issue Jan 4, 2024
Merged
@bkchr
Copy link
Contributor

bkchr commented Jan 4, 2024

Will be fixed by: #135

ggwpez pushed a commit that referenced this issue Jan 4, 2024
@bkchr
CI: Use shared cache (#135)
4b6c871 
Closes: #134
@ggwpez
Copy link
Member Author

ggwpez commented Jan 9, 2024 •
edited
Loading

I still dont get whats going on.

In this job, it restored fine from key v0-rust-fellowship-cache-35cfef99, now in this one it fails to restore from the same key. The second job even ran after the first one 🤔

@bkchr
Copy link
Contributor

bkchr commented Jan 9, 2024

Hmm weird 🙈

@alvicsam alvicsam mentioned this issue Jan 12, 2024
Merged
@alvicsam
Copy link
Contributor

I removed shared-key in this PR #142 and now it runs faster because each workflow has its own cache. Usually the first run in a PR doesn't use cache (it might download it from the main branch but cargo will not use it). But then the cache action will upload the updated version and use it in the next runs. Also GitHub cache is only 10GB for repo and now one PR uses ~3GB of it. The good news is that GitHub doesn't erase the oldest cache immediately so it can be used for some time.

In this job, it restored fine from key v0-rust-fellowship-cache-35cfef99, now in this one it fails to restore from the same key. The second job even ran after the first one 🤔

In this case enabling debug for runners can help to understand what went wrong.

@bkchr
Copy link
Contributor

bkchr commented Jan 12, 2024

Wait, what? But what @ggwpez was for downloading the cache? Then we should really move to only generate the cache in the main branch and only download them in prs. I added the shared-key exactly to not always rebuild everything the first time a pr is opened.

@alvicsam
Copy link
Contributor

alvicsam commented Jan 12, 2024 •
edited
Loading

In this case each workflow (clippy, migrations and tests) should have unique shared-key. I added them in my PR.

@bkchr
Copy link
Contributor

bkchr commented Jan 14, 2024

Ahh each job! Ty!

fellowship-merge-bot bot added a commit that referenced this issue Jan 17, 2024
@alvicsam @fellowship-merge-bot
[ci] Remove shared key for cache and add cache for migrations (#142)
418a2a9 
PR removes shared key for rust-cache action. When 2 different workflows
use the same shared-key they try to use the same destination for cache
which makes it unusable for them (they have race condition who will be
the first to write there). Also PR adds cache for migration jobs and
cancelling jobs on previous runs.

cc #134

---------

Co-authored-by: fellowship-merge-bot[bot] <151052383+fellowship-merge-bot[bot]@users.noreply.github.com>
@ggwpez
Copy link
Member Author

ggwpez commented Jan 25, 2024

CI seems to work now and picks up cache across branches, thanks.

@ggwpez ggwpez closed this as completed Jan 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

CI: Use shared cache polkadot-fellows/runtimes
3 participants
@bkchr @ggwpez @alvicsam