-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
pnpm cache race condition when using different node versions between workflows #641
Comments
Possibly related #286 |
Hello @allbetter-max , thank you for your input we started to investigate this issue |
@dsame thank you, highly appreciated. If you are able to provide an ETA or a workaround that would be great. |
Hello @maximveksler, can you please give us your estimations of "prolonged time" and a sample of race condiitons. From that we have now the time used for sharing the common cache is less then downloading 2 caches and having a common pnpm cache instead of per-project node_modules is exactly that pnpm is intended for. If you have some issues specific use cases the https://github.com/actions/cache action should be used for fine turning. |
@dsame please see reproduction and analay of the bug, This repo contains 2 workflows
1 app Called pnpm why muhammara
Legend: production dependency, optional only, dev only
[email protected] /home/m/code/pr/CoreLess/apps/demo
dependencies:
hummus-recipe 2.0.1
└── muhammara 1.10.0 Bug ReproductionIn total 3 commits were performed into this repo: 6f80832maximveksler/actions-setup-node-641@6f80832 is just the initial setup, and is irrelevant in this context. 9891f00maximveksler/actions-setup-node-641@9891f00 which triggers the action's cache generation. In this commit the 2 workflows have ran, Tests which uses node version 14, it takes 15 sec to run. This is bacuse muhammara 1.10.0 has precompiled build for node14 (as can be seen from the build logs In addition it generated cahce key
Version which uses node version 16, it takes 2m 55s to run. This is because muhammara 1.10.0 does not have a precompiled build for node16 (as can be seen from the build logs
So the job spends the 2:55 minutes no compliing the package. In addition it tries to cache the generated pnpm cahce, but correctly fails (because such cahce key already exists, as it was generated by Tests)
3b1dff9maximveksler/actions-setup-node-641@3b1dff9 which attempts to use the cache key `` for both workflows. In this commit the 2 workflows have ran again, Tests which uses node version 14, it takes 13 sec to run. It restore the cache key from the previous run, and pnpm uses it as expected:
Version which uses node version 16, it takes 3m 58s to run. Here the cache is also restored, much like the in Tests workflow
However pnpm does not finds the complied Root CauseThe bug is that it finds a cache to restore, however it's not the relevant cache because not all relevant variables are taken into account upon the cache token generation. Therefor the I belive that a simple solution would be to add to the hashed cahce token the version of the node version and the glibc version being used on the system. This would help prevent false positive of cache hits. |
To prove that Version can be cached, Please:
I won't be doing that in the repo, to not confuse the reader. |
Hello @maximveksler , thanks a lot for your explanation, now i understand the race condition but it does not seem to me it directly relates to the node version but rather it relates to the running workflows in parallel having the same lock file. The node version plays the role only if the same lock file resolves to the binary packages that is not built, i agree. Meanwhile i advise you to utilize the actions/cache that allows to set the custom cache. |
Thank you.
Sure, I'll cache externally. Are you able to suggest the best approach for caching a pnpm store as a workaround? |
This step is proven to work. It is not perfect because of hard-coded cached folder path, but it can be temporary solution until the feature request is not implemented.
|
The following setup worked for me, as pnpm is being installed using - uses: pnpm/action-setup@v2
with:
version: 7
- name: Install Node.js
uses: actions/setup-node@v3
with:
node-version: 16
- name: pnpm cache
uses: actions/cache@v3
with:
path: "/home/runner/setup-pnpm/node_modules/.bin/store/v3"
key: "pnpm-deps-node16"
- run: pnpm install --aggregate-output --reporter append-only --frozen-lockfile |
Even better steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0
- name: Install Node.js
id: setup-node
uses: actions/setup-node@v3
with:
node-version: 16
- name: Setup pnpm
uses: pnpm/action-setup@v2
with:
version: 7
- id: pnpm-cache-path
run: |
echo "STORE_PATH=$(pnpm store path)" >> $GITHUB_OUTPUT
- name: pnpm cache
uses: actions/cache@v3
with:
path: ${{ steps.pnpm-cache-path.outputs.STORE_PATH }}
key: ${{ runner.os }}-${{ steps.setup-node.outputs.node-version }}-pnpm-store-${{ hashFiles('**/pnpm-lock.yaml') }}
restore-keys: |
${{ runner.os }}-${{ steps.setup-node.outputs.node-version }}-pnpm-store-
- run: pnpm install --aggregate-output --reporter append-only --frozen-lockfile
|
Description:
Same cache us being reused, regardless of node versions. This brings prolonged time for once of the operations, and cache race conditions.
As can be seen from the images attached below, same cache key is used for both, but once is using node14, while the other is using node16.
Action version:
actions/checkout@v3
Platform:
Runner type:
Tools version:
Repro steps:
This is my Tests yaml
This is my Version yaml
Expected behaviour:
Node version is taken into account on the cache generation key.
Actual behaviour:
Same cache is being used, which leads to longer
pnpm install
steps, and cache race conditions.The text was updated successfully, but these errors were encountered: