Offline-first: retry schedule cannot survive a restart, and there is no idempotency key — duplicates plus a reconnect herd

Источник: dev/fera-research/docs/oc5/offline-retry-schedule-unpersistable-no-idempotency.md (read-only; сгенерировано scripts/sync-docs.mjs)

Offline-first: retry schedule cannot survive a restart, and there is no idempotency key — duplicates plus a reconnect herd

• kind: problem
• area: offline-first / storage / actor-model
• severity: high
• evidence:
  • ~/dev/fera/fera/core/crates/domain/src/outbox.rs:9 — use std::time::{Duration, Instant};
  • ~/dev/fera/fera/core/crates/domain/src/outbox.rs:22-23 — pub not_before: Option<Instant>, with doc /// Earliest next try; None = immediately eligible.
  • ~/dev/fera/fera/core/crates/domain/src/outbox.rs:99-110 — rows_to_entries hardcodes not_before: None
  • ~/dev/fera/fera/core/crates/domain/src/outbox.rs:52-58 — eligible_at filters on e.not_before.is_none_or(|nb| nb <= now)
  • no serde / Serialize / Deserialize anywhere in outbox.rs or domain/Cargo.toml
  • ~/dev/fera/fera/core/crates/domain/src/reconnect.rs:20-30 — Backoff has no max_attempts field
  • ~/dev/fera/fera/core/crates/domain/src/reconnect.rs:60-68 — on_failure has no terminal case; cap.mul_f64(1.0 - jitter.clamp(0.0, 1.0))
  • ~/dev/fera/fera/core/crates/domain/src/reconnect.rs:111 — test asserts d > Duration::ZERO || j >= 1.0, i.e. jitter == 1.0 yields zero delay
  • ~/dev/fera/fera/core/crates/protocol/src/state.rs:75-79 — send_message returns Result<MessageId, ProtocolError>; no idempotency token
  • ~/dev/fera/fera/core/crates/domain/src/chat.rs:237-242 — SendFailed keeps the row forever with status = Failed { attempts }
  • ~/dev/fera/fera/core/crates/storage/src/memory.rs:113-115 — get_outbox returns Ok(self.outbox.clone())
  • ~/dev/fera/fera/core/crates/storage/src/memory.rs:123 — confirm_sent’s retain is the only removal path
  • ~/dev/fera/fera/tools/bench-scene/src/main.rs:6 — frozen scene includes “50 pre-filled outbox entries”

Substrate

1. The retry schedule is structurally unpersistable, not merely unimplemented

RetryEntry is documented as the persisted shape — outbox.rs:14-15: “persistence itself is the storage layer’s outbox”. It carries:

pub not_before: Option<Instant>,   // outbox.rs:23

std::time::Instant is a process-local monotonic clock reading. It cannot be serialized, cannot be persisted, and cannot be compared across processes or machines. There is no serde derive on the type and no serde dependency in domain/Cargo.toml — not an oversight in the implementation, an impossibility in the type.

The code already routes around it. rows_to_entries (outbox.rs:99-110) reconstructs retry state from storage rows and hardcodes:

not_before: None,      // outbox.rs:107

So attempts is recovered (from OutboxStatus::Failed { attempts }) and the backoff deadline is not. The consequence is precise and quantified:

• Before a restart, a failed entry is backed off 1s → 2s → 4s → 8s (capped 5 min; outbox.rs:40-47).
• After a restart, every restored entry has not_before: None, and eligible_at treats None as “immediately eligible” (outbox.rs:55).

A client that crashes, is force-quit, or simply restarts therefore loses its entire backoff ladder and fires the whole outbox at once. With the frozen benchmark scene’s 50 pre-filled outbox entries, that is 50 simultaneous sends in the same tick, against a server the client had just decided was unreachable. Crash-looping clients turn this into a sustained reconnect herd, not a one-off.

The fix is a type change: not_before must become a wall-clock deadline (SystemTime/epoch millis, clamped against skew) or an absolute offset persisted in the same row as attempts. Keeping the schedule in the same persistence unit as the counter is the only way it can survive.

2. No idempotency key: a lost ACK becomes a duplicate message

eligible_at re-sends entries it believes unconfirmed. The send path (chat.rs:172-186) carries no client-generated correlation token — not in the RetryEntry (local_id, attempts, not_before only), not in Transport::send_message (state.rs:75-79), not in ChatCommand::SendConfirmed/SendFailed.

So the ACK-loss window is unclosable from the client side: the server accepts the send, the ACK is lost in transit, the entry is still Pending, the ladder retries, and the server has no way to recognise the second copy as the same message — because nothing in the request identifies it. The user sees their message twice.

This is the offline-first guarantee failing at its core promise, and it compounds finding #1 (local-id-reuse-ack-misdelivery.md): there the ACK lands on the wrong message, here the message arrives twice.

3. Parked entries are never evicted — unbounded plaintext growth

on_send_failed returns None once attempts >= max_attempts (outbox.rs:62-66), commented “parked”. Nothing acts on that. The actor keeps the row with status = Failed { attempts } (chat.rs:237-242), and the only removal path in storage is confirm_sent’s retain (memory.rs:123). There is no TTL, no eviction, no cap.

Consequences:

• get_outbox clones the entire outbox on every read (memory.rs:113-115) — cost grows without bound.
• Permanently-failed entries hold plaintext message bodies forever (see storage-seam-frozen-wrong-shape.md — the outbox is not classified in ADR-031).
• A user who was offline for a month returns to an outbox of arbitrary size, all of which the new type-level fix will retry in one tick.

4. Reconnect has no terminal state, and full jitter can produce a zero delay

Backoff (reconnect.rs:20-30) has base, max, factor, attempts — and no max_attempts, unlike OutboxRetry which has one (outbox.rs:29-31). on_failure has no branch that stops. Combined with finding #1, a client that restarts in a loop reconnects immediately after every restart, forever.

Separately, on_failure returns cap.mul_f64(1.0 - jitter.clamp(0.0, 1.0)). At jitter == 1.0 this is Duration::ZERO — an immediate hot retry. The caller supplies jitter (there is no RNG in the crate), and the test explicitly blesses the zero-delay case (reconnect.rs:111: assert!(d > Duration::ZERO || j >= 1.0)). jitter.clamp(0.0, 1.0) is inclusive of 1.0, so a caller that draws from a [0,1]-inclusive range — or any RNG whose clamp saturates — gets a busy-loop. The clamp should be exclusive (1.0 - jitter.clamp(0.0, 0.999)) or a floor applied.

Also worth noting: ProtocolError::SessionInvalid (protocol/src/error.rs:19) has no handler anywhere in non-test code — an invalid session produces no defined transition.

Options (tradeoffs)

1. Change not_before to a wall-clock deadline. Option<SystemTime> or epoch millis, persisted next to attempts. Fixes #1 at the type level. Cost: needs a skew policy (accept late/early deadlines gracefully) and a decision on monotonic-vs-wall-clock. SystemTime is the only choice that survives restart; keep Instant for in-process scheduling only.
2. Add a client idempotency key. A per-(local_id, attempt) UUID (or just (local_id, attempt)) sent with every send, deduplicated server-side. Fixes #2. Cost: server-side storage for dedup with a retention window, and a wire-format change in the MTProto layer — which means it must be raised with the protocol work early, since retrofitting a wire field is far costlier than adding it now.
3. Persist the whole RetryEntry as an outbox row rather than reconstructing from Message.status. attempts + next_attempt_at become first-class columns. Fixes #1 and makes #3 tractable. Cost: the storage trait’s outbox type changes shape.
4. Cap/evict parked entries (TTL, or a max-count with user-visible “N failed messages” affordance). Fixes #3. Cheap.
5. Give Backoff a max_attempts, or a gave_up() returning Option<Duration> like on_send_failed does. Fixes #4.

Proposed solution

Options 1 + 3 + 4 now; raise option 2 as a protocol-blocking item.

• Make the outbox row carry attempts: u32 and next_attempt_at: Option<SystemTime> as first-class persisted fields, and type RetryEntry with SystemTime so the unpersistable case is unrepresentable. Keep Instant strictly for in-process timing.
• Define the skew policy explicitly: a deadline in the past ⇒ retry now (correct after sleep/resume); a deadline absurdly far in the future (clock moved back) ⇒ clamp to now.
• Raise the idempotency key with whoever owns the protocol crate now. This is the one item on the list that gets materially more expensive with time, because it is a wire change. (local_id, attempt) is sufficient and needs no extra round trip.
• Add eviction for parked entries: a TTL plus a hard cap, with the count surfaced to the user rather than dropped silently.
• Add max_attempts to Backoff (or a gave_up() that mirrors on_send_failed’s Option<()> shape), and make the jitter clamp exclusive of 1.0 so Duration::ZERO is unreachable.
• Regression tests worth pinning: (a) restore-from-storage preserves the backoff deadline; (b) an entry is not eligible at now + delay - 1ms and is at now + delay; (c) N restored entries do not all become eligible in the same tick — the herd test.

This also gives ChatActor::next_local_id the persistence it needs for the id-reuse fix in local-id-reuse-ack-misdelivery.md: both require the outbox row to carry machine-readable scheduling state rather than being reconstructed from Message.status.