pd-client: evaluate TSO retry during primary failover
- Dominant language
- Go
- Stars
- 1.2k
- Forks
- 783
- Avg merge
- 5d 21h
- Merged PRs (30d)
- 36
Description
## Enhancement Task
### Background
During a TiDB Cloud Essential cluster failover in TSO microservice mode, the active TSO primary was terminated by a planned configuration change. The standby became ready shortly afterwards, but TiDB clients that still had an RPC stream to the old primary observed:
```text
rpc error: code = Unknown desc = server not started
```
The observed sequence was:
- The old TSO stream failed while requests were in flight.
- The client logged `getTS error after processing requests` and rebuilt the TSO stream.
- The new primary became ready before most clients refreshed their service-discovery URLs.
- The affected pessimistic transaction statements returned an error during `forUpdateTS` acquisition.
The relevant TiDB call path was:
```text
getForUpdateTs -> GetTimestampAsync -> tsFuture.Wait
```
The asynchronous Future returned the TSO error directly. The transaction statement was not replayed by the SQL execution layer.
### Problem
The current behavior appears to retry TSO request dispatch in limited cases, such as an unavailable dispatcher, but it does not clearly define whether an already-dispatched request should be retried after its stream fails during TSO primary failover.
The existing synchronous `getTimestampWithRetry` path is not used by the `forUpdateTS` path above. As a result, a short and otherwise recoverable TSO failover can surface as a user-visible transaction error.
This issue is intended to evaluate the behavior and ownership of the retry. It does not assume that the fix belongs entirely in PD or entirely in TiDB.
### Evaluation Scope
1. **pd-client behavior**
- Determine whether errors such as `server not started`, EOF, and transport-unavailable errors should be classified as retryable during TSO primary failover.
- Evaluate whether an in-flight TSO batch can be safely re-submitted after service discovery and stream reconstruction, including timestamp allocation, monotonicity, duplicate allocation, request deadlines, and cancellation.
- Evaluate whether member/service discovery should be refreshed synchronously for this error class instead of waiting for the asynchronous update interval.
2. **TiDB/client integration**
- Determine whether the asynchronous `GetTimestampAsync` or TiDB's `getForUpdateTs` path should perform a bounded retry.
- Define one retry owner and avoid unbounded or duplicated retries across pd-client, client-go, and TiDB.
- Add integration coverage for pessimistic `SELECT ... FOR UPDATE` and DML while the TSO primary is terminated and a standby becomes ready.
### Expected Outcome
- A documented decision on the retry owner and the supported behavior during TSO primary failover.
- Tests covering in-flight requests, stream reconstruction, service-discovery refresh, context cancellation, and bounded latency.
- If a code change is appropriate, an implementation plan covering pd-client and the required TiDB/client-go coordination.
- Metrics or logs that make retry attempts and the final outcome observable.
Retrying a complete application transaction remains outside the scope of this issue. The goal here is to determine whether the infrastructure/client path can absorb a transient TSO failover without returning an avoidable error.
### References
- [`dispatchTSORequestWithRetry`](https://github.com/tikv/pd/blob/master/client/inner_client.go)
- [`tsoDispatcher.handleProcessRequestError`](https://github.com/tikv/pd/blob/master/client/clients/tso/dispatcher.go)
- [`tsoDispatcher.cancelCollectedRequests`](https://github.com/tikv/pd/blob/master/client/clients/tso/dispatcher.go)
- [`PessimisticRRTxnContextProvider.getForUpdateTs`](https://github.com/pingcap/tidb/blob/v8.5.3/pkg/sessiontxn/isolation/repeatable_read.go#L72-L102)
Contributor guide
Research direction
Start with client/inner_client.go at dispatchTSORequestWithRetry, then read client/clients/tso/dispatcher.go, especially handleProcessRequestError and cancelCollectedRequests. Trace the referenced TiDB getForUpdateTs path and its GetTimestampAsync call to compare retry ownership. Done means a documented decision, bounded retry and cancellation behavior, integration coverage, and observable retry outcomes; any implementation plan should include pd-client and TiDB/client-go coordination.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- distributed-systems
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100