Fix cache retry assert on ServerAddrSet#13396
Conversation
A TSHttpTxnServerAddrSet retry can re-enter the cache miss path after ATS already holds a cache write lock for the same request. The redirect-only prepared-write reuse logic then asserts because this is not actually a redirect. Preserve the existing cache write lock when the retry returns through HandleCacheOpenReadMiss. Backport of apache#12972 to 9.2.x. The accompanying autest is omitted because its plugin infrastructure (apache#12733) is not present on 9.2.x; enum and debug-macro names were adapted to the 9.2.x code. Fixes: apache#12971
bryancall
left a comment
There was a problem hiding this comment.
Approving. This is a faithful backport of #12972: the condition, the branch placement before the terminal else, and the comment all match master, with the enum and debug-macro names correctly adapted to the 9.2.x code (CACHE_WL_SUCCESS, CACHE_DO_WRITE, TxnDebug). Verified those symbols exist on 9.2.x and that the new branch only fires on a re-entry after the write lock was already acquired, so first-miss behavior is unchanged.
Omitting the autest is the right call here since the plugin infrastructure from #12733 is not on 9.2.x. One request for the record: could you confirm you reproduced the original assertion from #12971 on a 9.2.x build and that it no longer fires with this change? Since CI cannot exercise this path on the branch, a note that it was validated manually would be good to have.
A TSHttpTxnServerAddrSet retry can re-enter the cache miss path after ATS already holds a cache write lock for the same request. The redirect-only prepared-write reuse logic then asserts because this is not actually a redirect.
Preserve the existing cache write lock when the retry returns through HandleCacheOpenReadMiss.
Backport of #12972 to 9.2.x. The accompanying autest is omitted because its plugin infrastructure (#12733) is not present on 9.2.x; enum and debug-macro names were adapted to the 9.2.x code.
Fixes: #12971