agent: fast queue rotation not requiring the current server to be online - #1847
agent: fast queue rotation not requiring the current server to be online#1847epoberezkin wants to merge 16 commits into
Conversation
92f05e6 to
5355221
Compare
2003790 to
80b32ae
Compare
There was a problem hiding this comment.
Adds v8 "fast" queue rotation: the peer secures the new receive queue R' over R' itself (an empty confirmation carrying a fresh queue-box DH) instead of over the current queue, so a rotation completes even when the current server is down. A new QEND agent message removes the replaced queue on both sides, and pending deliveries are copied to R' so the backlog survives the move.
I traced the whole flow — QADD (qAddMsg) → copyPendingSndDeliveries + ICQSndSecure → confirmation on R' + QEND to both queues → recipient qEndMsg / sender AM_QEND_ — plus the protocol/store additions and the test changes. The design is careful and the invariants hold:
- Confirmation-first ordering is structural, not timed.
securingSndQueuemakessubmitPendingMsg/resumeMsgDeliveryno-ops while R' isNew+ replacing, so no worker delivers to R' untilICQSndSecuresends the confirmation synchronously and only then activates the queue. Restart-safe:ICQSndSecureis a persisted command and delivery resume is guarded by the same predicate. - Redundant delivery + dedup absorbs the races.
copyPendingSndDeliveriesruns under the conn lock while the old worker deletes rows without it, but a doubly-copied or missed row is either deduplicated or already delivered on the other queue — no loss either way. QENDhandling is idempotent. Delivered on both queues;removeQP/L.partition+withConnLockNotifymean the second copy is a no-op, so no duplicateSPCompletedand no double-remove. Self-deletion of the old queue's worker fromsmpDeliveryWorkersmirrors the existingAM_QTEST_pattern.- Version gating is correct. Fast path only at agreed agent version ≥ 8 (
rpcAddressSMPAgentVersion),QENDis v8-only, andcanAbortRcvSwitchnow refuses to abort a committedRSSendingQADDat v8+. BothcanAbortRcvSwitchcall sites were updated. The recipient reacts to whichever message arrives, and repeated confirmations fall into the existing(Just e2eDh, Just _)idempotent branch. - Tolerant ack after queue removal (
ackreturningNothingon a missing queue) and the bounded-retryICDeleteRcvQueuefor a dead old server are both sound.
Tests: existing switch tests are correctly pinned to v7 (agentCfgV7) to keep exercising the QKEY/QUSE slow path and abort semantics, and new tests cover fast rotation, a dead old server with an in-flight message, and notification re-subscription on both paths. Good coverage.
One minor, non-blocking note (no fix needed): during securing, deliveries copied to R' and new deliveries scheduled to R' skip the submitPendingMsg increment of msgDeliveryOp.opsInProgress, but the worker started by ICQSndSecure decrements once per drained message. This over-decrements (clamped at 0 by endOperation), so the counter can reach 0 while R' still has queued deliveries. Impact is limited to graceful-suspend accounting; deliveries are persisted and redelivered on resume, so no message is lost. Worth a comment if you want to keep the accounting exact.
One small suggestion inline: reuse securingSndQueue inside sndDeliverTo instead of re-spelling status == New && isJust (dbReplaceQId sq).
No description provided.