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.

Sign up for GitHub

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

Jump to bottom

condfetch: Skip revalidation of an invalidated stale_oc #4082

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

condfetch: Skip revalidation of an invalidated stale_oc #4082

wants to merge 6 commits into from

Commits on Jun 18, 2024

  1. fetch: Split the startfetch step in two

    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    456a100 View commit details
    Browse the repository at this point in the history

  2. DRAFT: vcl: New restart(fetch) transition 

    TBD (missing documentation and coverage)
    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    6e9b09f View commit details
    Browse the repository at this point in the history

  3. condfetch: Bail out of 304 merger of dying stale_oc 

    A valid stale objcore may be selected during lookup, enabling a
conditional fetch from the backend. Depending on the backend, or
the resource being requested, this may leave a long window during
which the stale object may be invalidated.

In that case, a 304 response would defeat the invalidation, making
it possible for invalidated response header fields or bodies to be
perpetuated despite Varnish reporting a successful invalidation.

We should consider adding this to vcl_backend_error:

    if (beresp.was_304) {
            unset bereq.http.If-Modified-Since;
            unset bereq.http.If-None-Match;
            return (retry(fetch));
    }

This would mostly preserve the current behavior, as observed by a
client. In particular, when the ETag of the invalidated stale object
is still the same when a new copy is fetched, the client side INM
processing would remain. Varnish would simply no longer merge and
reuse invalidated stale objects internally.

From the point of view of an operator, there would of course be a
visible change in the presence of a retry.
    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    588d103 View commit details
    Browse the repository at this point in the history

  4. condfetch: Demote to fetch for dying stale_oc 

    When it is time to decide between making a bereq for a regular or
conditional fetch, we need to ensure that the stale object is still
valid. There is a window between the lookup and the begining of a
fetch task during which the object could have been invalidated. The
window is in theory much smaller than the one between the lookup and
the processing of a 304 response, but it can be significant if the
fetch task was queued or restarted.

Catching it early allows to proceed with a regular fetch.
    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    c461fb2 View commit details
    Browse the repository at this point in the history

  5. builtin: Split a new vcl_beresp_error subroutine

    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    b0d5920 View commit details
    Browse the repository at this point in the history

  6. builtin: Retry failed 304s by default 

    Named after vcl_backend_refresh from varnishcache#3994.
    @dridi
    dridi committed Jun 18, 2024
    Configuration menu
    Copy the full SHA
    2c12ab4 View commit details
    Browse the repository at this point in the history