pd: bound every wait on the PD path — a frozen peer must not hang the client

[eduralph]

Why: #516's txn_cleanup_2pc_locks hang, diagnosed end to end. In two CI runs (31042420081, 31043996146), the phase logs from #553 show every phase of the test completing in milliseconds; the only statement after the last phase is count_locks, which begins with client.current_timestamp() — a TSO request. The cluster-logs artifact shows the trigger: all three TiKVs logged monotonic time jumped back by ~65s simultaneously (a hypervisor-level pause of the runner VM), PD then measured a 2.6s scheduling gap on its 50ms TSO loop, lost its 1.67s election lease, resigned, and closed the TSO allocator — and recovered ~15s later, serving normally for the rest of the job, while the test stayed hung until nextest's 600s kill. Whatever holds the client after that point is client-side. And nothing about the trigger is CI-specific: any hypervisor pause or live migration, cgroup throttling under Kubernetes, or a middlebox silently expiring an idle long-lived connection produces the same frozen-open stream in production.

The defects. Nothing on the PD path bounded a wait against a peer that goes silent without terminating the connection:

1. The TSO receive loop waits on responses.next() unbounded — a request parked in pending_requests on a frozen-open stream hangs forever. (Termination is recoverable; silence is not.)
2. TSO stream creation (pd_client.tso(...)) is unbounded the same way.
3. The dial: a frozen peer's kernel still completes the TCP handshake, then the HTTP/2 preface never arrives and connect() hangs unbounded. Connection::connect even took a timeout parameter and ignored it (_timeout), and its get_members call carried no bound either.
4. Teardown alone cannot deliver failures: the cancelled gRPC request body owns the request channel inside the connection's task, which may not run again — releasing nothing — until the connection thaws (observed in reproduction: sends kept succeeding into a dead worker's channel).

What: follow client-go's shape — its TSO dispatcher arms a per-batch deadline with defaultPDTimeout = 3s and cancels the stream on expiry (pd/client/clients/tso/dispatcher.go), bounds stream creation via checkStreamTimeout (stream.go), and bounds dials — using the client's configured request timeout (Config::timeout, whose documented contract covers PD requests). Every bound is armed only while something is owed:

• The receive loop waits in whole timeout windows while batches are outstanding: a batch pending across one full silent window declares the stream stalled and the worker exits. A batch dispatched mid-window gets one full window, so teardown takes between one and two windows of genuine silence. Idle streams are never torn down.
• Stream creation is guarded by a watchdog with the same window rule, armed only while a batch is pending — PD (a gRPC-go server) sends response headers with its first response, so tso(..) legitimately waits while the client is idle; HTTP/2 lets request batches flow (and become pending) before response headers arrive, so a stalled creation is still detected.
• The worker's exit is signaled: get_timestamp selects on a watch channel the worker closes on exit — while enqueueing as well as while awaiting the response, since a dead worker's request channel can remain open with a full buffer and send would otherwise block forever.
• Dials are bounded, HTTP/2 handshake included: SecurityManager grows connect_with_timeout (the existing connect signature is preserved, now bounded by the default request timeout, so no API break), Connection::connect uses its hitherto-ignored timeout, and get_members is bounded like every other request in that path.

Stream termination is also observable now: a clean end stays info-level; stalls, creation timeouts, termination with batches outstanding, and stream status errors (previously swallowed) surface as warnings with the outstanding count — resolving the old TODO about indistinguishable terminations.

Considered and omitted: a timer-based backstop in get_timestamp (client-go has no analogue — ambient contexts cover it — and the worker-exit signal is event-driven and precise), and gRPC keepalive (connection-level health; client-go applies it to TiKV connections; a natural follow-up).

What this means for #516: the unbounded hang is gone. Under a short freeze the test recovers and passes; under a freeze longer than the retry budget it fails fast with a descriptive error instead of a 600s timeout — the trigger itself is outside any client's control.

Verification: 10 new unit tests — the receive loop and the creation watchdog driven under a paused clock (stall detected within [1, 2] windows; a mid-window batch granted its full window; idle streams and idle creation never torn down; termination clean only when nothing is owed; a stream status error surfaced; the happy path), get_timestamp failing fast when the worker dies, and enqueueing observing worker death with a full request buffer. The stall test bites: with the stall branch disabled it runs unbounded. 82 lib tests, make check, and the txn/raw/failpoint integration suites green against a local api-v2 cluster.

Manually, with docker pause on PD for 45s (SIGSTOP: TCP open, no FIN — the client-side view of a VM pause): before, one silent 44.5s hang ended only by the unpause; after, a 5s-idle client keeps its worker silently, the stall is detected in ~4s, reconnects against the frozen peer fail in 2s each, callers receive bounded, descriptive errors while the freeze lasts, and the client recovers within ~3s of the peer thawing.

Ref: #516

Summary by CodeRabbit

• Bug Fixes
  • Added timeout handling for cluster connections, security handshakes, and initial membership requests.
  • Timestamp requests now honor configurable time limits and fail promptly when services stall or become unavailable.
  • Improved handling of timestamp stream errors, idle termination, and worker shutdowns to prevent requests from remaining unresolved.
  • Reconnection attempts now apply the configured timeout consistently.
• Tests
  • Expanded coverage for connection timeouts, timestamp allocation, stream failures, and worker shutdown behavior.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tonyxuqqi for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 39d169f2-6fa4-4b51-a731-6f4f9662ed11

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf2a1b and b853f49.

📒 Files selected for processing (1)

• src/common/security.rs

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change applies timeouts to security and cluster connections, membership requests, and timestamp operations. TimestampOracle now monitors worker termination and handles stalled or terminated streams. Tests cover timeout and shutdown behavior.

Changes

Timeout handling

Layer / File(s)	Summary
Connection deadline propagation src/config.rs, src/common/security.rs, src/pd/cluster.rs	Connection paths apply configured timeouts to channel setup, membership requests, and TimestampOracle construction.
Timestamp worker timeout behavior src/pd/timestamp.rs	TimestampOracle monitors worker exit and bounds stream creation, response processing, and timestamp requests.
Timeout behavior validation src/pd/timestamp.rs, Cargo.toml	Tests cover stalled streams, idle behavior, stream errors, worker shutdown, and timestamp allocation. Tokio enables test-util for the tests.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to b853f

Connection and timestamp requests now fail within configured deadlines rather than waiting indefinitely on unavailable PD peers, with idle streams preserved and timeout behavior covered by tests.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant TimestampOracle
  participant TimestampWorker
  participant PD_TSO_Stream
  Client->>TimestampOracle: request timestamp
  TimestampOracle->>TimestampWorker: enqueue request
  TimestampWorker->>PD_TSO_Stream: create stream and send request
  PD_TSO_Stream-->>TimestampWorker: return response or stream error
  TimestampWorker-->>TimestampOracle: return result or worker-exit error
  TimestampOracle-->>Client: return timestamp or error

Loading

Suggested reviewers: coocood

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: adding bounds to waits across the PD client path to prevent frozen peers from hanging clients.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

• Fix all pre-merge checks with AI

Warning

Some tools did not complete. Review the errors below.

🔧 Clippy (1.97.1)

Clippy execution timed out

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[eduralph]

@pingyu for attention