Skip to content

fix(postgres): bound metadata query timeouts, derive budgets from query timeout, and keep cancel inside the budget - #6292

Open
Abeautifulsnow wants to merge 11 commits into
t8y2:mainfrom
Abeautifulsnow:fix/postgres-metadata-timeout-5897
Open

fix(postgres): bound metadata query timeouts, derive budgets from query timeout, and keep cancel inside the budget#6292
Abeautifulsnow wants to merge 11 commits into
t8y2:mainfrom
Abeautifulsnow:fix/postgres-metadata-timeout-5897

Conversation

@Abeautifulsnow

Copy link
Copy Markdown
Contributor

变更说明

Summary

Summary

Fixes #5897. Native PostgreSQL metadata introspection (list databases, list tables,
column/index/object listing, completion search) could hang indefinitely once a
client was checked out of the deadpool pool. A half-open connection — a cloud
load balancer silently evicting an idle connection, or a proxy stalling a
lazily-created second connection — blocked until TCP keepalive reclaimed the
socket and surfaced only a generic db error after ~30s.

This PR bounds every metadata query with a real budget, threads the per-connection
query timeout through the whole metadata path, and keeps the server-side cancel
inside the same budget the frontend deadline is aligned to.

What changed

1. Bound metadata query timeouts (b2431e549)

  • postgres_query_cached / postgres_query_one_cached wrap their bodies in
    tokio::time::timeout, with a budget derived from the pool wait/create/recycle
    timeouts (5s fallback). Timeouts return the distinctive PostgreSQL metadata query timed out error and log the elapsed budget.
  • Non-timeout metadata errors route through pg_metadata_error_to_string so
    SQLSTATE survives to the UI instead of tokio-postgres's literal db error.
  • Bounded the direct query_typed in list_tables_filtered and post-checkout
    identity resolution (resolve_postgres_client_key).
  • metadata_recovery maps the timeout prefix to Discard (evict the pool — no
    blind reconnect+re-run); is_retryable_metadata_error excludes it from the
    connection-error retry path.
  • Logs elapsed TLS/startup and pool-build phases so a TLS-stall is diagnosable.
  • Frontend: withMetadataLoadTimeout keeps the late-arriving real backend error;
    the ConnectionDialog visible-databases picker races a timeout, aborts the
    in-flight HTTP request, and guards late rejections with a run id + mounted flag.

2. Derive the metadata query budget from the query timeout (214d9089a)

  • Budget now floors at max(connect_timeout, 60s) via
    POSTGRES_METADATA_QUERY_BUDGET_FALLBACK, so slow-but-valid cloud metadata
    queries (11–60s) are no longer killed by the old 10s default.
  • query_timeout_secs (0 → 60s bounded fallback) is threaded through all native
    PG metadata functions and feature probes; on timeout, a real CancelToken
    cancel is sent server-side when a TLS cancel context is available.
  • Regression test proves connect_timeout < query_timeout yields a 60s budget
    (10s connect pool / connect=10 query=60 config).

3. Keep the cancel inside the budget and honor timeout inheritance (f5a288cfa)

  • P1: share metadataLoadTimeoutMs in lib/sql/queryTimeout so the
    visible-databases picker and connection store resolve the effective query
    timeout (inheritance + global) and treat 0 as the disabled window instead of
    capping at the 35s default. Fixes both callers' latent inheritance gaps.
  • P2: postgres_metadata_timeout_error always sends a best-effort server cancel;
    sslmode=disable (no PostgresCancelContext) falls through to the existing
    NoTls cancel path instead of being abandoned locally.
  • P3: POSTGRES_METADATA_CANCEL_ALLOWANCE (2s) is drawn from the metadata budget
    (postgres_metadata_budget_split), so backend total (query + cancel) stays
    within the budget the frontend deadline is aligned to; frontend disabled window
    bumped 60s → 65s. A fake PG-wire-protocol regression proves a stalled query AND
    a stalled cancel still surface the diagnostic within budget.

Tests

  • Unit tests in dbx-core for budget derivation, timeout/cancel behavior
    (including a fake PG-wire-protocol harness), and SQLSTATE diagnostics.
  • Frontend specs for the visible-databases picker timeout race and
    withMetadataLoadTimeout late-error retention.
  • cargo check passes for dbx-core, dbx-web, dbx-mcp, and the dbx
    Tauri crate; oxfmt/lint-staged formatting applied.

Notes

  • Branch was synced with origin/main before opening (merge commit 4b1b73f01).
  • No breaking changes; no package release included.

变更类型

  • 新功能
  • Bug 修复
  • 性能优化
  • 代码重构
  • 文档更新
  • CI / 构建

涉及前端

  • 本 PR 涉及前端改动,已附截图/录屏(见下方)

验证

  • make check 通过
  • make cargo-check-fast 通过
  • 相关测试通过

关联 Issue

Related #5897

…tics

Metadata introspection queries (list_databases, list_tables, column/index/object
listing, completion search) over the deadpool pool ran unbounded once a
client was checked out: a half-open connection (cloud LB silently evicting
an idle connection, or a proxy stalling a lazily-created second connection)
made them hang until TCP keepalive reclaimed the socket, surfacing only a
generic 'db error' after ~30s (issue t8y2#5897).

- postgres_query_cached/postgres_query_one_cached now wrap their bodies in
  tokio::time::timeout with a budget derived from the pool wait/create/
  recycle timeouts (fallback 5s); timeouts return the distinctive
  'PostgreSQL metadata query timed out' error and log elapsed budget.
- Non-timeout metadata errors route through pg_metadata_error_to_string so
  SQLSTATE survives to the UI instead of tokio-postgres's literal 'db error'.
- Bound the direct query_typed in list_tables_filtered and the post-checkout
  identity resolution (resolve_postgres_client_key) in checkout_postgres_client.
- schema metadata_recovery maps the timeout prefix to Discard (evict the
  pool, do not blind reconnect+re-run), and is_retryable_metadata_error
  excludes it from the connection-error retry path.
- Log TLS/startup phase elapsed in NoticeCapturingConnect and the pool-build
  phase in connect_with_optional_local_timezone so a TLS-stall is diagnosable.
- Frontend: withMetadataLoadTimeout keeps the late-arriving real backend
  error (mirrors withConnectionAttemptTimeout); the ConnectionDialog visible-
  databases picker is bounded by a timeout race, aborts the in-flight HTTP
  request on timeout/close/unmount, and guards late rejections with a run id
  + mounted flag; http.ts get/post accept an optional AbortSignal.
Metadata query budget now floors at max(connect_timeout, 60s) via
POSTGRES_METADATA_QUERY_BUDGET_FALLBACK instead of the pool connect-derived
10s default, so slow-but-valid cloud metadata queries (11-60s) are no longer
killed. The schema metadata path threads the per-connection
query_timeout_secs (0 -> 60s bounded fallback) through all native PG metadata
functions and feature probes, and on timeout sends a real CancelToken cancel
server-side when a TLS cancel context is available.

Regression test proves connect_timeout < query_timeout yields a 60s budget
(10s connect pool / connect=10 query=60 config), not the old 10s cap.

Follow-up to t8y2#5897 (b2431e5).
… timeout inheritance

- P1: share metadataLoadTimeoutMs in lib/sql/queryTimeout so the
  visible-databases picker and connection store resolve the effective query
  timeout (inheritance + global) and treat 0 as the disabled window instead of
  capping it at the 35s default bound. The dialog's stale 0->35s divergence and
  both callers' latent inheritance gaps are fixed by construction.
- P2: postgres_metadata_timeout_error always sends a best-effort server cancel;
  sslmode=disable (no PostgresCancelContext) now falls through to the existing
  NoTls cancel path instead of being abandoned locally.
- P3: draw POSTGRES_METADATA_CANCEL_ALLOWANCE (2s) from the metadata budget
  (postgres_metadata_budget_split) so the backend total (query + cancel) stays
  within the budget the frontend deadline is aligned to; frontend disabled
  window bumped 60s -> 65s (60s backend budget + connect round-trip). A fake
  PG-wire-protocol regression proves a stalled query AND a stalled cancel still
  surface the PostgreSQL diagnostic within the budget.

Refs t8y2#5897
…-timeout-5897

# Conflicts:
#	crates/dbx-core/src/db/postgres.rs
#	crates/dbx-core/src/schema.rs
@github-actions github-actions Bot added area/core Shared DBX core runtime area/desktop Desktop application or Tauri shell bug Something isn't working db/postgres Database: PostgreSQL ui-change Changes user-visible interface, text, or visual assets labels Aug 15, 2026

@t8y2 t8y2 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not ready to approve exact head 954067ecc3429875ebeda19b2f50565decd99075.

Please first rebase onto current main; GitHub reports CONFLICTING, and a local merge simulation produced 20 conflict hunks across queryTimeout.ts, its spec, postgres.rs, and schema.rs. This is not a mechanical small conflict because current main changed the same PostgreSQL metadata paths.

Three blockers remain:

  1. The frontend deadline is not aligned with the complete backend operation. metadataLoadTimeoutMs allows only query_timeout + 5s, while connect, checkout and identity work happen before the per-query timer starts. Use one end-to-end deadline and pass the remaining budget through connect/checkout, identity, query and cancel so the generic frontend timeout cannot hide the PostgreSQL diagnostic.
  2. postgres_metadata_budget_split gives a fixed 2s cancel allowance priority over the query. Valid 1s and 2s settings therefore produce a zero query window, and larger settings are shortened by 2s. PostgreSQL defines statement_timeout around statement execution, while cancellation is a separate protocol request; preserve a non-zero configured query window and budget cancellation separately. See the official timeout documentation and protocol flow: https://www.postgresql.org/docs/17/runtime-config-client.html and https://www.postgresql.org/docs/17/protocol-flow.html#PROTOCOL-FLOW-CANCELING-REQUESTS
  3. The exact head is not buildable. live_opentenbase still calls list_tables without its two new arguments; exact-head CI also has two Clippy failures, and the Web backend spec expects the old fetch call shape. Update all callers/tests and make the exact head green.

The cancellation harness also does not prove a stalled cancel connection/TLS/write path. PostgreSQL documents that the server does not reply to a CancelRequest; add a regression that stalls cancel establishment or writing if that timeout path is intended to be covered.

…-timeout-5897

# Conflicts:
#	apps/desktop/src/lib/__tests__/sql/queryTimeout.spec.ts
#	apps/desktop/src/lib/sql/queryTimeout.ts
#	crates/dbx-core/src/db/postgres.rs
#	crates/dbx-core/src/schema.rs
#	crates/dbx-core/src/transfer.rs
…ne, cover stalled TLS cancel

Addresses the review blockers for t8y2#5897:

- postgres: drop the fixed 2s cancel allowance from the metadata query
  budget. A CancelRequest is a separate protocol request the server never
  answers, so statement_timeout semantics apply to the query window only;
  valid 1s/2s settings no longer collapse to a zero query window, and larger
  settings are no longer shortened. Cancellation now runs in its own 2s
  allowance on top of the query budget.
- postgres: add a regression proving a stalled cancel connection/TLS/write
  path does not extend the operation beyond the query budget plus the
  cancel allowance (main connection uses SslMode::Prefer so the cancel
  negotiates TLS and stalls client-side).
- desktop: align the metadata-load frontend deadline with the complete
  backend operation (connect x3 + query + cancel + transport buffer) so the
  backend's PostgreSQL diagnostic always surfaces before the generic
  frontend timeout, honoring connect/query inheritance and the 60s fallback
  budget for unlimited query timeouts.
- desktop: restore the single-argument fetch shape when no abort signal is
  passed.
…dapters

Confirms the fetch abort signal is forwarded as the second argument when
present (Blocker 3 fetch-shape restoration), while the no-signal path
keeps the classic single-argument call.

@t8y2 t8y2 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

request changes: the latest head fixes several earlier build and cancellation issues, but two timeout-boundary problems and the current semantic conflicts still need author-side resolution.

  1. crates/dbx-core/src/schema.rs:5916 retries the complete metadata operation after reconnecting, but the retry wrapper does not receive or decrement an end-to-end deadline. apps/desktop/src/lib/sql/queryTimeout.ts:73 only estimates one connect/checkout/identity/query/cancel sequence. A failed first attempt followed by reconnect and retry can therefore exceed the frontend estimate and surface the generic frontend abort instead of the PostgreSQL diagnostic. Please create one deadline before recovery starts and pass the remaining budget through every attempt, reconnect, query, and cancel path.

  2. crates/dbx-core/src/schema.rs:180 still floors every finite Agent metadata timeout to 60 seconds with seconds.max(60), and several Agent/plugin metadata paths do not carry the native PostgreSQL cancel context. A connection configured below 60 seconds can therefore continue for a full minute on those paths. Please preserve the configured finite timeout and make the cancellation/deadline behavior explicit for every PostgreSQL metadata transport covered by this PR.

The branch also remains CONFLICTING with current main; the conflicts are in the same postgres.rs and schema.rs timeout/cancellation paths, so they are semantic rather than a safe mechanical maintainer resolution. Please rebase, preserve the current-main metadata behavior, and rerun the Rust and frontend timeout suites on the resolved head.

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

Labels

area/core Shared DBX core runtime area/desktop Desktop application or Tauri shell bug Something isn't working db/postgres Database: PostgreSQL ui-change Changes user-visible interface, text, or visual assets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] 飞书秒搭应用开发 PostgreSQL 的连接异常问题

2 participants