test(ci): implement best practices for caching in workflows #17930
Conversation
it was previously on the sliding v2 tag and could update to unknown code without notice
use the separate actions/cache "restore" and "save" actions so we may only save optional based on whether we are on main or not actions/cache isolates branch caches from main to avoid branch cache poison attacks on main runs but branches may see main cache as those caches are considered trustworthy branch caches do consume available LRU storage though, meaning branch caches could evict main caches and in high traffic / large cache repositories like this one that means caches LRU out frequently and cache hit rates are terrible unless you cache on main only
very occasionally, there are errors in the `hashFiles` function used to calculate cache keys, and this causes a build error that is a flake, not a valid CI failure signal so, continue the workflow even if the caching setup has errors, as that is a recoverable problem and the workflow may still have a valid success outcome
github workflow caches are considered immutable, they will never update if there was a hit on the primary cache key ergo, if the primary cache key never changes the caches will never update and you will only ever get fresh caches by chance if stale caches are evicted via LRU cache storage limit management So, all cache primary keys are unique now according to the requirements of the specific cache - the AVD cache is now unique on the api-level/target/arch tuple, and that is set and referenced in an environment variable since it is now used in two places - the firebase emulator cache is set based on the firebase-tools version, which always changes when new emulator versions are downloaded
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. |
the downloaded/cached files are pure javascript and java, they may be shared across operating systems safely
mikehardy
force-pushed
the
@mikehardy/ci-caching-best-practices
branch
from
0166a64 to
730d901
Compare
|
All of the changes implemented here have either been working well already in the react-native-firebase repo for the parts I had implemented there long ago, or they were implemented in parallel with this one in invertase/react-native-firebase#8820 which is now merged and working well. I understand CI things are difficult to test, but as far as it goes - that's testing of the same changes and it looks good. Caches over there are now long-lasting where they should be, and updating where they shouldn't, but not thrashing. CI runs are quick |
1 participant
Description
👋 this is started with the work on #17871 from @SelaseKay
I worked with Jude on that change in 17871 and while I was in there I noticed that the caching setup for CI in this repo was likely to be subject to severe LRU thrashing and very very low hit rates, with low useful caches even with hits.
In fact when I went to verify that, it was true: https://github.com/firebase/flutterfire/actions/caches?query=sort%3Acreated-asc
The oldest cache is only 35 minutes old - and all the caches are from a branch which means main builds or builds from other branches can't even see them to use them. Thus the caching is for most intents and purposes a lot of computational work for zero benefit at the moment.
How to review this PR - Each commit in this series is a fully separate idea to improve things, and each should be reviewed separately by reading the commit message which explains that specific change then checking the diff for that commit
The basic idea is this though:
In addition, I pinned the Android Emulator runner to a concrete SHA as is practice in this repo
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]).This will ensure a smooth and quick review process. Updating the
pubspec.yamland changelogs is not required.///).melos run analyze) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?