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

Fix setup-rust cache #537

Open
Sec-ant opened this issue Jun 8, 2024 · 7 comments
Open

Fix setup-rust cache #537

Sec-ant opened this issue Jun 8, 2024 · 7 comments
Assignees

Comments

@Sec-ant
Copy link
Member

Sec-ant commented Jun 8, 2024

The current cache settings of the action setup-rust are not working well. We've reached the limit and the caches are not being restored or saved properly. This also slows down the codegen job.

I'll look into it.

@Sec-ant Sec-ant self-assigned this Jun 8, 2024
@Sec-ant
Copy link
Member Author

Sec-ant commented Jun 8, 2024

Should be fixed by #539.

@Sec-ant Sec-ant closed this as completed Jun 8, 2024
@Sec-ant Sec-ant reopened this Jun 9, 2024
@Sec-ant
Copy link
Member Author

Sec-ant commented Jun 9, 2024

The synchronize workflow will update Cargo.toml and Cargo.lock when it runs the codegen job (bump the revision of biome). And the cache key is hashed against the Cargo.lock file. That's why every time it runs, it will save the cache with a new cache key. Thus filling up the cache storage rapidly (10 Gb limit)

I'll try disabling caching provided by that action and handling the cache by ourselves.

@ematipico
Copy link
Member

Do we need caching at this point?

@Sec-ant
Copy link
Member Author

Sec-ant commented Jun 9, 2024

Do we need caching at this point?

Yes, or rust will build all the dependencies each time we run codegen.

@Sec-ant
Copy link
Member Author

Sec-ant commented Jun 9, 2024

After #552, we now use a single cache key rust-cache to restore and update the rust cache, the cache storage will no longer be filled by different rust caches and we can still speed up our build time with it.

image
image

We probably should do the same for the pnpm cache. Because each time when synchronize workflow is exectued, pnpm lockfile is also updated, so its cache is invalidated. So the cache is not being used, and only eating up the storage.
image

@Sec-ant Sec-ant closed this as completed Jun 9, 2024
@Sec-ant
Copy link
Member Author

Sec-ant commented Jun 11, 2024

I'm sorry for reopening this again. But the current workaround is not ideal for the following reasons:

  1. The cache paths are not properly cleaned before saving, that will result in a bigger and bigger cache file. We should take the clean-up functions in moonrepo/setup-rust for reference
  2. The cache saving action cannot be put in a post run step, which prolongs the job duration.
  3. The current workaround is not reusable.
  4. We need something good enough we can use for all our repos.

To address the above issues, I plan to fork https://github.com/moonrepo/setup-rust and do some experiments.

So I reopen this issue as a reminder.

@Sec-ant Sec-ant reopened this Jun 11, 2024
@ematipico
Copy link
Member

CI and caching, two difficult things combined! You're brave @Sec-ant 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants