Skip to content

fix: dispatch cache revalidation directly to the route backend - #4

Merged
larry-dalmeida merged 3 commits into
fix/cache-revalidation-use-original-requestfrom
fix/cache-revalidation-direct-backend-dispatch
Oct 2, 2026
Merged

larry-dalmeida merged 3 commits into
fix/cache-revalidation-use-original-requestfrom
fix/cache-revalidation-direct-backend-dispatch

Conversation

@larry-dalmeida

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

Copy link
Copy Markdown
Owner

Stacked on zalando#4315 - reuses its revalidationRequest() helper for the fallback path rather than reimplementing it. This PR is opened against the fork (both branches live here) so the diff shows only the incremental change; it should be retargeted to zalando/skipper:master once zalando#4315 merges upstream.

Problem

Background revalidation always looped back through skipper's own listener so the full filter chain could rerun. That re-entry has to re-match a route, which is fragile to any upstream filter mutation, not just the modPath prefix-strip zalando#4315 addressed, and there was no guarantee it would ever work for a given route's configuration.

Fix

For force-mode routes with a resolvable static backend (ctx.BackendUrl() non-empty), dispatch the revalidation fetch directly there using the already filter-transformed ctx.Request(). No re-routing, so no route-matching failure is possible, regardless of which filter mutated what.

RFC mode and load-balanced/dynamic backends (ctx.BackendUrl() empty) intentionally keep today's self-loopback behavior via zalando#4315's revalidationRequest():

  • RFC mode: a Response()-filter after cache() could rewrite Cache-Control; only the self-loopback path sees that rewritten value before doRevalidate reads it for TTL/SWR.
  • LB/dynamic backends: no single backend URL to dial directly.

Follow-up fix: RFC mode never actually reached revalidation

While verifying the RFC-mode branch above, found that it was dead in practice: CreateFilter only parses swrWindow for the force-mode (3-5 arg) form, so RFC-mode entries always had swrWindow == 0, and Entry.IsStale can never be true with a zero-width window. RFC-mode cache() routes could never reach the stale-serve-and-revalidate path at all - independent of the dispatch-target change above.

Added resolveSWR, a resolveTTL-style sibling:

  • Force mode: operator swrWindow stays authoritative (unchanged behavior).
  • RFC mode: honors the response's stale-while-revalidate=N directive (RFC 5861), now parsed by parseCacheControl.

This closes the gap rather than papering over it. The RFC-mode self-loopback rationale above still holds and is now load-bearing rather than moot, since RFC-mode revalidation can actually fire.

Follow-up fix: direct-dispatch revalidation was invisible to standard metrics

Direct dispatch calls f.fetch (a plain skpnet.Client) straight at the backend URL, never
re-entering Proxy.ServeHTTP. The self-loopback path's inner hop through skipper's own listener
is an ordinary proxied request, so it already produces the usual MeasureBackend*/MeasureServe
metrics and an access-log line; direct dispatch has no such inner hop, so none of that fires for
it - the only existing signal was the cache_revalidation trace span, shared with self-loopback.

Added cache.reval_backend_dispatch, incremented in doRevalidate exactly when a request is
dialed directly at backendURL rather than looped back. Gives a metrics-level way to see
direct-dispatch revalidation volume on dashboards that don't have tracing wired up, separate from
what the standard backend/access-log metrics already show for the self-loopback path.

Testing

  • TestCacheFilter_Revalidation_DirectDispatchToBackend: force mode + static backend dispatches to ctx.BackendUrl(), not the listener; now also asserts cache.reval_backend_dispatch == 1.
  • TestCacheFilter_Revalidation_LBBackendFallsBackToLoopback: empty BackendUrl() still falls back to self-loopback; now also asserts cache.reval_backend_dispatch == 0.
  • TestRevalidationDispatch_RFCModeIgnoresBackendUrl / TestRevalidationDispatch_ForceModeUsesBackendUrl: direct unit tests of the gating logic.
  • TestCacheFilter_Revalidation_RFCMode_StaleServedAndRevalidated (new): RFC-mode integration test proving the previously-dead path now fires end-to-end - entry stored via stale-while-revalidate=3600, served STALE after max-age expiry, and revalidated via self-loopback (localhost:9090) even with ctx.BackendUrl() set.
  • TestParseCacheControl (extended): new cases for stale-while-revalidate parsing (present, zero, malformed, combined with max-age).
  • Existing TestCacheFilter_Revalidation_UsesOriginalRequestPath from fix: make background cache revalidation reliable in force and RFC mode zalando/skipper#4315 still passes unchanged; it exercises the fallback path (empty BackendUrl()), noted in its updated doc comment.
  • Full filters/cache suite passes; two Valkey/testcontainers tests fail in this sandbox for an unrelated reason (no Docker available), same failures, unchanged, on master without this change.

Docs

docs/operation/operation.md's metrics table now documents cache.reval_backend_dispatch, including why direct-dispatch traffic needs it (invisible to the standard backend metrics/access log otherwise).

Background revalidation looped every request back through skipper's own
listener so the full filter chain could rerun on it. That re-entry has to
re-match a route, which is fragile to anything an earlier filter changed
(not just the modPath case zalando#4315 fixed) and was never guaranteed to work
in the first place.

For force-mode routes with a resolvable static backend (ctx.BackendUrl()
non-empty), dispatch the revalidation fetch directly there instead,
using the already filter-transformed ctx.Request() — no re-routing
needed, so no route-matching failure is possible.

RFC-mode routes and load-balanced/dynamic backends (ctx.BackendUrl()
empty) keep today's self-loopback behavior via zalando#4315's
revalidationRequest() fallback: RFC mode because a Response()-filter
positioned after cache() could rewrite Cache-Control, and only the
self-loopback path sees that rewritten value before doRevalidate reads
it for TTL purposes; LB/dynamic backends because there's no single
backend URL to dial directly.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
CreateFilter only parses swrWindow for the 3-5 arg (force-mode) form,
so RFC-mode entries always had a zero-width SWR window and could never
reach the stale-serve-and-revalidate path, regardless of this PR's
dispatch-target change for that path.

Add resolveSWR, a resolveTTL-style sibling that keeps the operator
swrWindow authoritative in force mode but, in RFC mode, honors the
response's stale-while-revalidate directive (RFC 5861) parsed by
parseCacheControl. Use it at all four sites that previously read
f.swrWindow directly.

revalidationDispatch's RFC-mode gating is unchanged: RFC mode still
always self-loopbacks regardless of ctx.BackendUrl(), since a
Response()-filter after cache() can rewrite Cache-Control and only the
self-loopback path sees that rewritten value before doRevalidate reads
it. That reasoning no longer depends on RFC-mode SWR being unreachable,
so the comment and this PR's description are updated accordingly.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
Direct-backend revalidation dispatch bypasses skipper's own proxy
pipeline (Proxy.ServeHTTP), so it never produces the usual
MeasureBackend*/MeasureServe metrics or an access-log line - those
only fire for the self-loopback path's inner hop back through
skipper's listener. Without this counter, direct-dispatch
revalidation traffic was invisible to anything but the
cache_revalidation trace span.

Increment it in doRevalidate at the point backendURL is applied to
the outgoing request, and assert it (==1 for direct dispatch, ==0
for self-loopback) in the existing dispatch-target tests.

Signed-off-by: Larry D Almeida <hello@larrydalmeida.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 2, 2026
@larry-dalmeida
larry-dalmeida merged commit 6d19df0 into fix/cache-revalidation-use-original-request Oct 2, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant