fix(relay): multi-pod subscription coherence (one access-gated fan-out path + cross-pod cache invalidation + REQ/COUNT DB guard)#1261
Merged
Conversation
…d path Establish the invariant that no relay-local EVENT delivery can bypass the membership/access gate: a registered subscription is never sufficient for delivery — delivery always revalidates access on the sending pod. Introduce fan_out_event_to_local_subscribers(state, stored), which composes fan_out() -> filter_fanout_by_access() -> send EVENT frames, and route every live local fan-out callsite through it: - ephemeral channel events (handle_ephemeral, was an ungated bypass) - ephemeral channel-less / global events - agent-observer frames (kind 24200) - audio lifecycle events (was an ungated bypass) - membership-notification, ref-state (30618), and NIP-43 (8000) side effects - mesh call-me-now (24622) - git push ref-state (30618) in the HTTP transport The two previously-gated paths keep their inline filter call rather than the helper: dispatch_persistent_event layers a per-recipient DM-visibility-owner gate on top of the shared filter, and fan_out_pubsub_event additionally skips local echoes. Both are equivalent to the helper plus their own extra step. The ephemeral and audio paths were genuine pre-existing access-gate holes even single-pod: a subscription surviving an open->private flip or a membership removal could receive private-channel events. The global callsites are no-ops through the gate today (filter_fanout_by_access only applies the author-only- kind gate when channel_id is None, and none of these are author-only kinds), routed through it so the single send path stays the universal enforcement point and future channel-scoped paths cannot bypass it by accident. filter_fanout_by_access and the new helper take &AppState rather than &Arc<AppState> so the audio handler (which holds &AppState) can call the helper; existing &Arc<AppState> callers deref-coerce with no other change. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
Each pod keeps in-memory (moka) membership / accessible-channels / visibility caches dropped only on the pod that processed a write; other pods relied on the 10s TTL to expire stale entries. After scaling 1->2 pods this surfaced as agents intermittently not subscribed to rooms they were just added to (stale is_member / accessible-channels on the non-writer pod). Carry the same key drops to every pod immediately over a dedicated 'buzz:cache-invalidate' Redis pub/sub topic: - buzz-pubsub: CacheInvalidation enum (one variant per invalidate_* op), publish_cache_invalidation, subscribe_cache_invalidations, and a reconnecting subscriber loop mirroring run_subscriber. - state.rs: each public invalidate_* now does the local moka drop AND fire-and-forget spawns the matching publish, so all ~13 call sites stay untouched. The cross-pod consumer calls private *_local drop variants via apply_cache_invalidation, so a received drop is never re-published (no fan-out loop). - main.rs: spawn the subscriber and a consumer loop mirroring the multi-node event fan-out consumer. The message is a pure cache-key drop, never an 'evict these subscriptions' payload: the per-event access gate from commit #1 is the universal delivery-enforcement point, so dropping the stale key is sufficient (next read re-fetches authoritative state from the DB). A missed publish degrades to the <=10s TTL wait, backstopped by the REQ denial-path DB confirmation in commit #3 -- never a leak. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
3011abd to
d3be43d
Compare
…n REQ/COUNT A REQ or COUNT targeting a specific channel gates on `accessible_channels`, a per-request Vec built once from the 10s membership cache. On a multi-pod relay this Vec can be stale on a non-writer pod: a member just added on the pod that processed the write sees a cache-negative until the TTL expires or the cross-pod invalidation (commit #2) lands. That manifested as the create-channel readback coming back empty and agents not subscribed to rooms they were just added to. On a cache-negative, confirm membership against the DB uncached. On a verified positive, repair the request-local Vec by pushing `ch_id` once, via the pure helper `resolve_request_local_access`. The same Vec gates subscription registration, historical delivery, search scope, and COUNT — repairing it once makes all of them see the confirmed membership, not just the denial branch. A stale negative can no longer stay sticky for the rest of the request. The repair runs UP FRONT in req.rs, right after the subscription channel_id is extracted and before the NIP-50 search early-return — not in a late denial branch. A search scoped to `#h=<just-added>` would otherwise be scoped against the stale vector and false-miss; running the repair first means `handle_search_req` sees the repaired vector too. The helper takes a `token_allows` upper bound so a DB-positive can never push a channel back in past a narrower scoped token: a token scoped to channel A must not reach channel B merely because the user is a DB member of B. Both call sites compute it from the token's `channel_ids`. - req.rs: `resolve_request_local_access(&mut Vec, ch_id, token_allows, Option<bool>)` encodes the truth table (token-denies → denied no DB; cache-hit → allowed no DB; miss+DB-true → allowed & pushed; miss+DB-false → denied & unchanged) with unit tests for all four. The handler does the async `db.is_member` lookup only on a token-allowed miss. - count.rs: mirrors the same flow through the shared helper, gated by the same token bound, and applies the scoped-token `retain` that REQ does (after `get_accessible_channel_ids_cached`) so a scoped token cannot COUNT out-of-scope channels via the no-channel-filter SQL pushdown either. DB truth (`is_member` after `add_member`) is covered by the existing #[sqlx::test] membership tests in buzz-db/channel.rs. End-to-end handler coverage is noted in the PR: this crate has no AppState+DB test harness and standing one up was out of scope; the request-local repair invariant is proven by the pure-helper tests instead. Co-authored-by: Tyler Longwell <tlongwell@block.xyz> Signed-off-by: Tyler Longwell <tlongwell@block.xyz>
d3be43d to
2241648
Compare
Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co> Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta <d8473ee32b973aa31a21a65adddcc4b69cc2a8a4dee8121ecd51926e0cddbc02@sprout-oss.stage.blox.sqprod.co>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relay multi-pod subscription coherence
After scaling the relay 1→2 pods, agents intermittently weren't subscribed to rooms they'd just been added to (stopping/respawning fixed it). Root cause: each pod keeps in-memory (moka) membership / accessible-channels / visibility caches with only a 10s TTL and no cross-pod invalidation. A membership change applied on the writer pod left the other pod serving stale
is_member/accessible-channelsfor up to the TTL — denying valid subscriptions and returning empty create-channel readbacks.Design doc:
RESEARCH/RELAY_MULTIPOD_SUBSCRIPTION_GAP.md. One PR, three commits, in order.Commit 1 — invariant: one access-gated EVENT fan-out path (
53ee54e)Establishes the load-bearing invariant for the whole fix: no relay-local channel-scoped EVENT delivery can bypass
filter_fanout_by_access. A single helper (fan_out_event_to_local_subscribers) does fan_out → filter_fanout_by_access → serialize → send loop → drop-count. 8 raw callsites routed through it (net −61 lines: consolidated duplicate loops).Audit of every production
fan_out → send_to(EVENT)callsite (base8568159):req.rshistorical delivery is REQ-response, not live fan-out — already gated byaccessible_channels; handled by commit #3, not here. Safety:AUTHOR_ONLY_KINDS = [30300]only; none of the global callsite kinds are 30300, so routing them through is a true no-op today (verifiedbuzz-core/src/kind.rs).Commit 2 — coherence: cross-pod cache-key invalidation over Redis pub/sub (
eb2b1ed)A dedicated
buzz:cache-invalidateRedis topic carries each cache-key drop to every pod immediately. The message is a pure cache-key drop, never an "evict these subscriptions" payload — the commit-1 access gate is the universal delivery-enforcement point, so dropping the stale key is sufficient (next read re-fetches authoritative DB state).buzz-pubsub:CacheInvalidationenum (one variant perinvalidate_*op),publish_cache_invalidation,subscribe_cache_invalidations, reconnecting subscriber loop mirroringrun_subscriber.state.rs: each publicinvalidate_*does the local moka drop and fire-and-forget spawns the matching publish — all ~13 call sites untouched. The cross-pod consumer calls private*_localdrop variants viaapply_cache_invalidation, so a received drop is never re-published (no fan-out loop).main.rs: spawn the subscriber + a consumer loop mirroring the multi-node event fan-out consumer.A missed publish degrades to the ≤10s TTL wait, backstopped by commit #3 — never a leak.
Commit 3 — guard: denial-path DB confirmation in REQ/COUNT (
3011abd)The backstop for the brief window before a TTL expires or an invalidation lands.
accessible_channelsis a per-requestVecbuilt once from the cache and reused for subscription registration, historical delivery, search scope, and COUNT. On a cache-negative for the targeted channel, confirm membership against the DB uncached; on a verified positive, pushch_idinto the Vec via the pure helperresolve_request_local_access. The confirmation is request-local-authoritative: one repair, and registration + historical + COUNT all see it — a stale negative can't stay sticky for the rest of the request.Truth table (unit-tested, all three cases):
ch_id→ allowed, no DB, no repairch_idpushed (repair)Mirrored in
count.rsthrough the same shared helper.Verification
cargo build --workspaceclean;cargo clippy -p buzz-pubsub -p buzz-relayclean.cargo test -p buzz-relay -- --test-threads=1→ 367 passed (364 baseline + 3 new helper tests).cargo test -p buzz-pubsub -- --ignoredagainst live Redis → 6 passed, includingtest_cache_invalidation_roundtrip(publish on one manager, receive the exactCacheInvalidationon another's subscriber — actual cross-pod propagation).is_memberafteradd_member) is covered by existing#[sqlx::test]membership tests inbuzz-db/channel.rs.Notes / out of scope
AppState+DB test harness (all relay tests are in-memory;AppState::newneeds Db+Redis+audit+pubsub+auth+search+workflow+keypair+S3 GitStore and spawns workers). Standing one up was disproportionate to a ~30-line diff and a fragile harness is its own risk. The request-local-repair invariant is proven by the pure-helper tests; DB truth by the existing buzz-db tests. (Reviewed with Perci.)fanout_accesstests can fail intermittently under a parallel run becauseconfig.rstestsset_varbogusBUZZ_BIND_ADDR/BUZZ_GIT_REPO_PATHthat a concurrentConfig::from_env()reads. Full suite passes single-threaded (367/367) and the scoped fanout tests pass parallel. Flagging, not fixing, in this PR.