perf(relay): cache relay-membership checks off the writer pool - #3844
Open
tlongwell-block wants to merge 5 commits into
Open
perf(relay): cache relay-membership checks off the writer pool#3844tlongwell-block wants to merge 5 commits into
tlongwell-block wants to merge 5 commits into
Conversation
check_relay_membership runs a writer-pool SELECT on every authenticated HTTP request and WS AUTH — after the replica read routing shipped, this is the largest remaining read volume on the bb-public writer. Add a 10s-TTL moka cache (mirroring the existing channel-membership cache) in front of is_relay_member for the two hot callsites, with cross-pod Redis invalidation via a new CacheInvalidation::RelayMembership variant. All four membership-mutation flows drop the affected key: - v2 invite claim (Joined outcome) - v1 invite claim (was_inserted) - admin add/remove (kind 9030/9031) - NIP-43 self-leave Negative results are cached too, and invalidated on join, so a fresh member is admitted immediately rather than after TTL expiry. The two invite-flow verification reads stay on the direct DB path (pre-mutation, staleness unacceptable). A missed cross-pod publish degrades to the 10s TTL, same contract as the existing caches. Verified: cargo test -p buzz-relay (803 passed) and -p buzz-pubsub (25 passed); clippy -D warnings clean. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
…cache * origin/main: feat(relay): raise hosted community limit to five (#3829) Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Contributor
|
nice perf win. can you include a before/after on writer-pool wait under a membership-heavy load, even if its just a local sample? |
Review findings from Dawn on #3844, all four addressed plus the two nice-to-haves: - transfer_ownership now invalidates the incoming owner's cache entry: its step-4 upsert can insert a brand-new member row, and a recently cached negative would deny the new owner for up to the 10s TTL. The drop rides the existing require_relay_membership host-lookup block — the only configuration in which the cache is consulted. - Wiring guard: owner_mints_and_new_pubkey_claims (the CI invite- security suite) now seeds a cached negative for the joiner before the claim and asserts the entry is gone after. Deleting the claim-path invalidation callsite turns the suite red (verified RED/GREEN by commenting the callsites out and restoring them). - Cache keys are raw 32-byte pubkeys, matching the sibling membership_cache, instead of 64-byte hex strings; the hex round-trip now happens once at the invalidation seam (validated-hex decode with a debug_assert backstop). - Dropped support_invalidation_closures() from the builder: nothing predicate-invalidates this cache, and the flag adds an is-invalidated check to every read on the hottest cache in the relay. The state.rs unit test now also exercises the hex-keyed invalidate_relay_membership seam every mutation flow goes through. Verified: cargo test -p buzz-relay --lib (803 passed), invite-security ignored suite vs local Postgres (12 passed), -p buzz-pubsub (25 passed); clippy --all-targets -D warnings and fmt clean. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
…cache * origin/main: feat(desktop): auto-enable huddle transcription for agents (#3180) feat(agent): optional reply guard reminds a silent turn to publish (#3763) feat(desktop): upgrade Pocket TTS model (#3266) feat(desktop): delete a message by clearing its edit to empty (#3813) Signed-off-by: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@buzz.block.builderlab.xyz> Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
Dawn's re-review notes on #3844, both trivial ones: - check_relay_membership computed hex::encode(pubkey_bytes) before the cache lookup, but the only remaining users are the two log lines in the NIP-OA branch — a 64-byte heap allocation per authenticated request on the cache-hit fast path this PR exists to make cheap. The binding now lives inside the auth-tag branch. - The struct-field doc (state.rs) and the pubsub variant doc (cache_invalidation.rs) still enumerated four mutation flows; both now list ownership transfer like the fn docstring already did. Verified: cargo test -p buzz-relay --lib (803/0), invite suite vs local Postgres (12/0), -p buzz-pubsub (25/0), clippy --all-targets -D warnings clean, fmt clean. Co-authored-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com> Signed-off-by: tlongwell-block <109685178+tlongwell-block@users.noreply.github.com>
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.
Why
check_relay_membershipruns a writer-poolSELECTon every authenticated HTTP request and WS AUTH (api/mod.rs), plus a second one for the NIP-OA owner path on agent traffic. After replica read routing shipped (#3268 / bb-public#325), this is the largest remaining read volume on the bb-public writer — the writer settled at ~35% CPU and this path is the top lever left (analysis in Buzz channelbuzz-read-only-replica-usage, thread c4be38ff; independently verified by Perci inRESEARCH/RELAY_MEMBERSHIP_CACHE_LOC_VERIFICATION_2026_07_31.md).What
relay_membership_cacheinAppState: moka, keyed(CommunityId, raw 32-byte pubkey) → bool, 10s TTL, 100k capacity — the same shape and key convention as the existing channel-membership cache.is_relay_member_cached()with hit/miss counters (buzz_relay_membership_cache_{hits,misses}_total); the two hot callsites incheck_relay_membershipnow use it. The NIP-OA owner lookup rides the same cache.CacheInvalidation::RelayMembershipvariant on the existing Redis fan-out, dropped at all five production membership-mutation flows:Joinedoutcome,api/invites.rs)was_inserted,api/invites.rs)handlers/relay_admin.rs)handlers/ingest.rs)api/operator.rs) — its upsert can insert a brand-new member row; without invalidation a recently cached negative would deny the incoming owner for up to the TTLdb.is_relay_membercallsites — coverage is complete. (An earlier revision of this body claimedinvites.rs:1414,1531"deliberately stay on the direct DB path"; those lines are inside#[cfg(test)] mod tests— test assertions, not a production guard. Caught in review by Dawn.)update_relay_member_role(kind 9032) cannot change the membership boolean, andbootstrap_ownerruns at provisioning/startup against a cold cache.Semantics / trade
Identical contract to the existing membership/visibility caches: a missed cross-pod publish degrades to a ≤10s TTL window, never a permanent leak. Revocation propagates via the same Redis channel the channel-membership cache already relies on.
buzz-adminCLI mutations bypass the relay and therefore ride the 10s TTL — same as they already do for the channel caches.Note (per review): this PR is not a replica read —
is_relay_membermisses still hit the writer pool, so replica lag is not part of this path. The staleness introduced is cache staleness, worst case 10s on a dropped Redis publish, sub-second on the normal invalidation path.Review fixes (Dawn, Buzz thread 350817e6)
transfer_ownershipnow invalidates the incoming owner's entry (the missing sixth callsite).owner_mints_and_new_pubkey_claims(CI invite-security suite) seeds a cached negative for the joiner pre-claim and asserts it is gone post-claim — verified RED with the invalidation callsites commented out, GREEN restored. Removing a claim-path callsite now fails CI instead of passing silently.membership_cache; hex round-trip happens once at the invalidation seam.support_invalidation_closures()— nothing predicate-invalidates this cache, and the flag adds an is-invalidated check to every read on the hottest cache in the relay.Post-approval polish (Dawn's re-review, non-blocking notes)
At
6c8990b5f(3-line commit on top of the approved6259b175a): moved thehex::encode(pubkey_bytes)binding incheck_relay_membershipinside the NIP-OA auth-tag branch — its only remaining users are two log lines there, so the cache-hit fast path no longer pays a 64-byte allocation per authenticated request; updated the two doc comments (state.rs struct field, pubsub variant) to list ownership transfer alongside the other mutation flows.Known follow-up (not in this PR): extend the wiring guard to a removal path (
relay_admin.rsadmin remove) — the theft-shaped direction. Dawn's mutation sweep table is in the review thread.Verification
At
6c8990b5f:cargo test -p buzz-relay --lib: 803 passed, 0 failed on my runs; noteapi::mesh_demo::...round_trips_echois a known flake (test(relay): mesh_demo echo test flaky under parallel suite — passes standalone, fails full-suite on clean main #2458) and failed on one reviewer run — unrelated to this diffcargo test -p buzz-pubsub: 25 passed (incl.RelayMembershipJSON roundtrip)relay_membership_cache_hit_and_scoped_invalidation: cache hit serves without DB (lazy pool would error), negative caching, community-scoped invalidation, cross-podapply_cache_invalidationpath, and the hex→raw-key invalidation seamcargo clippy -p buzz-relay -p buzz-pubsub --all-targets -- -D warningsclean;cargo fmtcleanExpected effect on bb-public: removes up to ~1 writer SELECT per authenticated request at ~100k req/min scale; measurable via the new cache counters plus writer CPU.
Requested by @tlongwell-block (Buzz thread c4be38ff, event e879ad20). Reviewed by Wren (approve, 9/9/9 under the clarified replica-freshness bar) and Dawn (findings addressed above).