Skip to content

make replicated subscriptions authoritative in subscription reconcile - #151

Open
fabracht wants to merge 2 commits into
mainfrom
fix-subscription-reconcile-141
Open

fabracht wants to merge 2 commits into
mainfrom
fix-subscription-reconcile-141

Conversation

@fabracht

@fabracht fabracht commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Closes #141. SubscriptionCache::reconcile treated the local topic index as the source of truth, even though that index is filled only by a one-hop broadcast. When a node got the replicated subscription record but missed the broadcast, reconcile deleted the record, export_partition passed the loss on, and the node kept failing to route to that subscriber.

  • reconcile now treats the replicated record as the source of truth, but only for clients whose session partition this node currently holds as primary or replica. It adds recorded subscriptions missing from the local TopicIndex/WildcardStore and never changes the record.
  • the holder restriction matters: nothing deletes a node's copy when it loses a partition (clear_partition has no production caller). Without the restriction, an ex-owner turned its stale copy into routing entries for clients that had already unsubscribed. This was the quorum review's main finding; it is reproduced live below.
  • exact response topics (resp/…) are skipped in reconcile and now also in startup recovery, since their subscribe and unsubscribe are never broadcast. The check is one shared is_response_topic function, replacing four copies.
  • removed WildcardPendingStore and its 1-minute timer. Only its own tests called add_pending, so the timer never did anything.
  • specs/ClusterSubReconcile.tla models holders P and R, a non-holder O, and a former holder S with a stale copy.
  • Accepted residual: the index is never pruned. A holder whose record lags a delivered unsubscribe broadcast can re-add an entry, which then stays until the client's subscriptions are cleared. The cost is a wasted forward, not a wrong delivery: the receiving node re-publishes into its own broker, which only delivers to local subscribers (handle_forwarded_publish_no_dedup). The holderghost cfg records this.
  • Not fixed here:

Test plan

  • cargo make clippy
  • cargo make test
  • Repro unit test replicated_subscription_survives_reconcile_when_broadcast_was_missed fails on main (left: []), passes here.
  • New unit tests:
    • reconcile_ignores_records_for_partitions_not_held
    • reconcile_skips_response_topics (now also checks the record keeps the entry)
    • recovery_rebuilds_index_without_response_topics (fails before the recovery change)
  • TLA, new mode:
    • loss and resurrection hold by construction, since the record never changes;
    • the checker establishes InvNoStaleCopyGhost and RoutingRepaired (exhaustive at MaxOps=2);
    • unrestricted violates InvNoStaleCopyGhost at depth 6;
    • holderghost violates InvNoHolderGhost (accepted, see above).
  • TLA, old mode and controls:
    • old mode: oldloss fails at depth 4, oldresurrect at depth 8;
    • norepair and nonholder fail their liveness properties, as expected.
  • 5-node E2E, repair after a missed broadcast, 6 runs per build:
    • Setup: replica Q is killed; the client subscribes on another node; Q restarts and gets the record by snapshot; primary P is killed.
    • Controls: the topic's own partition is held by neither P nor Q, and this is re-checked after promotion.
    • main: 0/6 delivered from Q, with one removed=1 reconcile on Q per run.
    • This branch: 6/6 delivered. In 5 of the 6 runs Q's index was already repaired by the takeover reconcile when Q rejoined, before P died.
  • 5-node E2E, ex-owner ghost entries:
    • Setup: 40–150 clients subscribe on 4 nodes; node 5 joins and the rebalance moves partitions; the clients disconnect; a node death triggers reconcile on an ex-owner E; then topics are published at E.
    • Valid runs: main 0/4 with ghost entries; this PR's first version 6/6; this branch 0/4.
    • Many iterations were discarded because the rebalance left no usable ex-owner/K pair.

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.

cluster: investigate whether SubscriptionCache::reconcile deletes correct replicated state

1 participant