Skip to content

Commit

Permalink
Merge #123151
Browse files Browse the repository at this point in the history
123151: kvclient: check for invalid routing token r=arulajmani a=andrewbaptist

Previously in `DistSender.sendPartialBatch` we could hit an error path during processing of a proxy request if the method was called with an invalid token. This could happen if there was a range split or merge which could invalidate the routing token prior to the method being called.

This fix validates the token is not null before attempting to Sync.

Epic: none
Fixes: #123146

Release note: None

Co-authored-by: Andrew Baptist <baptist@cockroachlabs.com>
  • Loading branch information
craig[bot] and andrewbaptist committed May 2, 2024
2 parents 528a462 + 3ca2126 commit a45a622
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pkg/kv/kvclient/kvcoord/dist_sender.go
Original file line number Diff line number Diff line change
Expand Up @@ -2047,7 +2047,17 @@ func (ds *DistSender) sendPartialBatch(
// and our local replica before attempting the request. If the sync
// makes our token invalid, we handle it similarly to a RangeNotFound or
// NotLeaseHolderError from a remote server.
if ba.ProxyRangeInfo != nil {
// NB: The routingTok is usually valid when we get to this line on a
// proxy request since we never retry the outer for loop, however there
// is an edge case where we invalidate the token once here and then
// invalidate it a second time in the statement below and hit a
// retriable range loopup error.
// TODO(baptist): Consider splitting out the handling in this method for
// proxy requests vs non-proxy requests. Currently it is hard to follow
// the invariants when this is called. Alternativly move this call to be
// done immediately when the routingTok is created as it will always be
// valid at that point.
if ba.ProxyRangeInfo != nil && routingTok.Valid() {
routingTok.SyncTokenAndMaybeUpdateCache(ctx, &ba.ProxyRangeInfo.Lease, &ba.ProxyRangeInfo.Desc)
}
if !routingTok.Valid() {
Expand Down

0 comments on commit a45a622

Please sign in to comment.