Add fail action 6, will fallback to serving stale if retry attempts are exhausted

doc/admin-guide/files/records.yaml.en.rst

@@ -2892,7 +2892,7 @@ Dynamic Content & Content Negotiation
     The number of times to attempt a cache open write upon failure to get a write lock.
 
     This config is ignored when :ts:cv:`proxy.config.http.cache.open_write_fail_action` is
-    set to ``5`` or :ts:cv:`proxy.config.http.cache.max_open_write_retry_timeout` is set to gt ``0``.
+    set to ``5`` or ``6``, or when :ts:cv:`proxy.config.http.cache.max_open_write_retry_timeout` is set to gt ``0``.
 
 .. ts:cv:: CONFIG proxy.config.http.cache.max_open_write_retry_timeout INT 0
    :reloadable:
@@ -2901,7 +2901,7 @@ Dynamic Content & Content Negotiation
     A timeout for attempting a cache open write upon failure to get a write lock.
 
     This config is ignored when :ts:cv:`proxy.config.http.cache.open_write_fail_action` is
-    set to ``5``.
+    set to ``5`` or ``6``.
 
 .. ts:cv:: CONFIG proxy.config.http.cache.open_write_fail_action INT 0
    :reloadable:
@@ -2929,8 +2929,14 @@ Dynamic Content & Content Negotiation
          with :ts:cv:`proxy.config.cache.enable_read_while_writer` configuration
          allows to collapse concurrent requests without a need for any plugin.
          Make sure to configure the :ref:`admin-config-read-while-writer` feature
-         correctly. Note that this option may result in CACHE_LOOKUP_COMPLETE HOOK
-         being called back more than once.
+         correctly. With this option, CACHE_LOOKUP_COMPLETE HOOK is deferred for
+         read retries so that plugins see only the final cache lookup result.
+   ``6`` Retry Cache Read on a Cache Write Lock failure (same as ``5``), but if
+         read retries are exhausted and a stale cached object exists, serve the
+         stale content if allowed. This combines the request collapsing behavior
+         of ``5`` with the stale-serving fallback of ``2``. If stale is not
+         returnable (e.g., due to ``Cache-Control: must-revalidate``), go to
+         origin server.
    ===== ======================================================================
 
 Customizable User Response Pages

include/proxy/http/HttpConfig.h

@@ -371,6 +371,7 @@ enum class CacheOpenWriteFailAction_t {
   ERROR_ON_MISS_STALE_ON_REVALIDATE = 0x03,
   ERROR_ON_MISS_OR_REVALIDATE       = 0x04,
   READ_RETRY                        = 0x05,
+  READ_RETRY_STALE_ON_REVALIDATE    = 0x06,
   TOTAL_TYPES
 };
 

include/proxy/http/HttpTransact.h

@@ -487,6 +487,7 @@ class HttpTransact
     HTTPInfo         transform_store;
     CacheDirectives  directives;
     HTTPInfo        *object_read          = nullptr;
+    HTTPInfo        *stale_fallback       = nullptr; // Saved stale object for action 6 fallback during retry
     CacheWriteLock_t write_lock_state     = CacheWriteLock_t::INIT;
     int              lookup_count         = 0;
     SquidHitMissCode hit_miss_code        = SQUID_MISS_NONE;
@@ -703,6 +704,15 @@ class HttpTransact
     ///   configuration.
     bool is_cacheable_due_to_negative_caching_configuration = false;
 
+    /// Set when stale content is served due to cache write lock failure.
+    /// Used to correctly attribute statistics and VIA strings.
+    bool serving_stale_due_to_write_lock = false;
+
+    /// Set when CACHE_LOOKUP_COMPLETE hook is deferred for action 5/6.
+    /// The hook will fire later with the final result once we know if
+    /// stale content will be served or if we're going to origin.
+    bool cache_lookup_complete_deferred = false;
+
     MgmtByte cache_open_write_fail_action = 0;
 
     HttpConfigParams           *http_config_param = nullptr;
@@ -998,9 +1008,12 @@ class HttpTransact
   static void HandleCacheOpenReadHitFreshness(State *s);
   static void HandleCacheOpenReadHit(State *s);
   static void HandleCacheOpenReadMiss(State *s);
+  static void HandleCacheOpenReadMissGoToOrigin(State *s);
   static void set_cache_prepare_write_action_for_new_request(State *s);
   static void build_response_from_cache(State *s, HTTPWarningCode warning_code);
   static void handle_cache_write_lock(State *s);
+  static void handle_cache_write_lock_go_to_origin(State *s);
+  static void handle_cache_write_lock_go_to_origin_continue(State *s);
   static void HandleResponse(State *s);
   static void HandleUpdateCachedObject(State *s);
   static void HandleUpdateCachedObjectContinue(State *s);
@@ -1093,7 +1106,8 @@ class HttpTransact
   static void             handle_response_keep_alive_headers(State *s, HTTPVersion ver, HTTPHdr *heads);
   static int              get_max_age(HTTPHdr *response);
   static int              calculate_document_freshness_limit(State *s, HTTPHdr *response, time_t response_date, bool *heuristic);
-  static Freshness_t      what_is_document_freshness(State *s, HTTPHdr *client_request, HTTPHdr *cached_obj_response);
+  static Freshness_t      what_is_document_freshness(State *s, HTTPHdr *client_request, HTTPHdr *cached_obj_response,
+                                                     bool evaluate_actual_freshness = false);
   static Authentication_t AuthenticationNeeded(const OverridableHttpConfigParams *p, HTTPHdr *client_request,
                                                HTTPHdr *obj_response);
   static void             handle_parent_down(State *s);

src/proxy/http/HttpCacheSM.cc

@@ -43,6 +43,14 @@
 namespace
 {
 DbgCtl dbg_ctl_http_cache{"http_cache"};
+
+// Helper to check if cache_open_write_fail_action has READ_RETRY behavior
+inline bool
+is_read_retry_action(MgmtByte action)
+{
+  return action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY) ||
+         action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY_STALE_ON_REVALIDATE);
+}
 } // end anonymous namespace
 
 ////
@@ -215,12 +223,11 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
     break;
 
   case CACHE_EVENT_OPEN_WRITE_FAILED: {
-    if (master_sm->t_state.txn_conf->cache_open_write_fail_action ==
-        static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY)) {
+    if (is_read_retry_action(master_sm->t_state.txn_conf->cache_open_write_fail_action)) {
       // fall back to open_read_tries
-      // Note that when CacheOpenWriteFailAction_t::READ_RETRY is configured, max_cache_open_write_retries
+      // Note that when READ_RETRY actions are configured, max_cache_open_write_retries
       // is automatically ignored. Make sure to not disable max_cache_open_read_retries
-      // with CacheOpenWriteFailAction_t::READ_RETRY as this results in proxy'ing to origin
+      // with READ_RETRY actions as this results in proxy'ing to origin
       // without write retries in both a cache miss or a cache refresh scenario.
 
       if (write_retry_done()) {
@@ -264,8 +271,7 @@ HttpCacheSM::state_cache_open_write(int event, void *data)
       _read_retry_event = nullptr;
     }
 
-    if (master_sm->t_state.txn_conf->cache_open_write_fail_action ==
-        static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY)) {
+    if (is_read_retry_action(master_sm->t_state.txn_conf->cache_open_write_fail_action)) {
       Dbg(dbg_ctl_http_cache,
           "[%" PRId64 "] [state_cache_open_write] cache open write failure %d. "
           "falling back to read retry...",

src/proxy/http/HttpConfig.cc

@@ -1384,7 +1384,9 @@ HttpConfig::reconfigure()
   params->disallow_post_100_continue = INT_TO_BOOL(m_master.disallow_post_100_continue);
 
   params->oride.cache_open_write_fail_action = m_master.oride.cache_open_write_fail_action;
-  if (params->oride.cache_open_write_fail_action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY)) {
+  if (params->oride.cache_open_write_fail_action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY) ||
+      params->oride.cache_open_write_fail_action ==
+        static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY_STALE_ON_REVALIDATE)) {
     if (params->oride.max_cache_open_read_retries <= 0 || params->oride.max_cache_open_write_retries <= 0) {
       Warning("Invalid config, cache_open_write_fail_action (%d), max_cache_open_read_retries (%" PRIu64 "), "
               "max_cache_open_write_retries (%" PRIu64 ")",

src/proxy/http/HttpSM.cc

@@ -2537,9 +2537,11 @@ HttpSM::state_cache_open_write(int event, void *data)
   case CACHE_EVENT_OPEN_READ:
     if (!t_state.cache_info.object_read) {
       t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action;
-      // Note that CACHE_LOOKUP_COMPLETE may be invoked more than once
-      // if CacheOpenWriteFailAction_t::READ_RETRY is configured
-      ink_assert(t_state.cache_open_write_fail_action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY));
+      // READ_RETRY mode: write lock failed, no stale object available.
+      // CACHE_LOOKUP_COMPLETE will fire from HandleCacheOpenReadMiss with MISS result.
+      ink_assert(t_state.cache_open_write_fail_action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY) ||
+                 t_state.cache_open_write_fail_action ==
+                   static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY_STALE_ON_REVALIDATE));
       t_state.cache_lookup_result         = HttpTransact::CacheLookupResult_t::NONE;
       t_state.cache_info.write_lock_state = HttpTransact::CacheWriteLock_t::READ_RETRY;
       break;
@@ -2558,8 +2560,9 @@ HttpSM::state_cache_open_write(int event, void *data)
     t_state.source = HttpTransact::Source_t::CACHE;
     // clear up CacheLookupResult_t::MISS, let Freshness function decide
     // hit status
-    t_state.cache_lookup_result         = HttpTransact::CacheLookupResult_t::NONE;
-    t_state.cache_info.write_lock_state = HttpTransact::CacheWriteLock_t::READ_RETRY;
+    t_state.cache_open_write_fail_action = t_state.txn_conf->cache_open_write_fail_action;
+    t_state.cache_lookup_result          = HttpTransact::CacheLookupResult_t::NONE;
+    t_state.cache_info.write_lock_state  = HttpTransact::CacheWriteLock_t::READ_RETRY;
     break;
 
   case HTTP_TUNNEL_EVENT_DONE:
@@ -2663,7 +2666,21 @@ HttpSM::state_cache_open_read(int event, void *data)
 
     ink_assert(t_state.transact_return_point == nullptr);
     t_state.transact_return_point = HttpTransact::HandleCacheOpenRead;
-    setup_cache_lookup_complete_api();
+
+    // For READ_RETRY actions (5 and 6), skip the CACHE_LOOKUP_COMPLETE hook now.
+    // The hook will fire later with the final result: HIT if stale content is found
+    // during retry, or MISS if nothing is found (from HandleCacheOpenReadMiss).
+    // This ensures plugins see only the final cache lookup result, avoiding issues
+    // like stats double-counting and duplicate hook registrations.
+    if (t_state.txn_conf->cache_open_write_fail_action == static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY) ||
+        t_state.txn_conf->cache_open_write_fail_action ==
+          static_cast<MgmtByte>(CacheOpenWriteFailAction_t::READ_RETRY_STALE_ON_REVALIDATE)) {
+      SMDbg(dbg_ctl_http, "READ_RETRY configured, deferring CACHE_LOOKUP_COMPLETE hook");
+      t_state.cache_lookup_complete_deferred = true;
+      call_transact_and_set_next_state(nullptr);
+    } else {
+      setup_cache_lookup_complete_api();
+    }
     break;
 
   default: