Skip to content

agent: fast queue rotation not requiring the current server to be online - #1847

Open
epoberezkin wants to merge 16 commits into
masterfrom
ep/fast-rotation
Open

agent: fast queue rotation not requiring the current server to be online#1847
epoberezkin wants to merge 16 commits into
masterfrom
ep/fast-rotation

Conversation

@epoberezkin

Copy link
Copy Markdown
Member

No description provided.

@epoberezkin
epoberezkin requested a review from spaced4ndy as a code owner August 9, 2026 19:44
@epoberezkin epoberezkin changed the title agent: fast queue rotation does not requiring the current server to be online agent: fast queue rotation not requiring the current server to be online Aug 16, 2026
Base automatically changed from ep/drop-agent-versions to master August 17, 2026 11:51

@simplex-chat-agent simplex-chat-agent Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. securingSndQueue makes submitPendingMsg/resumeMsgDelivery no-ops while R' is New + replacing, so no worker delivers to R' until ICQSndSecure sends the confirmation synchronously and only then activates the queue. Restart-safe: ICQSndSecure is a persisted command and delivery resume is guarded by the same predicate.
  • Redundant delivery + dedup absorbs the races. copyPendingSndDeliveries runs 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.
  • QEND handling is idempotent. Delivered on both queues; removeQP/L.partition + withConnLockNotify mean the second copy is a no-op, so no duplicate SPCompleted and no double-remove. Self-deletion of the old queue's worker from smpDeliveryWorkers mirrors the existing AM_QTEST_ pattern.
  • Version gating is correct. Fast path only at agreed agent version ≥ 8 (rpcAddressSMPAgentVersion), QEND is v8-only, and canAbortRcvSwitch now refuses to abort a committed RSSendingQADD at v8+. Both canAbortRcvSwitch call 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 (ack returning Nothing on a missing queue) and the bounded-retry ICDeleteRcvQueue for 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).

Comment thread src/Simplex/Messaging/Agent.hs Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants