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

Fixing Github actions caching (issue #529) #530

Merged
merged 3 commits into from
Oct 22, 2023
Merged

Fixing Github actions caching (issue #529) #530

merged 3 commits into from
Oct 22, 2023

Conversation

0rphee
Copy link
Contributor

@0rphee 0rphee commented Oct 17, 2023

Solves #529

@tomjaguarpaw
Copy link
Collaborator

If we look at the first run of this PR we see, under "ubuntu-latest / ghc-8.4.4"

Cache saved with key: Linux-8.8.4-c98baee664c078d7ae377d063fcd91f0b6bf10fc9058d0d829eab5f403c1ec18

but then when I rerun it, also under "ubuntu-latest / ghc-8.4.4) we see

Cache not found for input keys: Linux-8.8.4-73b3e145e5bc780eaa510f8ab4d0bf369bbad9b5d2cfa558e1dac36ea7dea422

I don't understand why it's trying to use a different key. I guess the order of update, configure, freeze needs to be corrected, but I'm not sure how.

@0rphee
Copy link
Contributor Author

0rphee commented Oct 18, 2023 •
edited
Loading

Hmm that's weird... I'll check in my fork what are the differences in the cabal.project.freeze.

@0rphee
Copy link
Contributor Author

0rphee commented Oct 18, 2023 •
edited
Loading

@tomjaguarpaw I think I've found the issue. The hash of the cabal.project.freezediffers due to its last line, which tracks the index state of cabal & hackage. Take this two runs:

First

...
index-state: hackage.haskell.org 2023-10-18T14:41:26Z

Second

...
index-state: hackage.haskell.org 2023-10-18T16:48:34Z

Here are the whole files if you want to run a diff between them:

Details 

active-repositories: hackage.haskell.org:merge
constraints: any.Cabal ==3.0.1.0 || ==3.10.2.0,
             any.Cabal-syntax ==3.10.2.0,
             any.HUnit ==1.6.2.0,
             any.QuickCheck ==2.14.3,
             QuickCheck -old-random +templatehaskell,
             any.alex ==3.4.0.0,
             any.ansi-terminal ==1.0,
             ansi-terminal -example,
             any.ansi-terminal-types ==0.11.5,
             any.ansi-wl-pprint ==0.6.9,
             ansi-wl-pprint -example,
             any.array ==0.5.4.0,
             any.async ==2.2.4,
             async -bench,
             any.barbies ==2.0.4.0,
             any.base ==4.13.0.0,
             any.base-compat ==0.13.1,
             any.base-orphans ==0.9.1,
             any.base64 ==0.4.2.3,
             any.binary ==0.8.7.0,
             any.blaze-builder ==0.4.2.3,
             any.blaze-html ==0.9.1.2,
             any.blaze-markup ==0.8.3.0,
             any.boring ==0.2.1,
             boring +tagged,
             any.bytestring ==0.10.10.1,
             any.cabal-doctest ==1.0.9,
             any.call-stack ==0.4.0,
             any.case-insensitive ==1.2.1.0,
             any.clay ==0.14.0,
             any.colour ==2.3.6,
             any.colourista ==0.1.0.2,
             any.concurrent-output ==1.10.20,
             any.constraints ==0.14,
             any.containers ==0.6.2.1,
             any.cryptohash-sha1 ==0.11.101.0,
             any.data-array-byte ==0.1.0.1,
             any.deepseq ==1.4.4.0,
             any.dir-traverse ==0.2.3.0,
             any.directory ==1.3.6.0,
             any.distributive ==0.6.2.1,
             distributive +semigroups +tagged,
             any.prettyprinter ==1.7.1,
             prettyprinter -buildreadme +text,
             any.prettyprinter-ansi-terminal ==1.1.3,
             any.primitive ==0.8.0.0,
             any.process ==1.6.9.0,
             any.quickcheck-io ==0.2.0,
             any.random ==1.2.1.1,
             any.relude ==1.2.1.0,
             any.resourcet ==1.3.0,
             any.rts ==1.0,
             any.safe-exceptions ==0.1.7.4,
             any.scientific ==0.3.7.0,
             scientific -bytestring-builder -integer-simple,
             any.selective ==0.7,
             any.slist ==0.2.1.0,
             any.splitmix ==0.1.0.5,
             splitmix -optimised-mixer,
             any.stm ==2.5.0.0,
             any.tagged ==0.8.8,
             tagged +deepseq +transformers,
             any.template-haskell ==2.15.0.0,
             any.terminal-size ==0.3.4,
             any.terminfo ==0.4.1.4,
             any.text ==1.2.4.0,
             any.text-short ==0.1.5,
             text-short -asserts,
             any.tf-random ==0.5,
             any.time ==1.9.3,
             any.tomland ==1.3.3.2,
             tomland -build-play-tomland -build-readme,
             any.transformers ==0.5.6.2,
             any.transformers-base ==0.4.6,
             transformers-base +orphaninstances,
             any.transformers-compat ==0.7.2,
             transformers-compat -five +five-three -four +generic-deriving +mtl -three -two,
             any.trial ==0.0.0.0,
             any.trial-optparse-applicative ==0.0.0.0,
             any.trial-tomland ==0.0.0.0,
             any.type-equality ==1,
             any.unix ==2.7.2.2,
             any.unliftio-core ==0.2.1.0,
             any.unordered-containers ==0.2.19.1,
             unordered-containers -debug,
             any.validation-selective ==0.2.0.0,
             any.wl-pprint-annotated ==0.1.0.1
index-state: hackage.haskell.org 2023-10-18T14:41:26Z

Details 

active-repositories: hackage.haskell.org:merge
constraints: any.Cabal ==3.0.1.0 || ==3.10.2.0,
             any.Cabal-syntax ==3.10.2.0,
             any.HUnit ==1.6.2.0,
             any.QuickCheck ==2.14.3,
             QuickCheck -old-random +templatehaskell,
             any.alex ==3.4.0.0,
             any.ansi-terminal ==1.0,
             ansi-terminal -example,
             any.ansi-terminal-types ==0.11.5,
             any.ansi-wl-pprint ==0.6.9,
             ansi-wl-pprint -example,
             any.array ==0.5.4.0,
             any.async ==2.2.4,
             async -bench,
             any.barbies ==2.0.4.0,
             any.base ==4.13.0.0,
             any.base-compat ==0.13.1,
             any.base-orphans ==0.9.1,
             any.base64 ==0.4.2.3,
             any.binary ==0.8.7.0,
             any.blaze-builder ==0.4.2.3,
             any.blaze-html ==0.9.1.2,
             any.blaze-markup ==0.8.3.0,
             any.boring ==0.2.1,
             boring +tagged,
             any.bytestring ==0.10.10.1,
             any.cabal-doctest ==1.0.9,
             any.call-stack ==0.4.0,
             any.case-insensitive ==1.2.1.0,
             any.clay ==0.14.0,
             any.colour ==2.3.6,
             any.colourista ==0.1.0.2,
             any.concurrent-output ==1.10.20,
             any.constraints ==0.14,
             any.containers ==0.6.2.1,
             any.cryptohash-sha1 ==0.11.101.0,
             any.data-array-byte ==0.1.0.1,
             any.deepseq ==1.4.4.0,
             any.dir-traverse ==0.2.3.0,
             any.directory ==1.3.6.0,
             any.distributive ==0.6.2.1,
             distributive +semigroups +tagged,
             any.prettyprinter ==1.7.1,
             prettyprinter -buildreadme +text,
             any.prettyprinter-ansi-terminal ==1.1.3,
             any.primitive ==0.8.0.0,
             any.process ==1.6.9.0,
             any.quickcheck-io ==0.2.0,
             any.random ==1.2.1.1,
             any.relude ==1.2.1.0,
             any.resourcet ==1.3.0,
             any.rts ==1.0,
             any.safe-exceptions ==0.1.7.4,
             any.scientific ==0.3.7.0,
             scientific -bytestring-builder -integer-simple,
             any.selective ==0.7,
             any.slist ==0.2.1.0,
             any.splitmix ==0.1.0.5,
             splitmix -optimised-mixer,
             any.stm ==2.5.0.0,
             any.tagged ==0.8.8,
             tagged +deepseq +transformers,
             any.template-haskell ==2.15.0.0,
             any.terminal-size ==0.3.4,
             any.terminfo ==0.4.1.4,
             any.text ==1.2.4.0,
             any.text-short ==0.1.5,
             text-short -asserts,
             any.tf-random ==0.5,
             any.time ==1.9.3,
             any.tomland ==1.3.3.2,
             tomland -build-play-tomland -build-readme,
             any.transformers ==0.5.6.2,
             any.transformers-base ==0.4.6,
             transformers-base +orphaninstances,
             any.transformers-compat ==0.7.2,
             transformers-compat -five +five-three -four +generic-deriving +mtl -three -two,
             any.trial ==0.0.0.0,
             any.trial-optparse-applicative ==0.0.0.0,
             any.trial-tomland ==0.0.0.0,
             any.type-equality ==1,
             any.unix ==2.7.2.2,
             any.unliftio-core ==0.2.1.0,
             any.unordered-containers ==0.2.19.1,
             unordered-containers -debug,
             any.validation-selective ==0.2.0.0,
             any.wl-pprint-annotated ==0.1.0.1
index-state: hackage.haskell.org 2023-10-18T16:48:34Z

I think this could only be solved if we copy the file, removing the last line, and use the hash of that for the cache key. Tell me if I should try that.

@tomjaguarpaw
Copy link
Collaborator

Interesting! But doesn't the cabal update put that there? In which case it's unclear to me why caching doesn't work in the absence of the update.

@0rphee
Copy link
Contributor Author

0rphee commented Oct 18, 2023

I'm afraid that it seems not to be the case.

Here's the freeze file without running cabal update previously:

https://github.com/0rphee/stan/actions/runs/6565222577/job/17833263692#step:5:144

It keeps that last line.

@tomjaguarpaw
Copy link
Collaborator

Yes, sorry, I wasn't clear. I meant doesn't the most recent cabal update determine what that line is, so that in the absence of cabal update the line will be the same as it was last time?

@0rphee
Copy link
Contributor Author

0rphee commented Oct 18, 2023 •
edited
Loading

I'm inclined to think that in fact, when running cabal freeze, the hackage index is updated. As the machines do not keep state, they should need to check the index at least once: when running cabal freeze. That would be the reason why that timestamp changes every CI run. The cause for the few cache hits would be that maybe the hackage server "saves" the index, every few minutes, and consecutive runs might be in that window, depending on the speed of the runner.

Here's the timestamp of a more recent run without cabal update. It changes respective to the last one:

https://github.com/0rphee/stan/actions/runs/6565928553/job/17835539008#step:5:146

@tomjaguarpaw
Copy link
Collaborator

When I run cabal freeze locally I get a Hackage state of a few days ago, when I last ran cabal update, so I don't think cabal freeze itself changes the state. However, probably the Haskell setup action does! We might need to think of a clever way around this (or just look at whatever everyone else is doing about this -- we can't be the only ones with this problem).

@0rphee
Copy link
Contributor Author

0rphee commented Oct 18, 2023

I checked a bunch of repos and found this:

text, trifecta & bytestring don't use the freeze file for the cache key. flatparse does, but it seems that the same happens to them.

megaparsec, monad-validate, libsodium-bindings & mtl use restore-keys which according to the cache action documentation, are: An ordered list of prefix-matched keys to use for restoring stale cache if no cache hit occurred for key.. For example, one run does this with a cache hit:

key: Linux-9.2.8-0-89634a009158c04583fe9a47c7afda61e0815b183dea219af1c4a71e2dfd242c
restore-keys: Linux-9.2.8-0-

So, I see two alternatives:

  1. do something like I previously proposed, 'ignoring' the index timestamp.
  2. use restore-keys, like the other repos

@tomjaguarpaw
Copy link
Collaborator

OK, let's try calculating the hash from a file with the timestamp removed!

@0rphee 0rphee force-pushed the main branch 4 times, most recently from 7535bd4 to db99b0e Compare October 20, 2023 19:43
@0rphee
Copy link
Contributor Author

0rphee commented Oct 20, 2023

@tomjaguarpaw I think this has done the trick!

However, I suppose that if/when, dependencies are added, the whole cache will get thrown away, but I suppose that due to the state of the project, that won't happen very often, if at all lol.

Just another two things: 1) Perhaps your comment will no longer be valid? and 2) while checking the other repos, I saw that some of them also cached dist-newstyle. Maybe we could do this too?

tomjaguarpaw referenced this pull request in tomjaguarpaw/trial Oct 21, 2023
@tomjaguarpaw
Configure before freeze
9112e03 
otherwise we can't find a build plan
andreasabel
andreasabel reviewed Oct 21, 2023
View reviewed changes
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Show resolved Hide resolved
0rphee added a commit to 0rphee/stan that referenced this pull request Oct 21, 2023
@0rphee
Use dist-newstyle/cache/plan.json as the cache key
f09fa9f 
kowainik#530 (comment)
@0rphee
Copy link
Contributor Author

0rphee commented Oct 21, 2023 •
edited
Loading

@tomjaguarpaw It now seems to work just fine.

andreasabel
andreasabel approved these changes Oct 22, 2023
View reviewed changes
Copy link

@andreasabel andreasabel left a comment

Choose a reason for hiding this comment

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

Excellent!

@tomjaguarpaw tomjaguarpaw merged commit 71f4b7e into kowainik:main Oct 22, 2023
39 checks passed
@tomjaguarpaw
Copy link
Collaborator

That's great, thanks @0rphee and @andreasabel!

The "Setup Haskell" stage takes a long time so I wonder if we can cache that too: #536

@tomjaguarpaw tomjaguarpaw mentioned this pull request Oct 22, 2023
Closed
@tomjaguarpaw
Copy link
Collaborator

Hmm, but now the run for main doesn't seem to be using the cache:

https://github.com/kowainik/stan/actions/runs/6603780564/job/17937257826

No idea why not. Let's keep our eyes on this and see what happens.

@tomjaguarpaw
Copy link
Collaborator

Oh, I think that's because the caches are per-branch. So this is probably fine.

@tomjaguarpaw
Copy link
Collaborator

This seems to explain the policy: https://docs.github.com/en/actions/using-workflows/caching-dependencies-to-speed-up-workflows#restrictions-for-accessing-a-cache

A PR branched off main can use main's cache, but main cannot use the cache that was saved by a PR.

So I think we are good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomjaguarpaw tomjaguarpaw tomjaguarpaw approved these changes

@andreasabel andreasabel andreasabel approved these changes

@vrom911 vrom911 Awaiting requested review from vrom911 vrom911 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@0rphee @tomjaguarpaw @andreasabel