fix(buzz-waker): install the rustls CryptoProvider before opening feeds - #33
Merged
Merged
Conversation
buzz-waker was the only binary in this workspace that never installed a
process-level rustls CryptoProvider. buzz-relay, buzz-cli, buzz-admin,
buzz-dev-mcp, buzz-backend-sprites and buzz-backend-kubernetes all call
install_default() at startup for exactly this reason.
Without it the daemon starts, validates its watch list, logs "watching
agent" for each one, and then every watch task panics on its first wss://
connection:
Could not automatically determine the process-level CryptoProvider
from Rustls crate features.
main() returns Err, the process exits 1, and Fly restarts it — a crash loop
that reports a clean startup right up to the panic. ring and aws-lc-rs both
arrive transitively, so rustls cannot auto-select between them.
This stayed invisible in local development because a full-workspace cargo
build unifies features across crates. Dockerfile.waker builds only
buzz-waker and buzz-backend-sprites, so the ambiguity is real there, and it
reproduced on every boot of the Fly app fly.waker.toml describes.
The result is ignored rather than unwrapped: the provider is process-level,
a second install is a documented no-op, and the repo forbids new expect() in
production paths. This matches the majority pattern in the workspace.
Verified: cargo build, cargo fmt --check and cargo clippy --all-targets are
clean for -p buzz-waker; the rebuilt image boots on Fly, authenticates to
the relay and subscribes to channels with no panic.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Junchao Yan <yjc801@gmail.com>
yjc801
pushed a commit
that referenced
this pull request
Aug 26, 2026
## 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 block#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 block#6229 block#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 block#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>
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
buzz-wakerwas the only binary in this workspace that never installed a process-level rustlsCryptoProvider. Without it the daemon starts, validates its watch list, logsbuzz-waker: watching agentfor each configured agent — and then every watch task panics on its firstwss://connection:main()returnsErr, the process exits 1, and Fly restarts it. The result is an endless crash loop that reports a clean, healthy startup right up to the moment it dies. Bothringandaws-lc-rsarrive transitively, so rustls cannot auto-select between them.Why this was invisible until deployment: a full-workspace
cargo buildunifies features across every crate, which papers over the ambiguity locally.Dockerfile.wakerbuilds onlybuzz-wakerandbuzz-backend-sprites, so there the ambiguity is real. It reproduced on every boot of the Fly app thatfly.waker.tomldescribes (added in #26), which is how it surfaced.The fix mirrors what every other binary in this repo already does —
buzz-relay,buzz-cli,buzz-admin,buzz-dev-mcp,buzz-backend-sprites, andbuzz-backend-kubernetesall install the provider at startup for exactly this reason.The result is ignored rather than unwrapped: the provider is process-level, a second install is a documented no-op, and
AGENTS.mdforbids newexpect()in production paths. That matches the majority pattern in the workspace (5 of the 6 call sites above).Cargo.lockis included becauseDockerfile.wakerbuilds with--locked; without the lock entry the image build fails outright.Related issue
None found for
buzz-waker. This is the same class of bug upstream has fixed repeatedly for other binaries — block#2271, block#2322, block#2455, block#4153, block#5341 — none of which coveredbuzz-waker, since the crate is fork-only.Given it has now recurred six times across the workspace, a lint or a shared startup helper is probably the real fix; that felt out of scope for a one-line crash fix, but I'm happy to follow up if a reviewer wants it here.
Testing
Local, all clean for
-p buzz-waker:cargo build --bin buzz-wakercargo fmt -- --checkcargo clippy --all-targetsOn Fly (app
buzz-waker,fly.waker.toml+Dockerfile.waker, relaywss://openvelvet.communities.buzz.xyz):watching agentlogged, then four watch tasks panicked,Main child exited normally with code: 1, reboot, repeat.No UI change, so no screenshots.
Reviewer notes — two findings not addressed here
Both were found while verifying this fix on the live deployment. Neither is caused by this change, and both are out of scope for it, but they matter to anyone actually running the daemon.
1. Tight re-subscribe loop under relay rate limiting (real bug, unfixed). Once the panic is gone and the daemon reaches the relay, it subscribes to ~20 channels, gets rate-limited, and spins. The relay closes the subscription with
rate-limited: quota exceeded; retry in 0s;wake_loop.rsretries immediately by design ("Retry the one subscription immediately"), with no backoff and without reading the relay'sretry in Nshint. Measured 100 re-subscribes in 25 seconds, sustained and not decaying — unbounded load on a shared relay.I stopped the Fly machine rather than leave it hammering the relay, so the deployed app is currently
stoppedon purpose. Fixing it means honoring the retry hint with a sensible floor and distinguishing rate-limit closes from ordinary ones — a behavioral change to the reconnect ladder that deserves its own PR and its own review.2.
fly.waker.tomlcomment references a key that isn't there.fly.waker.toml:28states "min_machines_running below is what actually keeps it running", but no such key exists in the file. The config's behavior is still correct — with no[http_service]block Fly's autostop path never engages, and[[restart]] policy = "always"covers crash restarts — andmin_machines_runningis only valid inside an[http_service]block, which this app deliberately omits. So the comment is what's wrong, not the config. Left alone to keep this diff to the crash fix.🤖 Generated with Claude Code