Skip to content

fix(cache): fail the run when a cache hit's outputs can't be restored - #770

Merged
wan9chi merged 3 commits into
mainfrom
fix-cache-hit-restore-failure
Oct 2, 2026
Merged

wan9chi merged 3 commits into
mainfrom
fix-cache-hit-restore-failure

Conversation

@wan9chi

@wan9chi wan9chi commented Sep 27, 2026 •

Copy link
Copy Markdown
Member

Motivation

When a cache hit's output archive can't be restored, for example because it was deleted or is corrupt, vp run prints an error but exits 0, and both the summary and --last-details report the task as a cache hit. CI passes without the task's outputs. Remote hits are also recorded locally before they're restored, so a remote hit that can't be restored leaves a local entry that fails every later run the same way, even though the remote entry is fine.

Changes

  • A failed restore is saved in the summary as a new TaskResult::RestoreFailed. It counts as failed rather than as a hit, sets a non-zero exit code, and --last-details shows it as → Cache hit, but the outputs couldn't be restored (or Remote cache hit) followed by the error and its causes.
  • For a local hit, the error is now Cache restore failed. Run `vp cache clean` to clear the cache: failed to extract the output archive: … instead of Cache lookup failed: failed to restore cached outputs from <path>; …. The entry stays in the cache, so the hint is still needed. For a remote hit, it's Cache restore failed: failed to extract the output archive: …, without the hint, because nothing was saved locally.
  • A remote hit is saved locally only after its outputs are restored, so a failed restore just deletes the downloaded archive, and the next run fetches the remote entry again.
  • Tests: output_cache_test deletes the archive of a cached task and checks the failed hit, --last-details, and that the task executes again after vp cache clean. This runs on all platforms. remote_cache::restore_failure (ignored and not on Windows, like the other remote backend cases) makes a remote hit's restore fail and checks that the downloaded archive is removed and the next run fetches and restores the entry again. vtt rm --ext <suffix> <dir> removes the archive without hardcoding its name or the cache schema directory.

Closes #767

@wan9chi
wan9chi force-pushed the fix-cache-hit-restore-failure branch from 8282c74 to 7a0993c Compare September 27, 2026 16:40
@github-actions

github-actions Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  +0.50%  [ -6.00% ..  +8.82%]  overhead  +269.29%
dynamic/access             change  -0.15%  [ -1.60% ..  +1.85%]  overhead   +13.91%
dynamic/access-relative    change  -0.22%  [ -1.74% ..  +2.21%]  overhead   +60.53%
dynamic/access-contended   change  -0.69%  [ -3.76% ..  +1.39%]  overhead   +14.75%
static/launch              change  +1.99%  [ -4.32% ..  +7.33%]  overhead  +727.87%
static/access              change  -0.64%  [ -1.90% ..  +0.90%]  overhead  +808.63%
static/access-relative     change  +0.17%  [ -0.89% ..  +1.16%]  overhead +1385.11%
static/access-contended    change  -0.21%  [ -1.13% ..  +0.73%]  overhead +3162.54%

macos

dynamic/launch             change  -0.01%  [-10.67% .. +11.09%]  overhead  +238.39%
dynamic/access             change  +2.01%  [-13.42% .. +89.12%]  overhead    +8.45%
dynamic/access-relative    change  +0.74%  [ -6.43% ..  +6.72%]  overhead  +241.79%
dynamic/access-contended   change  +0.64%  [ -6.09% .. +14.89%]  overhead    +1.05%

windows

dynamic/launch             change  +0.35%  [ -5.37% ..  +8.05%]  overhead   +25.21%
dynamic/access             change  -0.51%  [-18.23% ..  +8.92%]  overhead    +1.74%
dynamic/access-relative    change  -1.86%  [-35.22% ..  +0.51%]  overhead    +1.19%
dynamic/access-contended   change  -0.89%  [-11.45% ..  +6.09%]  overhead    +2.20%

wan9chi and others added 2 commits October 2, 2026 10:37
A cache hit whose output archive can't be extracted now fails the run
with a non-zero exit code and is reported as failed, with its error, in
the run summary and `--last-details`. The entry and its archive are
removed from the local cache, so the next run misses instead of failing
the same way. Remote hits are recorded locally before they're restored,
so this covers them too; the next run fetches the remote entry again.

Closes #767

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A remote hit is now recorded locally only after its output archive is
extracted, so a failed restore leaves no entry to evict and only the
downloaded archive is removed. A local hit whose outputs can't be
restored is still evicted, but only if the entry still refers to the
archive that failed, so an entry another process recorded in the
meantime is kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi force-pushed the fix-cache-hit-restore-failure branch from 7a0993c to 2e24d9e Compare October 2, 2026 02:37
A local hit whose outputs can't be restored is no longer removed from
the cache. Its error suggests running `vp cache clean` instead, as on
main, while the cause chain stays `failed to extract the output
archive: <error>`. Remote hits are unchanged: they're recorded locally
only once their outputs are restored, so a failed restore only removes
the downloaded archive.

The fix is now listed under the remote caching changelog entry.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@wan9chi
wan9chi marked this pull request as ready for review October 2, 2026 03:36
@wan9chi
wan9chi merged commit 040e086 into main Oct 2, 2026
19 checks passed
@wan9chi
wan9chi deleted the fix-cache-hit-restore-failure branch October 2, 2026 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A cache hit whose outputs can't be restored exits 0 and is reported as a hit

1 participant