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: improve previous caches reuse #3187

Merged

Conversation

develop7
Copy link
Collaborator

@develop7 develop7 commented Jan 25, 2024

  • Enables setup-nix to try restoring more previous Nix caches, improving cache reuse
  • Lets Stack build jobs restore caches from previous runs

@develop7 develop7 changed the title ci: fix previous caches not being reused ci: relax previous caches reuse policy Jan 25, 2024
@develop7 develop7 changed the title ci: relax previous caches reuse policy ci: rollback cache-nix-action to v4 & improve previous caches reuse Jan 26, 2024
@develop7 develop7 changed the title ci: rollback cache-nix-action to v4 & improve previous caches reuse ci: improve previous caches reuse Jan 26, 2024
@wolfgangwalther
Copy link
Member

wolfgangwalther commented Jan 27, 2024

Has all of this been done in 411cf43 and 43f552d already, so that we can close this?

Edit: Ah, I think there are more cache restore keys here. Is that still relevant or were you just chasing bugs in cache-nix-action v5?

@develop7
Copy link
Collaborator Author

@wolfgangwalther yeah, still relevant. Couldn't wait for the #3190 to be merged.

@develop7 develop7 force-pushed the ci-fix-reuse_previous_caches branch 2 times, most recently from 148284a to 6f93b14 Compare January 29, 2024 15:22
@develop7 develop7 added the ci Related to CI setup label Jan 29, 2024
@wolfgangwalther
Copy link
Member

Enables setup-nix to try restoring more previous Nix caches, improving cache reuse

Lets Stack build jobs restore caches from previous runs

Seems like only the stack part is addressed now. Should this be reflected in the commit message, which still mentions both nix and stack caches, or were you missing those changes for the nix part?

@develop7
Copy link
Collaborator Author

develop7 commented Feb 1, 2024

Indeed, I've lost extra keys for nix during rebase. Thank you for the heads-up. Let me fix it in a sec.

@develop7 develop7 force-pushed the ci-fix-reuse_previous_caches branch from 9c34df7 to 2808cab Compare February 2, 2024 16:38
@develop7 develop7 marked this pull request as ready for review February 2, 2024 16:39
@wolfgangwalther wolfgangwalther merged commit a45058b into PostgREST:main Feb 5, 2024
33 checks passed
@wolfgangwalther
Copy link
Member

It seems like since we merged this, loadtest is failing with "no space left on device" errors. I suspect this is because the loadtest is actually restoring multiple caches, i.e. test-loadtest and common - and then runs out of diskspace. Is that a possible explanation, @develop7?

@wolfgangwalther
Copy link
Member

Hm. Reading the docs it seems that only one of those caches would be restored. Also https://github.com/PostgREST/postgrest/actions/caches tells me that we currently have two caches only - and only for the macos job. Something is entirely wrong here...

@develop7
Copy link
Collaborator Author

develop7 commented Feb 6, 2024

Exactly, (while I was toying multi-layer cache idea in my head) only one cache is restored, not according to the docs, but also to the logs — https://github.com/PostgREST/postgrest/actions/runs/7797378199/job/21263851394#step:5:83

My guess is Github cut their disc space allowance for action runners

@wolfgangwalther
Copy link
Member

My guess is Github cut their disc space allowance for action runners

Hm. At least not in general. #3211 seems to fix it, but I don't really know why. I just ran the loadtest on main again and it's still failing. So it does seem to be related to the changes that were made in this PR (which started to fail it) and to the changes in my PR (which seems to fix it). Looking at the logs, there is no cache restored on main at all right now.. so that's a bit strange.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Related to CI setup
Development

Successfully merging this pull request may close these issues.

2 participants