bneradt commented on code in PR #12845:
URL: https://github.com/apache/trafficserver/pull/12845#discussion_r2755452103


##########
src/proxy/http/HttpTransact.cc:
##########
@@ -3342,6 +3347,15 @@ 
HttpTransact::set_cache_prepare_write_action_for_new_request(State *s)
     // don't have a state for that.
     ink_release_assert(s->redirect_info.redirect_in_process);
     s->cache_info.action = CacheAction_t::WRITE;
+  } else if (s->cache_info.write_lock_state == CacheWriteLock_t::READ_RETRY &&
+             (!s->redirect_info.redirect_in_process || 
s->txn_conf->redirect_use_orig_cache_key)) {
+    // Defensive: Should not reach here if HandleCacheOpenReadMiss check is 
working.
+    // If we somehow get here in READ_RETRY state, bypass cache unless we're 
in a redirect
+    // that uses a different cache key (redirect_use_orig_cache_key == 0).
+    // When redirect_use_orig_cache_key is enabled, the redirect uses the same 
cache key
+    // as the original request, so we'd hit the same write lock contention.
+    Warning("set_cache_prepare_write_action_for_new_request called with 
READ_RETRY state");

Review Comment:
   Let's make this an Error instead of Warning.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to