Repository navigation
perf(relay): load only mention candidates for workflow messages - #9340
Draft
ThePumpingLemma wants to merge 1 commit into
Draft
ThePumpingLemma wants to merge 1 commit into
ThePumpingLemma wants to merge 1 commit into
Conversation
The workflow SendMessage sink read every channel member and then every member's user row to resolve @name mentions: O(members) rows for every workflow message, in two writer-pool reads. It now reads, in one query, only the members whose display name could match at an '@' anchor, and skips the read when the rendered text has no anchor (an '@' at the start or after whitespace or '('). That subsumes the '@' guard that was dropped from #9178. How the filter is built (mention_candidate_query): - For each left-boundary '@' in the rendered and authored texts, take the tail after it, folded per char with char::to_lowercase exactly as resolve_mention_pubkeys folds text, cut to 255 chars (users.display_name is VARCHAR(255)). Distinct tails are capped at 32; past that the sink reads every named member as before. - An ASCII name folds to its ASCII lowercase, so it is a candidate only if that is a prefix of a tail. Postgres applies it with translate() over A-Z and starts_with under COLLATE "C". - Any other name is a candidate only if its first char lowercases to something starting with a tail's first char. The relay sends every such char (a table built once from all of Unicode), and Postgres compares the name's first code point exactly. Case folding: Postgres lower() depends on the database locale and can disagree with Rust's char::to_lowercase (U+0130 becomes two code points in Rust, ß/ẞ, final sigma, Turkish rules), so the query never calls it. Both rules compare code points, so the set is a guaranteed superset, and the exact match stays in resolve_mention_pubkeys, which is unchanged. Why the result is the same as with the full roster: - A name that matches at no anchor neither binds nor consumes a span in resolve_mention_pubkeys, so leaving it out changes nothing. - Members with the same folded name match at the same anchors, so all of them are loaded and the ambiguity rule still sees them; longer names that win greedily also prefix the tail, so they are loaded too. - Authored tails are included because authority is resolved against the authored text with the same members. buzz-db gains get_named_members_for_event_write (channel_members joined with users, event-write pool operation) and MentionNameFilter. No schema change. get_users_bulk_for_event_write had no other caller and is removed. Tests: - candidate_filter_resolves_like_the_full_roster: a table of names with spaces, punctuation, U+0130, ß/ẞ, emoji, Greek sigma, the Kelvin sign, same-name members, prefix names and noise, and 30 text pairs. Tags from the filtered roster equal tags from the full roster, and the filter admits fewer than half the members. - anchor_rule_matches_the_resolver: for every BMP char before '@', an anchor is found exactly when the resolver can match. - mention_candidates_skip_the_database_without_an_anchor (unreachable database) and text_without_at_never_resolves_a_mention. - Postgres: named_members_filter_returns_only_admitted_names (buzz-db, 1,000-member channel) and mention_candidates_read_only_names_a_mention_can_match (relay, 537 members: "ping @robby" reads 2 rows, and every table case resolves like the full roster). With the filter disabled the relay test fails on the row count. Signed-off-by: David Grochowski <dgrochowski@squareup.com>
This branch has not been deployed
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.
Summary
Problem
To resolve
@Namementions, the workflowSendMessagesink read every active channel member, then every member'susersrow. That is O(members) rows for every workflow message, in two writer-pool reads, even though a message usually names a few people or nobody.Change
workflow_sink.rs:load_mention_candidatesreads only the members whose display name could match at a mention anchor. An anchor is an@at the start of the text or after whitespace or(, the same left-boundary ruleresolve_mention_pubkeysuses. Rendered text with no anchor issues no query. This subsumes the@guard that was dropped from perf(relay): skip fan-out access reads when no local subscriber matches #9178.mention_candidate_querycollects the tail after each anchor in the rendered and authored texts. Each tail is folded per char withchar::to_lowercase, exactly as the resolver folds text, and cut to 255 chars becauseusers.display_nameisVARCHAR(255). A tail that prefixes another tail is dropped, so repeated mentions of one name cost one tail. Above 32 distinct tails, the sink reads every named member as before.igivesi,I,İ;kgivesk,K, the Kelvin sign).resolve_mention_pubkeysis unchanged and still does the exact match.buzz-db:get_named_members_for_event_write(channel members joined with users, event-write pool operation) andMentionNameFilter. No schema change; no trigger, function, procedure, rule or foreign key is added.get_users_bulk_for_event_writehad no other caller, so it is removed.Case folding. Postgres
lower()depends on the database locale and can disagree with Rust. For example, Rust lowercasesİ(U+0130) to two code points, andß/ẞ, finalΣand Turkish locale rules differ too. So the query never callslower(). ASCII names are folded withtranslateoverA–Zand compared withstarts_withunderCOLLATE "C". Non-ASCII names are compared by their first code point only. Both rules are necessary conditions for the resolver to match, so the result is a guaranteed superset.Why the tags are the same as with the full roster. A name that matches at no anchor neither binds nor consumes a span, so leaving it out changes nothing. Members with the same folded name match at the same anchors, so the ambiguity rule still sees all of them. Longer names that win the greedy match also prefix the tail, so they are loaded too. Authored tails are included because authority (
buzz:workflow-mention) is resolved against the authored text with the same members.Testing
candidate_filter_resolves_like_the_full_roster: a table of 37 names with spaces and punctuation,İand decomposedi̇,ß/ẞ, emoji, Greek sigma, the Kelvin sign, same-name members, a same-pubkey duplicate, prefix names (Will/Will Pfleger,Rob/Robby/Robby Jr.) and unrelated members, run against 30 rendered/authored pairs with one or more mentions. Thepandbuzz:workflow-mentiontags from the filtered roster equal the tags from the full roster, and each filter admits fewer than half the members. The filter rule is restated in Rust in this test; the Postgres tests check the SQL.@İ @Robby. Dropping the authored tails failed oncc @Will/cc @Will Pfleger, where authority would otherwise go toWill.anchor_rule_matches_the_resolver: for every BMP char before@Robby, an anchor is found exactly when the resolver matches.mention_candidates_skip_the_database_without_an_anchor(unreachable database):build passed,mail alice@Robbyand a trailing@returnOk(empty);ping @RobbyreturnsActionSinkError::Database.text_without_at_never_resolves_a_mentionkeeps the old guard's test.postgres:16container with the desired-state schema:named_members_filter_returns_only_admitted_names(buzz-db): a 1,000-member channel returns only admitted names in roster order.R_b%is not treated as a pattern, and ASCIIIlkeris not matched by the taili̇lker. Removed members, other channels' members and nameless users are excluded, andNonereturns the full named roster.mention_candidates_read_only_names_a_mention_can_match(relay): in a 537-member channel,ping @Robbyreads 2 rows (Rob,Robby), and every table case resolves to the same tags as the full roster with at most 20 rows. Failing before: with the filter disabled (full roster, the old read), it fails with 537 rows instead of 2.workflow_sink::postgres_tests: all 6 pass, includingworkflow_send_message_binds_authority_to_authored_mentions.production_db_methods_emit_exact_pool_operation_labels: passes.cargo test -p buzz-relay --lib -- workflow_sink::tests: 28 passed.cargo test -p buzz-db --test observability_source: 23 passed.scripts/test-postgres-test-discovery.sh: passed.cargo fmt --all --checkandcargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings: clean.Database checklist: this is a query-only change with no schema phase, and the deployed relay is unaffected. No migration mutates rows. No index is added: the query still scans the channel's member rows (as
get_membersdid) and filters them, so the saving is in rows returned and in one read instead of two.Not covered
ORDER BY, so this was already unspecified. The new query orders byjoined_at, pubkey.