Skip to content
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

Almost complete rewrite of cache filter #37990

Draft
wants to merge 24 commits into
base: main
Choose a base branch
from

Re-add accidentally removed line

38b1699
Select commit
Loading
Failed to load commit list.
Draft

Almost complete rewrite of cache filter #37990

Re-add accidentally removed line
38b1699
Select commit
Loading
Failed to load commit list.
CI (Envoy) / Envoy/Publish and verify failed Jan 24, 2025 in 30m 51s

Envoy/Publish and verify (failure)

Check has finished

Details

Check run finished (failure ❌)

The check run can be viewed here:

Envoy/Publish and verify (pr/37990/main@38b1699)

Check started by

Request (pr/37990/main@38b1699)

ravenblackx @ravenblackx 38b1699 #37990 merge main@ad2a1c7

Almost complete rewrite of cache filter

Commit Message: Almost complete rewrite of cache filter
Additional Description: WIP - does not yet support vary headers (turns out the old one only did it poorly too, but I guess we can't step one thing backwards to fix all the other things this fixes). This is necessarily a massive change because the old cache filter was almost non-functional. Important differences:

  1. it's now resilient against "thundering herd" problem - only one request goes upstream if there are multiple simultaneous requests for the same resource.
  2. it now works with range requests, which, if there previously were only range requests, as can happen with Microsoft BITS as a client, would all just bypass the cache. The new behavior is a range request causes a full file request upstream, and then serves the range response once the requested part of the file is cached. This could be improved by also supporting partial cache entries, but it's reasonable for that to be a later PR.
  3. a lot of work that the cache filter previously offloaded to the cache implementation is now owned by the cache filter (e.g. vary headers, validation, thundering herd resistance). This makes cache implementation much less onerous, but adds some intermediate layers.
    Risk Level: Pretty bad, but the filter is still tagged WIP, so not critical.
    Testing: There's quite a lot, but still needs a bit more coverage. Existing integration tests still function mostly as they were, and added integration tests for range and parallel requests which would have failed with the old cache.
    Docs Changes: Internal only.
    Release Notes: Not yet.
    Platform Specific Features: n/a
Environment

Request variables

Key Value
ref 8fda111
sha 38b1699
pr 37990
base-sha ad2a1c7
actor ravenblackx @ravenblackx
message Almost complete rewrite of cache filter...
started 1737751084.314012
target-branch main
trusted false
Build image

Container image/s (as used in this CI run)

Key Value
default envoyproxy/envoy-build-ubuntu:d2be0c198feda0c607fa33209da01bf737ef373f
mobile envoyproxy/envoy-build-ubuntu:mobile-d2be0c198feda0c607fa33209da01bf737ef373f
Version

Envoy version (as used in this CI run)

Key Value
major 1
minor 34
patch 0
dev true