tikv / tikv/pd

client: router client does not fall back to unary RPCs when PD doesn't implement QueryRegion

Open
#11,138 0 comments 0 reactions 0 assignees View on GitHub
type/bug
Dominant language
Go
Stars
1.2k
Forks
783
Avg merge
5d 21h
Merged PRs (30d)
36

Description

## Bug Report

### What did you do?

Run a new PD client (with `EnableRouterClient` / the "query region" feature turned on) against an old PD server that does not implement the `QueryRegion` RPC.

Relevant code path:
- `client.GetRegion`/`GetPrevRegion`/`GetRegionByID` (`client/client.go`) route exclusively through `routerClient` once `EnableRouterClient` is on (`client/client.go:725`, `:777`, `:828`), with no check of whether the connected PD actually supports `QueryRegion`.
- The router client's stream is created successfully against the old PD (`client/clients/router/client.go:498`), because gRPC only sends stream headers at creation time; the failure only shows up later.
- On the first `send`/`recv` (`client/clients/router/client.go:776-790`), the old PD returns `codes.Unimplemented: unknown method QueryRegion for service pdpb.PD`. This error is propagated to every caller (`handleProcessRequestError` → `cancelCollectedRequests` → `Request.wait()`), and `handleProcessRequestError` (`client/clients/router/client.go:811`) then releases the connection and schedules a member-changed check, so the stream gets rebuilt and immediately fails again — a persistent retry/error loop, not a crash.

### What did you expect to see?

When the connected PD doesn't support `QueryRegion`, the client should fall back to the legacy unary RPCs (e.g. `GetRegion`/`GetRegionByID`/`GetPrevRegion`) so region queries keep working, the same way other capability gaps between mixed client/server versions are handled elsewhere in the client.

### What did you see instead?

There is no fallback: once `EnableRouterClient` is enabled, `GetRegion`/`GetPrevRegion`/`GetRegionByID` fail persistently with an `Unimplemented` error whenever the server doesn't implement `QueryRegion`, and the router client keeps rebuilding and failing the stream in a loop. It does not panic or crash the process, but the feature becomes silently unusable against older PD servers — a compatibility regression when a newer client talks to an older PD during a rolling upgrade.

### What version of PD are you using (`pd-server -V`)?

N/A — found via code review of the client's router-client path (`client/clients/router/client.go`, `client/client.go`), reproducible against any PD build predating the `QueryRegion` RPC.

Contributor guide

Open the contributing guide

Research direction

Start with GetRegion, GetPrevRegion, and GetRegionByID in client/client.go, then trace router stream creation and send/receive handling in client/clients/router/client.go:498 and :776-790. Reproduce against an older PD server, follow the Unimplemented path through handleProcessRequestError, and verify that region queries use the legacy unary RPCs instead of repeatedly rebuilding the failing stream.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, grpc
Domain
backend-api-design, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.