-
Notifications
You must be signed in to change notification settings - Fork 406
Description
Description:
If a repository has many caches being used between runs of actions/stale, such that the "_state" cache entry is not on the first page of results returned from the list caches API, actions/stale will fail to restore its state and then will fail to update the state.
Action version:
9.0.0
Platform:
- Ubuntu
- macOS
- Windows
Runner type:
- Hosted
- Self-hosted
Repro steps:
- Create a repo set up with
actions/stalewith a lowoperations-per-runand enough issues that a single run cannot process them all. - Have
actions/stalerun, which should create the "_state" cache entry. - Run other actions to create various other cache entries, until visiting
https://api.github.com/repos/{owner}/{repo}/actions/cachesno longer includes "_state" as it has been pushed to the second page of results. - Have
actions/stalerun again.
My test repo for this issue is at https://github.com/anomiex/test-stale.
Expected behavior:
For step 4, the state is restored correctly and processing continues from where the previous run left off.
Actual behavior:
In step 4, near the start, it reports
The saved state was not found, the process starts from the first issue.
Processing begins from the first issue. Then at the end it reports
Failed to save: Unable to reserve cache with key _state, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/master, Key: _state, Version: fa41d75081481069cfb6b92a5f83a94c6e06ef3ab2e6b762649ac5f86f46153f
Analysis:
The checkIfCacheExists function only checks the first page of results, using the default value of 30 entries per page.
stale/src/classes/state/state-cache-storage.ts
Lines 33 to 46 in 3f3b017
| const checkIfCacheExists = async (cacheKey: string): Promise<boolean> => { | |
| const client = getOctokitClient(); | |
| try { | |
| const issueResult = await client.request( | |
| `/repos/${context.repo.owner}/${context.repo.repo}/actions/caches` | |
| ); | |
| const caches: Array<{key?: string}> = | |
| issueResult.data['actions_caches'] || []; | |
| return Boolean(caches.find(cache => cache['key'] === cacheKey)); | |
| } catch (error) { | |
| core.debug(`Error checking if cache exist: ${error.message}`); | |
| } | |
| return false; | |
| }; |
While you could fix the bug by adding pagination so the relevant cache entry is found even if it's not on the first page, a more performant fix would be to make use of the
key and ref parameters to the list caches API to specify the branch and key that we actually care about.
I suppose doing both would be an even better idea, just in case some repo is creating a lot of other cache entries with the "_state" prefix.