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

cabal-*.project: index-state +1s #2515

Merged
merged 2 commits into from
Dec 21, 2021

Conversation

Anton-Latukha
Copy link
Collaborator

@Anton-Latukha Anton-Latukha commented Dec 20, 2021

It is made to fix the current situation, for example, if Caching workflow in master got canceled for some reason, on a successful run of the action in the workflows - workflows still runs the post-hooks that actions have, which would cache & share the cabal store of the canceled workflow.

On merge, this change would refresh caches, which should fix Windows build waits.

This is vacuous change, Hackage rerolls to the previous snapshot time
nevertheless of this change, but the caches get a new key namespace.
@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Dec 20, 2021

Ok. It currently fixes the cache situation for the time being.

In https://github.com/haskell/haskell-language-server/runs/4584037502?check_suite_focus=true we see that for Windows build the input (partial) cache was at 110-150Mb, & the output (full) cache is 203-300Mb.

@Anton-Latukha Anton-Latukha marked this pull request as ready for review December 20, 2021 17:04
@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Dec 20, 2021

@Anton-Latukha Anton-Latukha added the merge me Label to trigger pull request merge label Dec 20, 2021
@jneira
Copy link
Member

jneira commented Dec 20, 2021

Maybe it is a minor thing but with non existent hackage index timestamps all builds logs:

Warning: Requested index-state 2021-11-29T12:30:08Z is newer than
'hackage.haskell.org'! Falling back to older state (2021-11-29T12:28:17Z).

which is a little bit annoying.

I see the advantage of using artifical index state to invalidate cache but maybe we could update to the last index state given by cabal update
I mean we will have to update it sooner or later and ci will tell us if there is something wrong so we can try to find an older one.

@Anton-Latukha
Copy link
Collaborator Author

Anton-Latukha commented Dec 20, 2021

  1. The timestamp was reverting to 2021-11-29T12:28:17Z before also. In fact, that is why +1s - changes literally nothing, except running cache update, as cabal still uses the last discrete Hackage snapshot 2021-11-29T12:28:17Z.

  2. If I to update Hackage index not +1 second, but +1 908 282 seconds (~ time from that snapshot) - it is a bit other thing, and has all sort of bigger consequences - contributor would need to be ready & address all kinds of development updates - which expands the scope from fixing the cache for the project - to opening a number of reports to a number of places & asking people about syndromes, doing the project dependency update without discussion is that update desired in the first place - & so 1) that PR would be harder to do & 2) harder to pass review on 3) agenda of PR initially was to fix caching to continue work on the situation, it changes the scope and focus of the work overall.

PR just wanted to fix the caching situation for everybody.

Even the work on updating the index-state pinning to the current one - would be faster having the Windows cache restored before that.

Copy link
Member

@jneira jneira left a comment

Choose a reason for hiding this comment

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

ok, let it be +1, we will end to have a real one anyways and the actual one emits the warning, thanks for fixing cache

@jneira jneira merged commit 9bd5271 into haskell:master Dec 21, 2021
@Anton-Latukha
Copy link
Collaborator Author

Thank you.

I just was focused on the other thing in this PR & was not prepared to change the scope.

@Anton-Latukha Anton-Latukha deleted the 2021-12-20-upd-cache branch December 21, 2021 06:14
@Anton-Latukha
Copy link
Collaborator Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Label to trigger pull request merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants