cmcfarlen commented on PR #13821: URL: https://github.com/apache/trafficserver/pull/13821#issuecomment-6324160528
@bryancall thanks for the thorough review. The blocking items are addressed: - **Lock miss in `mainEvent`:** 88980c33c5 re-arms before the `EVENT_CONT` return. - **Enabled connection with no deadline:** adcce4a24f adds `NetEvent::arm_default_inactivity_timeout()`, which applies the old sweep's rule (enabled, open, no inactivity deadline: arm the default). It is called from `set_enabled`, `set_inactivity_timeout(0)`, `rebase_default_inactivity_deadline`, the end of `mainEvent` once the handler has kept the connection, and the cop. `netActivity` now re-arms on any 0 to armed transition, explicit or default. The "safety net" wording in `net-timeouts.en.rst` is gone. - **Backward clock step:** 0cc3bf3a73 re-buckets the wheel when `now` is behind the cursor. A note from reading the event system: the cop runs via `schedule_every` on the same realtime clock, so after a backward step it doesn't run until the clock catches up, and that was true of the old sweep too. Details are in the thread on `TimerWheel.h`. I'd propose moving the event system and net deadlines to a monotonic clock as a follow-up. - **Tests:** 731fbf5dee drives a real `UnixNetVConnection` through the cop for the lock-miss retry, the `set_inactivity_timeout(0)` fallback, a connection kept after a delivered timeout, and `netActivity`'s re-arm. All four fail without the fixes. One behavior to call out: as with the old cop, the default replaces a spent explicit timeout. A handler that keeps a connection after an explicit timeout now gets the default immediately, rather than up to a second later. The non-blocking notes (keep-alive eviction for a foreign-closed connection, the `closed` store ordering, the unlocked deadline reads) are still open. I'll take them next. -- 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]
