Copilot commented on code in PR #13325: URL: https://github.com/apache/trafficserver/pull/13325#discussion_r3470728104
########## src/proxy/http/HttpSM.cc: ########## @@ -2665,6 +2665,11 @@ HttpSM::state_cache_open_read(int event, void *data) ink_assert(server_entry == nullptr); ink_assert(t_state.cache_info.object_read == nullptr); + // Record the cache open-read completion before state handling can advance + // into cache serving and stamp later transaction milestones. + ATS_PROBE1(milestone_cache_open_read_end, sm_id); + milestones[TS_MILESTONE_CACHE_OPEN_READ_END] = ink_get_hrtime(); + Review Comment: Stamping TS_MILESTONE_CACHE_OPEN_READ_END unconditionally at function entry changes behavior for the compatibility-key retry path (CACHE_EVENT_OPEN_READ_FAILED + cache_try_compat_key_read). In that path the function calls do_cache_lookup_and_read(), which re-stamps TS_MILESTONE_CACHE_OPEN_READ_BEGIN; with this change, BEGIN can become later than END (and if the txn aborts before the retry completes, the final milestone set can remain inconsistent). Consider stamping CACHE_OPEN_READ_END inside each switch case, *after* the retry decision in the FAILED case, but still before any call_transact_and_set_next_state()/API callout that can advance and stamp later milestones. -- 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]
