Summary
shard_connection::peer_client_has_any_setup_in_progress() (shard_connection.cpp) gates on is_ready_for_reads(), not is_conn_setup_done() -- a peer stuck at setup_sent on AUTH/HELLO/CLIENT NO-TOUCH is not counted as "mid-setup" by this predicate, matching is_ready_for_reads()'s own scope.
Why this needs investigation, not just a decision
During PR #526 review, an earlier version of this PR widened the gate to is_conn_setup_done() (counting AUTH/HELLO/NO-TOUCH-pending peers as mid-setup too). That was reverted because: a peer that exhausts --max-reconnect-attempts on one of those setup commands would then be counted as permanently "in progress" under the wider gate, rather than bounded by connection_stage_should_abort() -- and whether that window is actually bounded in every case (across the reconnect / --read-preference interaction) was never confirmed. The revert was the safe choice given that uncertainty, not a confirmed-correct design.
What's needed
Someone needs to actually trace connection_stage_should_abort() together with the reconnect and read-preference routing paths to determine:
- Is the current (narrower) gate's behavior definitely livelock-free for every
--read-preference != primary configuration?
- Would the wider gate (
is_conn_setup_done()) actually introduce a livelock, or was that risk overstated?
Whichever way this resolves, the code comment at peer_client_has_any_setup_in_progress() should stop saying "deliberately" (implying settled design) and instead point at this issue while the question is open.
Discovered while working on
PR #526 (--client-no-touch) review, round 22.
Summary
shard_connection::peer_client_has_any_setup_in_progress()(shard_connection.cpp) gates onis_ready_for_reads(), notis_conn_setup_done()-- a peer stuck atsetup_senton AUTH/HELLO/CLIENT NO-TOUCH is not counted as "mid-setup" by this predicate, matchingis_ready_for_reads()'s own scope.Why this needs investigation, not just a decision
During PR #526 review, an earlier version of this PR widened the gate to
is_conn_setup_done()(counting AUTH/HELLO/NO-TOUCH-pending peers as mid-setup too). That was reverted because: a peer that exhausts--max-reconnect-attemptson one of those setup commands would then be counted as permanently "in progress" under the wider gate, rather than bounded byconnection_stage_should_abort()-- and whether that window is actually bounded in every case (across the reconnect /--read-preferenceinteraction) was never confirmed. The revert was the safe choice given that uncertainty, not a confirmed-correct design.What's needed
Someone needs to actually trace
connection_stage_should_abort()together with the reconnect and read-preference routing paths to determine:--read-preference != primaryconfiguration?is_conn_setup_done()) actually introduce a livelock, or was that risk overstated?Whichever way this resolves, the code comment at
peer_client_has_any_setup_in_progress()should stop saying "deliberately" (implying settled design) and instead point at this issue while the question is open.Discovered while working on
PR #526 (
--client-no-touch) review, round 22.