mirror of
https://github.com/block/buzz.git
synced 2026-10-27 20:41:57 +00:00
codex/agent-pool-startup
6 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
16eb0b6685 |
feat(buzz-db): run push enqueue and channel TTL refresh in app code (#8135)
## Summary Moves the producer side of the two event follow-up triggers into application code. The triggers stay in place, so both producers run until a later change drops the triggers after this ships to every pod. Builds on #8130 (`AdmittedTx`). Refs BUZZ-176. ### Push match enqueue New `store::event_follow_up::enqueue_push_match` replaces the `events_enqueue_push_match` trigger (`schema/functions/enqueue_push_match_job.sql`, migration 0023) and matches it exactly: - Same kind allowlist (9, 40002, 45001, 45003), now `PUSH_MATCH_KINDS`. - Takes the same SHARED `buzz_push_gate:<community>` advisory lock as its own statement, so under READ COMMITTED the lease check gets a snapshot taken after the lock is granted. Lease activations still take it EXCLUSIVE in `push.rs`. - Same lease predicate (`active AND endpoint_enabled AND expires_at > now`), then `INSERT push_match_queue ... ON CONFLICT DO NOTHING`. - Fails closed: any error rejects the event, as the trigger did. It runs right after the INSERT, only when the insert affected a row. The consumer side (claims, `lease_until`, attempts, reaper) is already app-owned and is unchanged. ### Channel TTL refresh `AdmittedTx` now records every channel that received an event (except kind 9007 and channel-less events). `AdmittedTx::commit()` refreshes their `ttl_deadline` as the last statement before COMMIT, which keeps the commit-time behavior of the deferred `events_refresh_channel_ttl` trigger (migration 0024): - One savepoint per channel, in sorted order, under the same SHARED `buzz_channel_ttl:<community>:<channel>` lock. - Runs the same `UPDATE` with the same `archived_at`/`deleted_at` filters as its own statement after the lock, so its snapshot follows the lock grant. Its `ttl_seconds IS NOT NULL` qual filters a permanent channel before any tuple lock, so a permanent channel's row is never locked. An ephemeral channel's row lock is held through the remaining refreshes and the COMMIT round trip (the trigger held it only inside COMMIT). - Lock keys come from one builder per protocol (`channel::channel_ttl_lock_key`, `push::push_gate_lock_key`), shared by the SHARED and EXCLUSIVE sides. The shared push-gate wait is observed through `observe_advisory_lock(LockType::PushGate, …)`. - On failure it rolls back the savepoint, logs a warning, and increments `buzz_db_channel_ttl_refresh_failures_total{reason}`, where `reason` is `lock_timeout` or `error`. The event still commits, matching the trigger's `EXCEPTION WHEN OTHERS`. A cancelled statement (`57014`) still rejects the transaction, because plpgsql never caught that either. - While both producers run, an app refresh that hits `lock_timeout` is counted, and then the trigger retries it inside COMMIT and may succeed. So during the dual window the failure counter can over-report and the worst-case wait doubles. Both go away when the retirement PR drops the triggers, after which the counter is exact. ### Call sites Every production `INSERT INTO events` calls `event_follow_up::after_admitted_insert`: - `insert_event_on` - `insert_event_with_thread_metadata_tx` - `LockedMemberSnapshot::replace_member_event` - `replace_addressable_event` - `publish_nip43_membership_locked` - `accept_lease_event` (kind 30350, a no-op today, kept so the policy has no exceptions) `replace_parameterized_event_in_transaction_impl` inserts inside the `parameterized_replace` savepoint. It enqueues the push job inside the savepoint, so a rollback to that savepoint also drops the job, and records the TTL channel only after the savepoint is released. The two rolled-back verification probes (`verify_floor_guard_behavior`, `verify_channel_roster_fence_behavior`) are named exceptions, because they never commit. ### Source policy `tests/observability_source.rs` adds `every_production_event_insert_runs_the_follow_up_hook`. It scans every `crates/*/src` and fails on any function that inserts into `events` without the hook. Matching is normalized (letter case, whitespace, `\` continuations, `\n`/`\t`/`\r` escapes, optional `public.`). The file is masked once first: comments (`//`, nested `/* */`) and string, raw-string (including `br"`/`cr"`), and char literals become spaces. `#[cfg(test)]` markers, `fn` signatures and the hook are all found in that masked view, so a marker in a comment (even a multiline block comment) strips nothing, an `fn` line inside a multiline string doesn't split a function, and a commented-out or quoted hook doesn't count. SQL is still matched in the original text. A table test pins the masker's output for each literal form. A fixture test pins what the scan rejects (no hook, push without TTL, a hook in a line comment, block comment, or string, lowercase or line-broken or escaped SQL, writers after test-only items or under comment mentions) and what it ignores (writers in `#[cfg(test)] mod`). A floor of seven hooked writers catches a scan that silently stops seeing writers. ### Tests `src/store/event_follow_up_postgres_tests.rs` is the contract suite from the BUZZ-176 discovery work, ported to the `AdmittedTx` API. It is a crate-internal `#[cfg(test)]` module because it runs `SET LOCAL` timeouts and in-transaction reads through the crate-private `AdmittedTx::conn()`. It runs on both the desired and migration schemas, in two arms: - **dual**: production during the migration window, with the triggers and the app code both running. - **app-only**: drops both triggers in the disposable database (and asserts no partition clones remain), the way the retirement migration will. The trigger-only baseline is the base branch, where the same suite passed before this change. The suite also pins the two branches that differ from "swallow everything": a `lock_timeout` failure increments `buzz_db_channel_ttl_refresh_failures_total{reason="lock_timeout"}` exactly once, and a statement cancel (`57014`) during the refresh rejects the event with no row, job, or deadline change. ## What got simpler Nothing yet. This adds a parallel producer during the migration window. It is what lets the follow-up PR delete two plpgsql functions, two triggers, and the "keep in sync with migrations/00xx" duplication between `schema/functions/` and `push.rs`. ## Verification Rebased onto `main` @ `745501962`. All runs used a Linux cloud workstation with the pinned toolchain (rustc/clippy 1.95.0, hermit), and the tree was clean before and after each run. The final head, `c5a06a82`, changes only `crates/buzz-db/tests/observability_source.rs` compared with `1eb11bde` (via `fbc9a598`, `7c0baa41`, `9199992c` and `d405b2f6`). Production source is identical between the two. | Check | SHA | Result | |---|---|---| | `cargo fmt --check`, `cargo clippy -p buzz-db --all-targets -D warnings` | `fbc9a598` | pass | | `cargo clippy` (workspace) | `1eb11bde` | pass | | source policy (`--test observability_source`), fmt, buzz-db clippy | `c5a06a82` | 17/17, pass, pass | | `cargo test --locked -p buzz-db` (non-PG) | `c5a06a82` | 148 lib + 17 policy + 5 doctests | | source policy (`--test observability_source`) | `fbc9a598` | 16/16 | | `cargo test --locked -p buzz-db` (non-PG) | `fbc9a598` | 148 lib + 16 policy + 5 doctests | | contract suite (`store::event_follow_up_postgres_tests`, desired/migration × dual/app-only) | `1eb11bde` | 4/4 | | buzz-db PG full | `1eb11bde` | 464/464 | | buzz-relay PG | `1eb11bde` | 378/378 | | GitHub CI | `fbc9a598` | 53 pass, 0 fail (Codex security review pending) | **Falsifications** (each mutation made the named test fail, then was reverted): - dropping the `relay_members` hook → policy names `publish_nip43_membership_locked` - no-op push producer → app-only arms fail, dual arms pass (trigger covers) - skipping the TTL refresh → app-only fails on the deadline; dual fails on the failure-metric assertion - swallowing `57014` → all four arms fail - dropping the metric increment → all four arms fail - unanchored `#[cfg(test)]` match, or no escape handling → fixture test fails - a `#[cfg(test)]` constructor that skips admission, added inside `impl AdmittedTx` (indented or as a column-0 `mod`) → `admitted_tx_is_constructed_only_by_admitting_constructors` fails; putting the strip back into the constructor scan → its fixture test fails - at `7c0baa41`: `accept_lease_event`'s hook commented out and the real call made through an alias → policy names `accept_lease_event` (the same mutation passes under the `fbc9a598` policy); hook matching on unmasked text → the comment/string fixtures fail - at `9199992c`/`d405b2f6`: no nested block-comment depth, no string-escape skip, no `br"`/`cr"` raw-literal prefix, or no raw-string branch at all in the masker → the follow-up fixture test fails - at `c5a06a82`: finding `#[cfg(test)]` in the unmasked text → `writer_after_block_commented_cfg_test` is no longer reported; finding `fn` signatures in the unmasked text → `writer_after_raw_string_example` is no longer reported; no `cr` raw prefix → the masker table test fails on `cr"` - unhooked writers appended to `side_effects.rs` (after test-only items, under a doc-comment mention, with an escaped newline) → policy names each **Live relay E2E**: an independent reviewer ran this at `1eb11bde` against a real relay and CLI, with PostgreSQL 17 and Redis 7, the follow-up triggers absent, and no partition clones. - A kind 9 on a TTL channel advanced the shortened deadline by about 580 s. On a permanent channel, the deadline stayed NULL. - Each accepted event produced exactly one pending `push_match_queue` row with the same event id. - An exact signed replay was a no-op. - While the exclusive TTL lock was held, the event and its job stayed invisible and the deadline didn't change. After release, both persisted once. - Roster create/join/add/remove kept SQL and wire state in step (1→2→3→2), with one current 39002 head. - NIP-33 replace, duplicate, stale, failure and retry cases all passed. Limits: the push lease was a SQL fixture, and push consumers were disabled. Kind 30023 is global, so channel-scoped TTL on the parameterized path is covered only by the PG contracts. No live relay rerun was done at `fbc9a598`, because that delta is test-only. **Latency** (measured at pre-rebase `f8380927`, same production delta; debug build on loopback, 1,000 samples per cell, EVENT→OK): head app-only p50/p99 3.80/4.58 ms (permanent) and 3.87/4.64 ms (TTL), against base trigger-only 3.49–3.53/4.00–4.03 ms and 3.56–3.62/4.30–4.46 ms. About +0.3 ms p50, from the extra client round trips (lock, UPDATE, savepoints, push lock/insert) that the triggers ran server-side. One debug loopback run, not a production p99. Not measured: contention from many writers on one hot TTL channel, where each writer now waits on the channel row lock for about two more client round trips (RELEASE SAVEPOINT and COMMIT) than with the trigger. In review, a burst of 8 concurrent posts to one TTL channel committed with no deadlock or lock timeout. Watch send p99 and `pg_locks` waits on `channels` during rollout. **Known pre-existing, not touched here:** `api::mesh_demo::tests::demo_join_forwarded_arm_round_trips_echo` is flaky when Redis is present. It returns a 504 in 4 of 5 runs at base `745501962` and 2 of 5 at `fbc9a598`, always at the same assertion. No CI lane selects it. --------- Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> |
||
|
|
f8bf2c529c | refactor(db): type-level admitted event-write transactions (AdmittedTx) (#8130) | ||
|
|
d0fd96fca6 |
BUZZ-175: Route serving event writes through the tenant lock (#7828)
## Summary
Route every serving event write through one community-admitted
transaction entry point. `Db::begin_event_write_transaction(community)`
is now the only `Db` constructor for event-write transactions: it takes
the shared community admission lock before returning, so callers that
use it cannot take domain or row locks ahead of tenant admission, and a
quiescing community rejects the write at entry. Database triggers and
commit-time fences remain the authoritative backstop.
This migrates normal, replaceable, roster, relay membership, reaction,
reminder, admin-delete, push-owned, artifact-accept, huddle-join
(48101), workflow-deletion, and command-executor event writes. Coupled
mention writes stay in the same transaction; pool-level
`insert_mentions` now opens its own admitted transaction.
**Review fixes (since `950d5f6`):**
- The unguarded event-write constructor is gone. Removed with it: the
deprecated `begin_transaction` alias, the test-only replica-floor
constructor, and `begin_community_write_transaction` (now identical to
the single constructor). Artifact accept, the huddle join, and both
workflow-deletion paths now admit before any domain or row lock;
`persist_command_event` no longer hand-rolls `guard_transaction`;
read-only `query_artifacts` no longer opens an event-write transaction.
- `insert_channel_head_checked` is now `insert_canvas_head_checked`: it
rejects non-canvas kinds before opening a transaction and always takes
the coordinate lock.
- Source policy now also scans `buzz-relay`: any function that hands a
transaction to an event-write helper must have opened it through the
admitted constructor. The scan drops each `#[cfg(test)]` item instead of
stopping at the first one: it tracks brace depth and ends the item on a
line ending in `;` or `}` (ignoring a trailing `//` comment, even after
a string that contains `//`), with a column-0 `}` as fallback. Fixtures
pin a unit struct, a multi-line static, a `const { … }` block, a struct
with fields, a module, and one-line items with trailing comments,
including after `"https://…"` and `"ws://…"` strings; a raw opener after
each must still be flagged. Known limit: braces inside string or char
literals or comments are still counted, and only `//` trailing comments
are recognized, so a test-only item with unbalanced braces in a literal
or comment, or with a trailing `/* */`, would hide what follows; none
exists today. The scan checks that admission is present; the per-path
PostgreSQL tests pin that it comes before domain locks.
- `just test-unit` (and the non-nextest fallback in
`scripts/run-tests.sh`) now runs `buzz-db --test observability_source`.
Before this, no CI job ran these source-policy tests: the unit lane ran
`buzz-db --lib` only and the PostgreSQL lane runs only ignored tests.
- Renamed or removed since `950d5f6`: `begin_transaction`,
`begin_community_write_transaction`, and the replica-floor test
constructor are gone; `insert_channel_head_checked` →
`insert_canvas_head_checked`; test helper `quiesce_community_for_tests`
now delegates to `main`'s `set_deletion_state`, which reads the real
`deletion_fence_generation` instead of hard-coding `'0'`;
`runtime::insert_mentions` is pinned to the admitted constructor by
`pool_level_insert_mentions_opens_the_tenant_local_chokepoint`.
**What got simpler:** four transaction constructors collapse to one, and
the hand-rolled admission guard is deleted. The compiler rejects callers
of the removed constructors, but a transaction opened from `Db::pool()`
can still reach the public `*_in_transaction` helpers; only the
source-policy scan and the database fences catch that until BUZZ-254.
**Behavior note:** #7932 made `insert_event` skip the transaction for
kinds that are not listener mentions. Every serving event write now has
to run in a transaction that holds the community admission lock, so
`insert_event` always opens the admitted transaction again. `main`'s
`soft_delete_event_and_update_thread` / `_in_tx` split and its
removal-marker guard are kept; the outer function now opens the admitted
transaction.
### Related issue
[BUZZ-175](https://linear.app/squareup/issue/BUZZ-175). Caller-migration
stage following #7706 (merged); rebased onto `main` at `5173fad66`.
Follow-up: [BUZZ-254](https://linear.app/squareup/issue/BUZZ-254) makes
event-write helpers require a type-level admitted transaction.
**Known debt, out of scope:** `create_channel`, `update_channel`, and
`insert_thread_metadata` write channel and thread rows (not events)
outside the admitted constructor.
### Testing
At `efe4a94623dd69973780a3c39f16dd972cf7cb76` on a cloud workstation:
fmt; `buzz-db` clippy (all targets, `-D warnings`);
`observability_source` 14/14; `just --dry-run test-unit` includes the
new step. Reverting just the end-of-item check to an earlier rule makes
a trailing-comment fixture fail.
At `eaf190ae59d1595741c3d19dc4c23ce492b5f4d4` (the commit after it only
changes this test file, `Justfile`, and `scripts/run-tests.sh`): full
GitHub CI green; on the workstation, clippy for `buzz-db` and
`buzz-relay`, ordinary `buzz-db` 154/154, `just test-unit` green, and
the PostgreSQL lane 805 of 806 (exception below). An end-to-end run
against a local relay and the built desktop app covered artifact accept,
huddle join, and workflow deletion for both a healthy and a quiescing
community, plus p-tag reaction rollback and replay of the same signed
event.
New tests: one fenced-community test per migrated path (artifact accept,
huddle join, both workflow deletions) puts the community into
`quiescing`, holds the lock the path takes next, and asserts rejection
at entry with nothing stored; a DB-free test pins the non-canvas
rejection.
Exceptions:
-
`runtime::postgres_tests::writer_pool_rejects_non_read_committed_database_default`
hangs on that workstation's local Postgres 16 at both this head and an
earlier `main` base, `f0eb5575f` (2/2 runs each). The test, `Db::new`,
and the pool setup are unchanged by this PR, and the test passes in
GitHub CI. Live traces put the hang in the client, not the database: the
1 s acquire timeout stops firing after sqlx's `after_connect` rejection
path under tokio's `current_thread` runtime. No database lock waits were
observed.
- Two `observability_source` assertions were already failing on `main`,
unnoticed because no CI job ran that test binary: a stale
`fetch_optional` pin after #8005, and lease-seam attribution that now
lives in the `bounded_serving_lease_sql` helper. Both are corrected
here, and the binary now runs in `just test-unit`.
Generated with Codex
---------
Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
Co-authored-by: Codex <noreply@openai.com>
Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz>
|
||
|
|
91ab9d31b8 |
Add operation-aware database pool acquisition metrics (#7195)
## Why
Buzz already reports coarse writer/reader database checkout waits, but
those
signals cannot explain which startup or serving operation is blocked by
pool
pressure. That makes rollout diagnosis and postmortems ambiguous: a
readiness
probe, NIP-42 authentication, authorization check, reconnect history
repair,
event write, and background maintenance can all wait on the same pool
while
appearing identical.
This PR implements Package 2A of the pod-handoff plan: operation-aware
pool
borrow causality. It is observability-only; it does not change
configured pool
sizes, SQL semantics, transaction ordering, or timeout policy. Physical
DNS/TCP/TLS/Postgres authentication and session initialization remain
the
separate Package 2B boundary.
## Metric contract
The final contract separates three questions:
| Question | Metric |
|---|---|
| How long did checkout wait? |
`buzz_db_pool_acquire_duration_seconds{pool_role,operation}` |
| How did the attempt end? |
`buzz_db_pool_acquire_attempts_total{pool_role,operation,outcome}` |
| Who is waiting now for a tracked operation? |
`buzz_db_pool_waiters{pool_role,operation}` |
Outcomes are `success`, `timeout`, `error`, and `cancelled`. Operations
are
`bootstrap`, `readiness`, `tenant_resolution`, `authentication`,
`authorization`, `subscription_history`, `event_write`, and
`maintenance`.
Only these eleven pool/operation pairs are constructible:
```text
writer/bootstrap reader/bootstrap
writer/readiness
writer/tenant_resolution
writer/authentication
writer/authorization reader/authorization
writer/subscription_history reader/subscription_history
writer/event_write
writer/maintenance
```
The duration histogram intentionally does not carry `outcome`. Result
data
remains available on the terminal counter for historical counts and
rates,
without multiplying the expensive histogram family. Nine finite buckets
plus
`+Inf`, sum, and count produce 12 duration series per valid pair.
Together with
four outcome counters and one waiter gauge, the new-family ceiling is
exactly
187 raw Prometheus series per pod, asserted from the production
exporter.
The existing coarse acquisition families remain temporarily for
dashboard
compatibility while the new series are validated in staging.
The operation-specific waiter family covers the explicitly routed,
deployment-critical operations above; it is not a census of every
possible
SQLx checkout in the process. The dashboard pairs it with SQLx pool
active/idle/max gauges for whole-pool capacity context, and treats a
missing
operation series as unknown rather than healthy zero.
## What changed
### Cancellation-safe acquisition ownership
- Add writer- and reader-specific typed operation APIs so invalid label
pairs
cannot be constructed and store modules cannot emit reader labels.
- Own every polled acquisition with one RAII terminal guard.
- Record exactly one duration and terminal outcome for success, timeout,
error,
or cancellation.
- Emit nothing for a future that is created but never polled.
- Balance the operation-specific waiter count exactly once on every
terminal or
dropped future.
- Periodically refresh every expected waiter pair, including healthy
zero, so
missing telemetry is not presented as zero. Reader pairs are emitted
only
when a distinct read pool is configured; a writer-only pod cannot
fabricate
healthy reader-zero state.
### Production attribution
Route the deployment-critical acquisition paths through caller-owned
semantic
entry points, including:
- writer and reader bootstrap;
- the real post-#7149 readiness acquisition and deletion-catalog
validation;
- tenant resolution and community lifecycle checks;
- NIP-42 allowlist authentication;
- membership, moderation, invite, operator, Git, agent-owner, and policy
authorization;
- operator community create/list/archive/unarchive, reverse host/channel
tenant
resolution, and REQ row-community conformance lookups;
- writer/reader subscription history, feed, thread, and routed fallback
paths;
- primary and command event writes, replaceable events, mention
indexing,
reaction/channel/member/archive side effects, and thread metadata;
- push matching, usage rollups/leadership, replica-fence startup and
recurring
probes, periodic reconciliation, channel/deletion reapers, partitions,
and
other bounded maintenance/bootstrap paths.
Shared helpers now accept caller-owned intent or expose named semantic
variants
instead of assigning one misleading operation to every caller. No known
P0 path
uses `other`.
### Readiness and size-one-pool correctness
- Rebase on the post-#7149 readiness implementation and instrument the
actual
`Db::readiness_check` acquisition rather than the superseded ping-only
seam.
- Acquire once for deletion-catalog validation and run its queries on
that
connection, preserving the shared readiness deadline.
- Scope the channel-roster catalog checkout before the behavior probe so
a
size-one writer pool cannot self-deadlock during startup verification.
### Exporter, documentation, and CI
- Register metric HELP/type/unit metadata through the production
Prometheus
builder.
- Configure dedicated checkout buckets at 1ms, 5ms, 10ms, 25ms, 50ms,
150ms,
500ms, 1s, and 3s.
- Add a production scrape-contract test for exact names, labels,
buckets,
valid pairs, sensitive-label exclusion, and the 187-series ceiling.
- Add source mutation guards for the P0 semantic entry points and
raw-checkout
bypasses.
- Add an exact backend-integration CI selector for the production
attribution,
cancellation, readiness, and size-one-pool PostgreSQL tests.
- Document the frozen label vocabulary, valid combinations, semantics,
and
cardinality budget in the Helm chart README.
## Dashboard intent
The new Stage 2 row in **Buzz Startup & Rollout Safety** is
deployment-first:
- baseline-versus-candidate attempts, failure rates, cancellation rates,
and
maximum wait by operation;
- outcome counts and percentages over time by SHA/ReplicaSet;
- acquisition wait heatmap, average, and maximum through the rollout;
- historical waiter pressure beside writer active/idle/max context;
- per-pod postmortem drilldown, including terminated pods;
- a smaller current-waiter table with explicit stale/missing semantics.
Percentile widgets remain disabled until Datadog metadata confirms
percentile
support for the new distribution. Current gauges use no fill,
interpolation, or
`default_zero`; missing means unknown.
## Risk assessment
Moderate. The patch touches many database acquisition call sites, but
preserves
the selected physical pool and executes the same SQL on the acquired
connection. The main risks are incorrect semantic attribution,
cancellation
double-counting, and a helper accidentally acquiring twice. Typed APIs,
production-method PostgreSQL tests, source guards, the raw scrape
contract, and
the size-one-pool regression cover those risks.
No tenant, community, user, pubkey, event, channel, SQL, URL, pod,
version,
ReplicaSet, or request-controlled value is emitted as an application
metric
label. Deployment identity is supplied by infrastructure enrichment.
## Verification
- `cargo fmt --all -- --check` — passed.
- `cargo clippy -p buzz-db -p buzz-relay --all-targets --all-features --
-D warnings`
— passed.
- `cargo test -p buzz-db` — 122 passed, 0 failed, 263 ignored;
source-contract integration test: 3 passed, 0 failed.
- Focused relay compatibility, metric-contract, and readiness tests —
passed.
- `scripts/test-postgres-test-discovery.sh` — passed.
- Full `buzz-relay` package run from the identical tree reached 1,015
passes;
the six media-test failures all stopped in their shared local PostgreSQL
setup with `Sqlx(PoolTimedOut)` because Docker/PostgreSQL was
unavailable.
The same six failed in isolation, while every changed exact test passed.
- Exact implementation head:
`f92910b353086e9edf85918ca5f72190edbbe22f`.
- Exact multi-architecture staging image:
`dev-sha-f92910b353086e9edf85918ca5f72190edbbe22f-run-33607968668-1`
(`sha256:161712c8ed2e265a15df9b63e02248d5973481f875ff129d7d2ae78a09d487a2`).
- Focused staging GitOps PR:
<https://github.com/squareup/builderbot-platform-core-infrastructure/pull/299>
— merged after renderer, inventory, infrastructure test, Kargo, Semgrep,
and
Intersect gates passed; the source/generated-artifact diff was exactly
two
image lines.
- Exact GitHub head reports 47 terminal checks: 35 successful and 12
intentionally skipped. PostgreSQL, unit, lint, security, both server
cross-compiles, backend integration, relay E2E, desktop, mobile, image,
Helm, Semgrep, zizmor, and DCO gates are green.
- Datadog readback identifies two exact-image pods,
`buzz-d79c8d8f7-ckv2l` and `buzz-d79c8d8f7-qzqdp`, in ReplicaSet
`buzz-d79c8d8f7`; both report the full source SHA above.
- Both pods report all eleven allowed pool/operation waiter pairs at
current
zero, with no invalid pair. The acceptance window observed nonzero
success
receipts for readiness, tenant resolution, authorization, subscription
history, event write, and maintenance, and no timeout, error, or
cancelled
outcome. Maximum observed wait was about 101 ms for maintenance and 50
ms
for reader subscription history.
The main **Buzz Startup & Rollout Safety** dashboard now has a live
Stage 2
database row with eight widgets and nineteen fully scoped queries. Final
readback preserved all seven top-level groups, found zero under-scoped
Row 6
queries, and confirmed the tracked-operation waiter boundary in the
panel
descriptions.
Generated with Codex.
---------
Signed-off-by: Ravneet Arora <rarora@squareup.com>
|
||
|
|
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> |
||
|
|
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> |