Skip to content

cache: preserve representations across refresh and failure - #4371

Closed
larry-dalmeida wants to merge 6 commits into
larry-almeida-zse-cache-dispatch-prfrom
larry-almeida-zse-cache-correctness-pr
Closed

larry-dalmeida wants to merge 6 commits into
larry-almeida-zse-cache-dispatch-prfrom
larry-almeida-zse-cache-correctness-pr

Conversation

@larry-dalmeida

@larry-dalmeida larry-dalmeida commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Summary of changes

Preserve eligible stale responses without extending SIE expiry, isolate Vary variants, prevent standalone 304 responses from overwriting cached bodies, and skip superseded queued refreshes.

These changes affect refresh, failure and variant-selection behavior.

How to Verify

Shared setup

mkdir skipper-review && cd skipper-review
git clone https://github.com/zalando/skipper.git skipper && cd skipper
for PR in 4369 4370 4371 4372 4384 4385 4386 4387; do
  git fetch origin pull/$PR/head:pr-$PR
  git worktree add ../skipper-pr-$PR pr-$PR
done
cd ..
git clone https://github.com/larry-dalmeida/skipper-cache-e2e-tests.git
cd skipper-cache-e2e-tests
make verify SKIPPER_CHECKOUT=../skipper-pr-4387

make verify starts or reuses Minikube, the mock origin and the stack; runs the smoke and coalescing checks; and validates supported dashboard queries. It prints a report path and exits non-zero on failure. For outage evidence, run make chaos SKIPPER_CHECKOUT=../skipper-pr-4387 NAME=origin-stale; it includes its own timed load. Requirements: Python 3.9+, Make, Bash, Git, curl, kubectl, Minikube, Docker and Go compatible with Skipper's go.mod. Free ports: 8080, 8081, 6379, 4318, 9091, 9999, 9911 (8089 for endless load). Dynamic URLs: .runtime/urls.txt; stop with make stop.

Validated on #4387 (d767f636): smoke: 6,398 requests, 0 failures; origin-stale: 155 failed revalidations across 77 routes. Smoke metric checks: 42 PASS, 2 fault-only NOT EXERCISED, 0 FAIL.

Focused tests

go -C ../skipper-pr-4371 test -race -count=1 ./filters/cache -run '^(TestCacheFilter_SIE_RetentionAfterProcessAndVolatileL2Restart|TestProxy_CacheVary304PreservesBodyMetadata|TestCacheFilter_QueuedRefresh|TestCacheFilter_SIE_WindowExpiresDuringFetch|TestCacheFilter_SIE_FailureClassesPreserveStoredResponse|TestCacheFilter_SIE_EligibilityAndFailureBoundaries|TestCacheFilter_Vary_RevalidationPreservesVariantKey|TestProxy_CacheableVary_ConcurrentColdAndWarmIsolation)$'

Expected: 8 focused tests pass, covering SIE retention, queued refresh, Vary isolation and 304 metadata. origin-stale shows 503s and eventual recovery. hard/* and swr/* share cache("30s","10s","15s") with SIE disabled; 503s may replace good entries with 10-second cached errors, so these are not an SIE comparison.

Checklist

Rollback Plan

Revert #4372 first if merged, then this PR. No cache data-format migration is required.

References & Additional Context

Depends on #4370; followed by #4372. Real 24-hour SIE exhaustion remains unverified.

E2E Testing Results

E2E runs exercising refresh and origin failure. Full results: larry-dalmeida/skipper-cache-e2e-tests/results

Test Scenarios:

Scenario Build Requests Failures Status
Origin Stale bda674b (top of stack) 50,870 33.3% (503s while origin was failing) ✓ Pass
Smoke 8a80d62e + local changes 6,270 0% ✓ Pass
Origin Stale 8a80d62e + local changes 44,288 26.7% (503s while origin was failing) ✓ Pass
Valkey Kill 8a80d62e + local changes 47,653 0% ✓ Pass

Validation Panels:

  • Hit/Miss/Stale Rate — cache behavior while the origin returned 503 (bda674b origin-stale): hits stay high (up to ~184 req/s), misses rise to ~28 req/s, and stale serves drop to ~0 during the failure, then recover
    Hit/Miss/Stale Rate

  • Origin Responses by Status — the same window at the mock origin: 503s peak at ~25 req/s, 200s fall to ~2 req/s and 304s to 0, then both resume once the origin recovers
    Origin Responses by Status

Not shown by these runs: the run recorded no refresh errors (all 1,554 background refreshes succeeded), so the Refresh Errors panel stays empty. No panel shows a stale entry being kept when a refresh fails, or which variant was selected.

Serve eligible stale responses on network and body-read errors, timeouts, and HTTP 500/502/503/504. Preserve their original storage timestamps through failed foreground and background refreshes, without extending the SIE window. Keep normal negative caching when no eligible stale response exists.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Require matching Vary dimensions before sharing cacheable cold responses, including when Authorization is in the cache key. Route mismatched cacheable followers through their variant singleflight key. Share variant entry and sentinel persistence across cold fetches, Response, and background refresh; retain the sentinel through SIE. Cover concurrent and warm Accept-Language requests through a real proxy, with and without Authorization keys.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Scale the full-cycle sequence with virtual time: seed a Vary response, restart the process, optionally wipe volatile L2, then age beyond freshness plus SWR. Retained L2 serves stale with original timestamps and 27s TTL for 3s fresh plus 24s SIE; losing both tiers correctly yields origin500/MISS. No runtime TTL changes.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Recheck the exact variant silently inside revalidation singleflight. Skip only fresh entries stored since queue admission, preserving retries after failed refreshes without retaining an in-flight guard.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Reject standalone 304 storage in the shared admission helper so RFC loopback Response cannot replace a stored representation with an empty conditional response before the refresh worker merges it. Keep validators and variant keys unchanged.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Cover both omitted Content-Length and selected representation length for en/de variants in RFC loopback and force direct/loopback modes. Both valid forms reproduce the baseline RFC empty-body overwrite.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
@larry-dalmeida
larry-dalmeida force-pushed the larry-almeida-zse-cache-correctness-pr branch from ebcd7ff to e003f9a Compare October 26, 2026 14:37
@zalando-robot

Copy link
Copy Markdown

Docker image "registry-write.opensource.zalan.do/teapot/skipper:e003f9a39de77e79e64be13be264d1099a2555e7" is not based on an approved base image. Any production deployment relying on this image will be blocked.

To create a compliant Docker image of your application, you must reference an allowed Docker image as its base image in your Dockerfile. This base image must come from the Zalando Container Registry namespace library and use a recommended version as listed in the documentation.

@larry-dalmeida

Copy link
Copy Markdown
Collaborator Author

Superseded by #4394. Changes consolidated into five focused commits with published e2e evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants