Commit Graph

2 Commits

Author SHA1 Message Date
Luke Tornquist
a3730784fc refactor(db): extract domain stores from database runtime (#6987)
## Summary

Finish the remaining database-store extraction tracked by
[TheSentinel454/buzz#2](https://github.com/TheSentinel454/buzz/issues/2)
in one reviewable PR.

This consolidates the previously stacked domain slices after #6782
merged. It preserves the runtime/store boundary established by #6660,
#6668, #6700, and #6782 while separating database runtime infrastructure
from domain-owned persistence:

- `runtime/` owns pool construction and sizing, writer/reader routing,
read sessions and route proofs, transaction infrastructure,
observability primitives, replica fencing, health support, migrations,
and cross-cutting runtime tests.
- `store/` owns domain records, SQL, row parsing, locks and invariants,
`Db` domain methods, focused tests, and logical-operation datastore
spans.
- `lib.rs` remains a 57-line compatibility facade that preserves
existing crate-root paths and `Db` method signatures through re-exports.

Domain coverage includes API tokens, authentication allowlists,
reminders, event queries, threads, reactions, feeds, users and DMs,
push, workflows/runs/approvals, relay membership and invites, product
feedback, moderation/admin moderation, relay admin actions/operators,
git repositories, archived identities, usage, partition maintenance,
deletion, channel membership inherited from merged #6782, and the final
runtime/store layout.

The branch has been rebased onto current `main`. Database changes that
landed there were incorporated rather than overwritten:
`relay_admin_actions.rs` and `relay_operators.rs` now live under
`store/`, their 27 public `Db` wrappers and existing behavior remain
intact, and every wrapper has exactly one fixed-name datastore span.
Concurrent changes to migration, moderation, admin moderation, and error
handling are also retained.

### Exact base and head

- Base: `main` at `ed11c8d8bf0a17402be5cf243724f89471530d2f`
- Head: `codex/issue-2-store-extraction` at
`be24430472d1a87ac5c0d6026c620cd6caea3537`

### Related issue

- Structural tracker:
[TheSentinel454/buzz#2](https://github.com/TheSentinel454/buzz/issues/2)
- Domain trackers:
[#6](https://github.com/TheSentinel454/buzz/issues/6),
[#7](https://github.com/TheSentinel454/buzz/issues/7),
[#12](https://github.com/TheSentinel454/buzz/issues/12),
[#13](https://github.com/TheSentinel454/buzz/issues/13)
- Acceptance trackers:
[#17](https://github.com/TheSentinel454/buzz/issues/17),
[#19](https://github.com/TheSentinel454/buzz/issues/19)

This supersedes #6783, #6784, #6787, #6788, #6789, #6792, #6820, #6794,
#6796, #6797, #6798, #6799, #6804, #6805, #6806, #6808, #6809, #6811,
#6812, #6813, #6814, #6815, and #6890. Their discussions remain
available for review history.

### #17 / #19 acceptance

- Preserves the metric names, fixed labels, transaction/lock timing
boundaries, and privacy/cardinality constraints introduced by #6700.
- Keeps exactly one datastore span per public logical operation,
including the 27 relay-admin wrappers added on `main`.
- Removes `store_ownership.rs`; physical ownership and focused source
guards now enforce the boundary directly.
- Leaves no `impl Db`, domain SQL, focused domain test group, or
datastore span in `lib.rs`.
- Preserves existing public paths such as `buzz_db::channel`,
`buzz_db::event`, and `buzz_db::workflow` through crate-root re-exports
while keeping internal `runtime` and `store` namespaces private.

### Non-goals

- No SQL, schema, locking, transaction, retry, timeout, or
client-visible behavior changes.
- No generic store traits, domain handles, broad `PgExecutor` migration,
new store crate, raw pool accessor, or broader directory reorganization.
- No tracker issues are closed by this PR.

### Risk

The cumulative diff is large but structural. Risk is primarily
module-path, ownership, or conflict-resolution drift. It is mitigated by
preserving public re-exports, comparing the newly moved `main`
implementations to their upstream source, source guards, touched-crate
compilation, PostgreSQL-backed test coverage, and an independent
exact-head review on a separate clean Blox workstation.

### Testing

Author workstation `buzz-tornquist-pr-6987-rebase`, rebased branch
ending at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 PostgreSQL tests
intentionally ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- PostgreSQL-backed `buzz-db` coverage under native PostgreSQL — 235
passed in the shared serial run; the five shared-state/config-sensitive
cases passed as isolated reruns against fresh schemas, including the two
owner-limit tests with their fixture's
`BUZZ_MAX_COMMUNITIES_PER_OWNER=3`
- `cargo test -p buzz-relay --lib -- --test-threads=1` under native
PostgreSQL/Redis — 991 passed; the three current-month
partition-sensitive identity-archive cases passed after provisioning the
August 2026 test partition; 87 infrastructure-marked tests remained
ignored
- Source/diff guards — relay-admin implementation bodies match current
`main`; all 27 public wrapper signatures are retained; exactly one
datastore span wraps each wrapper; `lib.rs` has zero `impl Db` blocks
and zero datastore spans; no duplicate top-level relay-admin modules or
`store_ownership.rs`; `error.rs` matches current `main`

Independent clean review workstation `buzz-tornquist-pr-6987-review`,
detached at exact head `be24430472d1a87ac5c0d6026c620cd6caea3537`:

- `cargo fmt --all --check`
- `cargo clippy -p buzz-db -p buzz-relay --all-targets -- -D warnings`
- `cargo test -p buzz-db --lib` — 113 passed, 240 ignored
- `cargo test -p buzz-db --test observability_source` — 2 passed
- Exact-head ownership/re-export/instrumentation audit — no remaining
actionable findings

---------

Signed-off-by: OpenAI Codex <codex@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
2026-08-28 15:24:32 -04:00
Luke Tornquist
113a33b7e4 Add database pressure observability (#6700)
## 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](https://github.com/TheSentinel454/buzz/issues/26),
[#28](https://github.com/TheSentinel454/buzz/issues/28), and
[#33](https://github.com/TheSentinel454/buzz/issues/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>
2026-08-25 13:00:22 -04:00