Skip to content

fix: instrument all pool acquisition paths - #40

Open
torchello wants to merge 1 commit into
chmodas:trunkfrom
torchello:codex/upstream-pool-acquisition
Open

torchello wants to merge 1 commit into
chmodas:trunkfrom
torchello:codex/upstream-pool-acquisition

Conversation

@torchello

Copy link
Copy Markdown

Implicit pool queries and Pool::begin() currently bypass the wrapper's acquisition metrics by using the raw SQLx pool. Separately, dropping an in-flight acquire() leaves db.client.connection.pending_requests incremented. A workload using ordinary query(...).fetch_*(&pool) can therefore saturate the pool without reporting its waits/timeouts, while caller cancellation leaves a stale pending count.

Route plain, annotated, streaming, prepare/describe and transaction acquisition through the same instrumented Pool::acquire(). A private executor runs the query on the acquired raw connection, retaining the existing outer query instrumentation so spans and operation metrics are recorded once. An owned usage guard follows the connection into a transaction until commit, rollback or drop.

Acquisition accounting:

  • A Drop guard clears pending on success, errors and caller cancellation.
  • Completed attempts record wait time; caller cancellation does not count as a completed wait or a pool timeout. Only PoolTimedOut increments timeouts.
  • Pending/timeouts start at zero; explicit wait-time buckets cover submillisecond acquisition.
  • Documentation distinguishes inline accounting from polling/export intervals that can miss brief peaks.

Regression tests cover acquisition counts, absence of duplicate query spans, transaction lease duration, cancellation versus real pool timeouts across acquisition paths, and release after dropping a partially consumed stream.

Validation (Rust 1.97.1, SQLx 0.9):

  • cargo test --all-features --workspace — 569 passed, 1 existing ignored doc example; includes real PostgreSQL 15 and MySQL 8 containers.
  • cargo test --features "sqlite,runtime-async-std" --test pool_metrics — 15 passed.
  • cargo clippy --all-targets --all-features -- -D warnings and cargo fmt --all -- --check — passed.

Based directly on trunk; independent of the owned-transaction lifetime fix in #39.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant