Instrument database pool roles - #7356
Conversation
Expose one closed physical Postgres pool-role vocabulary — writer,
reader, audit, and search — used only at pool construction and the
utilization-metrics boundary. Emit a fixed-cardinality role-labelled
contract (buzz_db_pool_connections{pool_role,state} plus
buzz_db_pool_configured{pool_role}) so all four roles are always present
and unconfigured optional pools report zero rather than disappearing.
The existing writer buzz_db_pool_* and reader buzz_db_read_pool_* gauges
are preserved unchanged for dashboard compatibility. Audit and search
keep cheap PgPool clones purely as statistics handles; service
ownership, pool capacities, timeouts, and query routing are unchanged,
and no aggregate deployment connection budget is enforced.
Document the new families in the Helm chart operator README and correct
the stale ARCHITECTURE.md claim that search indexing is an asynchronous
pipeline step — search_tsv is a generated column maintained on insert.
Signed-off-by: tornquist <tornquist@squareup.com>
Co-authored-by: Codex <noreply@openai.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🔐 Codex Security Review
|
Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
Signed-off-by: tornquist <tornquist@squareup.com>
Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
kalvinnchau
left a comment
There was a problem hiding this comment.
🤖 Approved. I independently verified the fixed-cardinality role contract and legacy compatibility at exact head e1019fd10371e94dba91c6c744a24a6a926af220, including live relay scrapes with audit disabled and enabled.
Nit, nonblocking: consider separating size and max from the state dimension. idle and active are connection states, but size contains both and max is capacity. Consequently, an unqualified sum over buzz_db_pool_connections is not meaningful (size + idle + active + max double-counts current connections and adds capacity).
Prometheus's metric naming guidance says:
As a rule of thumb, either the
sum()or theavg()over all dimensions of a given metric should be meaningful (though not necessarily useful). If it is not meaningful, split the data up into multiple metrics. For example, having the capacity of various queues in one metric is good, while mixing the capacity of a queue with the current number of elements in the queue is not.
A follow-up could retain idle|active as states and expose role-labelled current size and capacity separately. This does not block the present change: values are accurate, labels are bounded, and there are no existing consumers of the new family in the reviewed repository paths.
Signed-off-by: tornquist <tornquist@squareup.com> Co-authored-by: Codex <noreply@openai.com>
|
Addressed the metric-semantics suggestion at
The feedback was submitted in an approval review body, not an inline review thread, so GitHub exposes no thread to resolve. This comment records the addressed SHA. Generated with Codex |
What
Name the relay's four Postgres pool roles—writer, reader, audit, and search—and export one fixed-cardinality utilization metric contract for all of them. Existing writer and reader gauges remain unchanged.
Why
Audit and search already use separate pools, but they are missing from pool pressure telemetry. Operators cannot see all process-level connection demand before setting a deployment-wide connection budget.
How
A closed
DbPoolRolevocabulary now labelsbuzz_db_pool_connections{pool_role,state}andbuzz_db_pool_configured{pool_role}. The relay retains cheap pool clones for statistics only; ownership, capacities, timeouts, query routing, and failure behavior do not change. The docs also replace the stale async search-index description with the generated-column behavior used today.This simplifies four ad hoc observability states into one bounded model without adding a pool manager or changing service boundaries.
Risk
Low to moderate. This changes relay metrics and pool construction plumbing, but not SQL execution or routing. Legacy metric names remain available for current dashboards.
Testing
3f5468a0463bdfc5879bcdd3878fa06b42ca84c2: all 14 Rust test lanes passed; desktop Tauri checks passed.cargo test --workspace --all-targetswith repository-declared Postgres services.cargo clippy --workspace --all-targets -- -D warnings.cargo fmt --all -- --check.Bigger picture
Aggregate deployment budget enforcement remains deferred. This PR exposes the per-role facts that a later deployment-policy change can consume.
Generated with Claude Code