*: ensure QueryRegion parity with unary region requests
- Dominant language
- Go
- Stars
- 1.2k
- Forks
- 783
- Avg merge
- 5d 21h
- Merged PRs (30d)
- 36
Description
## Development Task
### Background
#8690 introduced the QueryRegion bidirectional stream and client-side batching as a replacement for the unary `GetRegion`, `GetPrevRegion`, and `GetRegionByID` request paths. The router client has since been enabled by default.
The normal leader-side happy path is largely equivalent today: QueryRegion returns the same Region metadata, deep-copies shared responses, and preserves per-request `NeedBuckets` semantics. However, an audit against master commit `9eba2bc52362fa433ac19a98e1e3cab467de4332` found that the two paths are not yet behaviorally equivalent across compatibility, failure handling, rate limiting, context propagation, metrics, and tracing.
The goal of this task is to make enabling QueryRegion transparent to users: existing callers, dashboards, alerts, tracing workflows, failure handling, and operational controls should continue to behave as they did with the unary Region APIs, except for intentional and documented batching-related improvements.
### Resolved gaps
- `GetRegionByID(0)` now uses the request selector fields as a total encoding and returns no Region without panicking when QueryRegion is enabled. Fixed by #11181.
### Current gaps
#### API correctness and compatibility
- A new client does not fall back to unary RPCs when an old PD server returns `Unimplemented` for QueryRegion. This is already tracked by #11138 and should be treated as part of the parity requirement.
- QueryRegion batches are limited by request count, up to 10,000 entries, but not by serialized byte size. A large batch, especially one containing many unique Regions or Buckets, can exceed transport message limits even when the equivalent unary calls would succeed individually.
#### Routing and failure handling
- The unary path supports PD request forwarding, while the router client still has an explicit forwarding TODO:
https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/clients/router/client.go#L540-L567
- The PD QueryRegion handler also does not implement the unary forwarding middleware path.
- When a follower cannot find a requested Region, unary Region APIs return `REGION_NOT_FOUND`, allowing the client to retry against the leader. QueryRegion instead returns a successful batch response containing an ID of zero or a nil Region entry once RegionSyncer is running. This can turn an incomplete follower view into a successful `nil, nil` result.
- Unary semantics: https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/pkg/utils/grpcutil/cluster.go#L34-L113
- QueryRegion handler: https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/server/grpc_service.go#L1605-L1665
- Batched result encoding: https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/pkg/core/region.go#L1607-L1743
- Fix in progress: #11182
- The caller's context controls `Request.wait`, but not the persistent stream's Send/Recv operations. The dispatcher has no processing timeout after selecting a stream.
- A canceled request remains queued or collected and may still be sent to PD.
- If Send/Recv stalls, the single dispatcher can stop draining the request channel.
- Enqueue operations do not select on the request or client context and can block after the channel fills.
- Relevant code: https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/clients/router/request.go#L59-L114
- Dispatcher TODO: https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/clients/router/client.go#L569-L648
#### Rate limiting
The three unary handlers invoke `rateLimitCheck` for every logical request and use the corresponding method name as the limiter label.
QueryRegion invokes it only once when the long-lived stream handler starts:
https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/server/grpc_service.go#L1513-L1665
As a result:
- existing `GetRegion`, `GetPrevRegion`, and `GetRegionByID` QPS limits do not apply to QueryRegion traffic;
- a QueryRegion QPS limit controls stream creation rather than logical request throughput;
- a QueryRegion concurrency token remains held until the persistent stream closes;
- all subsequent batches on an accepted stream bypass per-request rate checks.
#### Request attribution
Unary requests populate `ClusterId`, `CallerId`, and `CallerComponent`:
https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/client.go#L1329-L1335
QueryRegion currently populates only `ClusterId`:
https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/clients/router/client.go#L753-L760
Batches can also contain requests from client wrappers with different caller components. The current protocol and batching key do not preserve that per-logical-request attribution.
#### Client metrics
The outer command duration metrics remain populated because they are recorded before selecting the router client:
- `pd_client_cmd_handle_cmds_duration_seconds{type="get_region"}`
- `pd_client_cmd_handle_cmds_duration_seconds{type="get_prev_region"}`
- `pd_client_cmd_handle_cmds_duration_seconds{type="get_region_byid"}`
The corresponding failure metrics are not preserved. Once the router client is selected, the public methods return directly and no longer execute `respForErr`, so failures move to QueryRegion-specific labels or are omitted for context cancellation:
- `pd_client_cmd_handle_failed_cmds_duration_seconds{type="get_region"}`
- `pd_client_cmd_handle_failed_cmds_duration_seconds{type="get_prev_region"}`
- `pd_client_cmd_handle_failed_cmds_duration_seconds{type="get_region_byid"}`
QueryRegion request metrics also record `RequestDurationQueryRegion` before checking the response header. Header errors such as `NOT_BOOTSTRAPPED` are therefore counted in the successful request-duration series rather than the failed series:
https://github.com/tikv/pd/blob/9eba2bc52362fa433ac19a98e1e3cab467de4332/client/clients/router/client.go#L775-L807
#### Server metrics
Unary `pd_server_region_request_cnt` observations preserve:
- the logical method: `GetRegion`, `GetPrevRegion`, or `GetRegionByID`;
- `caller_id`;
- `caller_component`;
- success or failure event.
With QueryRegion:
- all three methods collapse into `request="QueryRegion"`;
- caller labels become `unknown`;
- `RequestCounter` is called once per batch rather than once per logical request;
- successful 1% sampling is applied to batches, so logical request throughput is undercounted by approximately the average batch size;
- early `NOT_BOOTSTRAPPED` and follower-not-ready responses bypass the custom QueryRegion request counter.
The new `pd_core_query_region_count{type="keys|prev-keys|ids"}` metric records logical input volume, but it does not preserve the legacy caller, method, event, or error dimensions and is not a one-to-one replacement.
#### Generic gRPC metrics
Unary RPC instrumentation records one started/handled/duration observation per logical request for:
- `grpc_server_started_total`
- `grpc_server_handled_total`
- `grpc_server_handling_seconds`
For the long-lived QueryRegion stream:
- started is incremented once when the stream is established;
- handled and handling duration are emitted only when the stream closes;
- handling duration measures connection lifetime rather than request latency;
- stream message counters represent batches rather than logical Region requests.
Existing dashboards and alerts grouped by `grpc_method=GetRegion`, `GetPrevRegion`, or `GetRegionByID` therefore lose their original QPS, error-rate, and latency semantics.
#### Tracing
- The public `pdclient.GetRegion`, `pdclient.GetPrevRegion`, and `pdclient.GetRegionByID` OpenTracing spans remain present.
- The router client creates a per-request `pdclient.processRegionRequests` span, but the public API span is not written back into the request context. The process span is therefore a sibling of the public API span rather than its child.
- The persistent stream uses a client-lifetime context and the QueryRegion message does not contain a per-item trace carrier, so individual logical requests cannot be correlated across the stream to server-side processing.
- Go runtime tracing intentionally uses one batch-level `pdclient.regionReqSendBatch` region, but `pdclient.regionReqDone` is currently recorded by both the response finisher and the waiter for a successful request.
### Expected behavior
For every public `GetRegion`, `GetPrevRegion`, and `GetRegionByID` call, enabling QueryRegion should preserve:
1. the returned Region data and missing-Region semantics;
2. request deadlines, cancellation, forwarding, retry, fallback, and mixed-version compatibility;
3. follower safety and leader fallback behavior;
4. per-logical-request rate limiting;
5. caller identity and component attribution;
6. existing success/failure, QPS, and latency observability;
7. logical trace identity and parent-child relationships;
8. bounded request size and failure isolation, so one batch does not make otherwise valid unary requests fail together unnecessarily.
Additional batch-level metrics and traces are welcome, but they should complement rather than silently replace the existing logical request view.
### Completion criteria
- [x] Preserve the full `uint64` ID range, including `GetRegionByID(0)`, without adding a separate request-kind field. Fixed by #11181.
- [ ] Complete #11138 or otherwise provide capability negotiation and unary fallback for servers without QueryRegion.
- [ ] Support forwarding for QueryRegion or safely fall back to unary requests when the leader cannot be reached directly.
- [ ] Preserve unary follower missing-Region behavior, including selective leader retry. Tracked by #11182.
- [ ] Make enqueue, batching, Send, and Recv context-aware; do not dispatch requests that have already been canceled.
- [ ] Add a bounded batch processing timeout and recover the dispatcher after a stalled stream.
- [ ] Bound batches by serialized size as well as request count.
- [ ] Apply rate limiting per QueryRegion batch/logical request with semantics compatible with the three unary method limits.
- [ ] Preserve `CallerId`, `CallerComponent`, and logical method for every request, splitting batches where required.
- [ ] Preserve the existing client failed-command metric series and classify QueryRegion header errors as failures.
- [ ] Provide per-logical-request server metrics that retain the existing method, caller, event, QPS, error-rate, and latency semantics.
- [ ] Update dashboards and alerts so switching transports does not silently change their meaning.
- [ ] Correct the OpenTracing parent-child relationship and define how logical request spans correlate with batch and stream spans.
- [ ] Remove duplicate or ambiguous runtime trace regions.
- [ ] Add a parity test matrix that runs the same Region API cases with the router client enabled and disabled.
- [ ] Add exact Prometheus registry delta assertions for success, header error, transport error, timeout, and cancellation.
- [ ] Add trace-recorder assertions for span names, counts, and parent relationships.
- [ ] Cover zero ID, old server, forwarding, follower cache miss, rate limiting, queue saturation, stalled stream, mixed caller components, mixed options, and large payloads.
### Related issues and changes
- #8690: original QueryRegion and batching tracking issue
- #11181: handle the full `uint64` Region ID range in QueryRegion
- #11182: retry follower Region cache misses selectively on the leader
- #11138: missing fallback when QueryRegion is unimplemented
- #10719: RegionSyncer correctness and operability
- #9076: QueryRegion metrics
- #9196: follower handling for QueryRegion
- #10105: enable the router client by default
- #10846: preserve per-request NeedBuckets behavior
- #10965: use batch-level runtime trace regions
Contributor guide
Research direction
Start by comparing the unary paths with client/clients/router/request.go, client/clients/router/client.go, and the QueryRegion handler in server/grpc_service.go. Run or build the parity test matrix described in the completion criteria, covering cancellation, fallback, metrics, tracing, batching, and follower behavior. Done means QueryRegion preserves the listed unary semantics without silently changing operational signals.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, grpc, prometheus
- Domain
- backend-api-design, distributed-systems, observability, testing-qa
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100