Extract community persistence - #6668
Conversation
5c245fd to
6f50e6b
Compare
wpfleger96
left a comment
There was a problem hiding this comment.
🤖 Reviewed at 6f50e6b. Verified the pure-move claim mechanically: diffing the 872 lines removed from buzz-db/src/lib.rs against the new community.rs as whitespace-stripped line multisets shows zero lines lost — every SQL string, doc comment, datastore span, and Postgres test body carries over unchanged. The only additions are module scaffolding, module-local test helpers, and the new community_implementation_tests_and_spans_have_single_owners guard, which runs in the infra-free unit job and pins single ownership of all 13 operations, 7 record/result types, and 8 moved tests — nice drift protection for this extraction series. Crate-root re-exports preserve every downstream path (community_provisioning.rs, tenant.rs, workflow_sink.rs verified unchanged).
Live verification also exercised operator provisioning, wrong-owner archive/unarchive rejection, fail-closed archived-host serving, and cross-community isolation against a release relay at this exact SHA — all held. The moved ignored Postgres tests still fall outside CI's selected lanes, which is the pre-existing gap you're addressing separately.
Signed-off-by: tornquist <tornquist@squareup.com>
6f50e6b to
9046f8b
Compare
…c-agent-commit-identity * origin/main: (54 commits) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) fix(desktop): emit singular `mention` feed category so alerts route correctly (#6665) fix(mobile): recover stale and shuffled messages (#6691) feat(mobile): browse and join open channels (#6243) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…trigger-foundation * origin/main: Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) Signed-off-by: Logan Johnson <loganj@squareup.com> # Conflicts: # crates/buzz-relay/src/handlers/command_executor.rs
…picker * origin/main: (57 commits) Add staging dev relay image workflow (#6709) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) fix(desktop): emit singular `mention` feed category so alerts route correctly (#6665) fix(mobile): recover stale and shuffled messages (#6691) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
…arer-auth * origin/main: (58 commits) Fix TipTap editor mount race (#6779) feat(buzz-agent): gate LLM tool calls on session/request_permission (#5712) Add staging dev relay image workflow (#6709) Extract community persistence (#6668) Fix mobile Huddle agent voice turn states (#6611) Add inline profile camera capture (#6680) Hide Huddles in mobile agent DMs (#6676) fix(desktop): polish inline chip states (#6718) Centralize replaceable event persistence (#6660) feat(workflows): discover trigger filter values (#6712) feat(desktop): simplify the message action rail (#6529) fix(desktop): restore icon-only remote marker (#6491) fix(ci): prevent poisoned Rust caches (#6618) docs(security): route reports through private advisories (#6728) fix(composer): wrap Buzz chip labels without orphaning icons (#6581) fix(desktop): bound thread /query and surface load errors, not false-empty (#6447) fix(messages): route edits to the owning composer (#6575) fix(mobile): join starter channels after accepting invite (#5915) Add mobile profile editing (#6583) fix(desktop): align jump-to-latest pill with composer height (#6606) ... Signed-off-by: Duncan <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz> # Conflicts: # crates/buzz-db/src/lib.rs
## Why Database pressure currently collapses several distinct delays into one symptom. This adds the evidence layer needed to distinguish pool acquisition wait, logical database operation time, advisory-lock wait, and selected transaction duration before changing timeout or retry policy. This is the phase 2 Lane A observability bundle for [#26](TheSentinel454#26), [#28](TheSentinel454#28), and [#33](TheSentinel454#33). It is stacked on #6668. ## What - Record explicit reader/writer checkout wait and acquisition outcomes with `buzz_db_pool_acquire_wait_seconds` and `buzz_db_pool_acquisitions_total`. - Extend the compile-time `#[datastore_span(name = "...")]` seam with `buzz_db_operation_duration_seconds`, so operation labels remain static source literals instead of request data. - Route correctness-critical replacement, membership, push-gate, deletion, and migration/schema-safety advisory locks through one observer without changing their SQL, order, scope, or blocking behavior. - Measure six internally owned transaction lifetimes with `buzz_db_transaction_duration_seconds`, starting after `BEGIN` succeeds and ending after explicit commit/rollback or scope exit. - Emit root slow-operation warnings at 500 ms, logging the first slow completion and then 1/100 per call site with only `operation`, `outcome`, and `elapsed_ms`. - Document names, units, fixed label vocabularies, measurement boundaries, and blind spots in this PR description. Fixed labels are deliberately small: - `pool_role`: `writer`, `reader` - `lock_type`: `replacement`, `membership`, `push_gate`, `deletion`, `migration_schema_safety` - `outcome`: `success`, `error`, `timeout` where SQLx/PostgreSQL can distinguish it accurately - `operation`: compile-time datastore names plus the six closed transaction operation names documented in the runbook No metric or slow warning contains community IDs, event IDs, event kinds, coordinates, d-tags, SQL/query text, query IDs, returned errors, or event content. ## Coverage boundaries - Operation duration is the complete annotated logical function body, not pure SQL execution; it may include implicit checkout, lock wait, nested operations, and application work. Cancelled futures do not reach its completion hook. - Pool timing covers explicit helper checkouts, including proved-reader routing and selected writer-owned transactions. Implicit SQLx checkout through `&PgPool` remains folded into operation duration. - Lock timing covers application-side blocking locks in the five named families. Trigger/stored-procedure locks, channel-TTL locking, the usage try-lock, and the audit service session lock remain outside this slice. - Transaction timing covers only the six wholly owned boundaries documented in the runbook. It excludes pool wait, `BEGIN`, asynchronous rollback cleanup after an early return, and caller-owned `Db::begin_transaction` lifetime. ## Relationship to #6229 #6229 is the incident-driven timeout precursor. This PR does not add or change `statement_timeout`, `lock_timeout`, `idle_in_transaction_session_timeout`, retries, audit durability, or client-visible conflicts. It provides the missing distributions needed to evaluate those policies later and intentionally leaves #6229's open audit retry/durability finding untouched. The branches overlap in `crates/buzz-db/src/lib.rs` and `crates/buzz-db/src/migration.rs`, so a later rebase may need textual conflict resolution, but the behavior is complementary rather than duplicated. ## Risk assessment Moderate-low. The primary risk is instrumentation overhead and added static series. Cardinality is source-bounded, slow logs are sampled/redacted root events, and the lock/transaction changes wrap existing awaits without changing policy or ordering. ## Verification Author workstation: `buzz-tornquist-db-pressure-observability` (`2010927`), exact head `d7cf833e26c528adfcde3917ded80daf6f4ddac9`, parent `6f50e6b2b2a996349149af61d35bdd6a355f77fd`. - `cargo fmt --all --check` — passed - `cargo clippy -p buzz-datastore-tracing -p buzz-db -p buzz-audit -p buzz-search -p buzz-relay --all-targets -- -D warnings` — passed - `cargo test -p buzz-datastore-tracing --quiet` — 4 passed - `cargo test -p buzz-db --quiet` — 109 passed, 200 ignored - `cargo test -p buzz-audit -p buzz-search --quiet` — 16 passed, 25 ignored - `cargo test -p buzz-relay --lib --quiet -- --test-threads=1` — 906 passed, 48 ignored - Native PostgreSQL focused tests for pool success/timeout/error, lock success/contention/timeout/error, replacement, membership serialization, push ordering, deletion fencing, migration/schema exclusion, and reader fallback — 8 passed The default-parallel relay run passed once; subsequent runs exposed the existing load-sensitive `api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo` 504 at the end of the suite. That test passes in isolation and the full relay suite passes serially. Independent exact-head review workstation: `buzz-tornquist-db-pressure-observability-review` (`2013067`). Formatting, the same all-target clippy command, datastore instrumentation tests, DB unit tests, source privacy guards, and diff/non-goal audits passed; no review findings. Generated with Codex --------- Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Matt Kursmark <kursmark@squareup.com> Signed-off-by: Matt Kursmark <kursmark@squareup.com> * origin/main: (21 commits) feat: navigate images across message threads (block#6705) Add database pressure observability (block#6700) revert fixed mention highlight (block#6716) highlight search terms in results and messages (block#6702) fix(desktop): make lightbox zoom controls interactive (block#6710) Support community deletion in versioned media buckets (block#6738) Fix TipTap editor mount race (block#6779) feat(buzz-agent): gate LLM tool calls on session/request_permission (block#5712) Add staging dev relay image workflow (block#6709) Extract community persistence (block#6668) Fix mobile Huddle agent voice turn states (block#6611) Add inline profile camera capture (block#6680) Hide Huddles in mobile agent DMs (block#6676) fix(desktop): polish inline chip states (block#6718) Centralize replaceable event persistence (block#6660) feat(workflows): discover trigger filter values (block#6712) feat(desktop): simplify the message action rail (block#6529) fix(desktop): restore icon-only remote marker (block#6491) fix(ci): prevent poisoned Rust caches (block#6618) docs(security): route reports through private advisories (block#6728) ... Signed-off-by: Matt Kursmark <kursmark@squareup.com>
## Why Finish the replaceable-event slice of [tracker #2](TheSentinel454#2) and [domain issue #6](TheSentinel454#6) without disturbing the runtime/store boundary established by #6660 and #6668. PR #6700 has merged; this PR now targets current main containing its replacement lock and transaction observability. ## What - Move `Db::replace_addressable_event`, its SQL, lock/transaction instrumentation, and focused addressable/parameterized replacement tests from `lib.rs` to `replaceable.rs` - Preserve the transaction-required parameterized API, replacement ordering, rollback semantics, mention indexing, and exactly one datastore span per public operation - Keep implementation and focused PostgreSQL tests co-located; follow-up review removed the dedicated replaceable ownership source guard as low value ## Stack - Exact base: main at f249710 - Exact head: codex/issue-6-finish-replaceable-store at da01840 - Tracker: TheSentinel454#2 - Domain: TheSentinel454#6 - Test/span acceptance: TheSentinel454#17 and TheSentinel454#19 ## Non-goals - No SQL, schema, retry, timeout, lock ordering, transaction boundary, or client-visible behavior changes - No store traits, domain-handle redesign, `PgExecutor` migration, raw pool accessor, new crate, or directory reorganization - No changes to, retargeting of, or merge action on PR #6700 ## Risk Assessment Low. This is a mechanical ownership move with unchanged signatures and SQL. The primary review risk is losing or nesting instrumentation, covered by exact moved PostgreSQL tests and the cumulative production-diff audit. ## Blox Verification Author workstation: `buzz-tornquist-issue-2-store-stack` (`2046520`), exact head `2de5444e14606ce6912d1bb932bc88bffa379a77`. - `cargo fmt --all --check` — passed - `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` — passed - `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; both integration source guards passed - Native PostgreSQL: `cargo test -p buzz-db replaceable::tests:: -- --ignored --test-threads=1` — 11 passed - `cargo test -p buzz-relay --lib api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo -- --exact --test-threads=1` — passed - Full serial relay suite was also exercised: 908 tests passed but the existing load-sensitive mesh demo test returned 504 under suite load, matching PR #6700's documented baseline; it passed in isolation and this diff does not touch that subsystem Independent exact-head Blox review: `buzz-tornquist-pr-6777-review` (`2047422`) at `2de5444e14606ce6912d1bb932bc88bffa379a77`. - Review findings — none (critical, important, or minor) - `cargo fmt --all --check` and the ownership guard — passed - `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings` — passed - `cargo test -p buzz-db --quiet` — 108 passed, 200 ignored; source guards passed - Native PostgreSQL moved suite — 11 passed - `cargo test -p buzz-relay --lib` — 909 passed, 48 ignored Generated with Codex ## Superseded pre-comment restack verification PR #6700 merged before publication completed. This layer was restacked onto current main through the exact parent named above; the final cumulative tip is 2ddcc8a. Cumulative author gates passed: formatting and diff checks; buzz-db and buzz-relay all-target clippy with -D warnings; DB lib 111 passed / 200 ignored; ownership 22/22; observability 1/1; the full isolated PostgreSQL domain matrix; and relay lib 910 passed / 49 ignored. - Workstation: `buzz-tornquist-pr-6777-final-review` (`2057617`), fresh shallow checkout - Base: `f24971033178926153b49d320bd876d15d9cb2bf` - Head: `ffbeaaf00810aa359ab85818ff4820f92263e45f` - Findings: none Reviewed `base..head` for the replaceable-store extraction. The addressable replacement SQL, stale/duplicate outcomes, advisory-lock ordering, transaction rollback/commit boundaries, mention-index atomicity, and public `Db` signature are retained in `replaceable.rs`. The PR #6700 transaction timer remains around the same logical transaction through `TransactionTimer::observe`, and the public operation has exactly one datastore span. Focused addressable and parameterized tests moved with the implementation; the ownership guard excludes duplicates from `lib.rs`. Verification: format and diff checks passed; `buzz-db --all-targets` clippy passed with `-D warnings`; DB lib tests passed (111 passed, 200 PostgreSQL tests ignored); ownership (1/1) and observability (1/1) guards passed; all 11 replaceable PostgreSQL tests passed on native PostgreSQL 17 with migrations 1-32 successful; relay lib test target compiled successfully. Final worktree was detached at the exact head and clean. Complete evidence archive SHA-256: `e37f92bbde4e2343196085095b3093b1bc78196be58bc86826325e0e1639d7ea`. ## Comment-addressed restack Review follow-up removed the dedicated `replaceable_store_has_single_ownership` source test as requested. No production code changed. - Exact base: `f24971033178926153b49d320bd876d15d9cb2bf` - Exact head: `da018405cc83605362125c2d5e5a3f91492431ac` - Final cumulative tip: `6fa2f104d42c6ba85bdf62e7ccb74ceaf4a84f67` - Focused PR verification: formatting and diff checks passed; strict `buzz-db`/`buzz-relay` Clippy passed; DB lib 111 passed / 200 ignored; all 11 replaceable PostgreSQL tests passed. - Cumulative Blox gate: formatting and diff checks; strict `buzz-db`/`buzz-relay` Clippy; DB lib 111 passed / 200 ignored; ownership 21/21; observability 1/1; every moved PostgreSQL test; relay lib 910 passed / 49 ignored. - Independent re-review at this exact head: no findings; fresh exact-parent/head Blox review passed fmt/diff, strict `buzz-db` Clippy, DB lib 111 passed / 200 ignored, manual one-owner/one-span replacement checks, observability, 11 replaceable PostgreSQL tests, and relay compilation. --------- Signed-off-by: tornquist <tornquist@squareup.com>
Why
Community persistence is the next incremental
buzz-dbstore extraction, keeping tenant lifecycle SQL, records, tests, and instrumentation out of the database runtime module without changing behavior.What
impl Dboperations intocommunity.rswhile preserving crate-root re-exports.Risk Assessment
Low — this is a structural move of the existing records, SQL, method bodies, and focused tests; database runtime concerns, schema, and behavior remain unchanged.
References
Generated with Codex