kv: introduce request RoutingPolicy configuration#68191
kv: introduce request RoutingPolicy configuration#68191craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
Ran out of time for this review while wrapping up loose ends before PTO, sorry Nathan! I won't be reviewing this until the week of Aug 16th at which point you'll surely already have it merged. |
irfansharif
left a comment
There was a problem hiding this comment.
Reviewed 11 of 11 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/kv/kvclient/kvcoord/dist_sender.go, line 1821 at r1 (raw file):
// Order by latency. log.VEvent(ctx, 2, "routing to nearest replica; leaseholder not required") replicas.OptimizeReplicaOrder(ds.getNodeDescriptor(), ds.latencyFunc)
Missing a ds.dontRorderReplicas check here.
pkg/kv/kvserver/replica.go, line 1293 at r1 (raw file):
} // Is the lease valid?
Comment here needs updating.
Touches cockroachdb#67562. This commit introduces a new RoutingPolicy configuration that lives on a BatchRequest header. A request's routing policy specifies how the request should be routed to the replicas of its target range(s) by the DistSender. There are initially two routing policies: ``` enum RoutingPolicy { // LEASEHOLDER means that the DistSender should route the request to the // leaseholder replica(s) of its target range(s). LEASEHOLDER = 0; // NEAREST means that the DistSender should route the request to the // nearest replica(s) of its target range(s). NEAREST = 1; } ``` The default policy is `LEASEHOLDER`. Routing policies allow us to stop overloading the use of the ReadConsistency enum to dictate both how the client should route a request to a server and which kinds of requests should be eligible to be served by a given replica. Routing policies are a client-side only concept. They do not dictate which replicas in a range are eligible to serve the request, only which replicas are considered as targets by the DistSender, and in which order. A request that is routed to an ineligible replica (a function of request type, timestamp, and read consistency) will be rejected by that replica and the DistSender will target another replica in the range. As discussed in cockroachdb#67725 (review), we will likely need to introduce a third routing policy that called `SINGLE_REPLICA` to address cockroachdb#67554. This policy would be accompanied by a ReplicaDescriptor and would specify that a given request must be sent to that replica and DistSender should throw an error if the replica is not part of its cached range descriptor. This is important to ensure that a QueryResolvedTimestampRequest and its follow-up ScanRequest are both sent to the same replica.
0c56d6b to
79a956f
Compare
nvb
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @irfansharif)
pkg/kv/kvclient/kvcoord/dist_sender.go, line 1821 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
Missing a ds.dontRorderReplicas check here.
Done.
|
Build succeeded: |
Half of #67551.
Touches #67562.
This commit introduces a new RoutingPolicy configuration that lives on a BatchRequest header. A request's routing policy specifies how the request should be routed to the replicas of its target range(s) by the DistSender. There are initially two routing policies:
The default policy is
LEASEHOLDER.Routing policies allow us to stop overloading the use of the ReadConsistency enum to dictate both how the client should route a request to a server and which kinds of requests should be eligible to be served by a given replica. Routing policies are a client-side only concept. They do not dictate which replicas in a range are eligible to serve the request, only which replicas are considered as targets by the DistSender, and in which order. A request that is routed to an ineligible replica (a function of request type, timestamp, and read consistency) will be rejected by that replica and the DistSender will target another replica in the range.
As discussed in #67725 (review), we will likely need to introduce a third routing policy called
SINGLE_REPLICAto address #67554. This policy would be accompanied by a ReplicaDescriptor and would specify that a given request must be sent to that replica and DistSender should throw an error if the replica is not part of its cached range descriptor. This is important to ensure that a QueryResolvedTimestampRequest and its follow-up ScanRequest are both sent to the same replica.