Nice catch! It looks like this is fallout from 7fb06a2#diff-271a3179bf7f12a2ed29b993611bf24f4c3c4a582a785d5d29a61d2d02c5f21fL1553, where we removed the needsTruncate argument and logic from sendPartialBatch but continued passing the untruncated rs span. So we won't need to backport this to v22.1 or earlier, but we'll want to get this back to v22.2 as soon as possible.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani)

-- commits line 17 at r1:
It's probably worth pointing at 7fb06a2 in the commit message, in case someone wants to understand this in the future.

pkg/kv/kvclient/kvcoord/dist_sender_test.go line 4604 at r1 (raw file):

	splitRDB := mockRangeDescriptorDBForDescs(splitDescs...)

	var numAttempts atomic.Int32

atomic.Int32 is new as of Go 1.19. I thought we had kept the go directive in the go.mod file at 1.17 so that new features (i.e. generics) could not be used yet. And yet, CI is green. @rickystewart do you know what's going on here?

In the meantime, let's switch this back to a var numAttempts int32 and use the old atomic.AddInt32 and atomic.LoadInt32 so we can proceed with this PR.

pkg/kv/kvclient/kvcoord/dist_sender_test.go line 4637 at r1 (raw file):

	require.True(t, testutils.IsError(pErr.GoError(), "boom"))
	// 6 attempts each for the two partial batches.
	require.Equal(t, int32(12), numAttempts.Load())

How exactly does this fail without your fix?

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @arulajmani)

pkg/kv/kvclient/kvcoord/dist_sender.go line 1535 at r1 (raw file):

}

// sendPartialBatch sends the supplied batch to the range specified by desc.

s/by desc/by the routing token/

pkg/kv/kvclient/kvcoord/dist_sender.go line 1539 at r1 (raw file):

// The batch request is supposed to be truncated already so that it contains
// only requests which intersect the range descriptor and keys for each request
// are limited to the range's key span. positions argument describes how the

Consider updating this comment to mention the relationship between rs and the current range descriptor (in routingTok).

TFTR!

Might be worth having a look at some typical stack sizes in steady-state clusters

I looked at the DistSender stack, and doing some stack pointer/frame pointer math, these stack frames seem to be around ~2KB. The stack doesn't seem deep enough to hit that 1GB limit, so maybe there's more to look at here. I'll reword the commit message so that merging this PR doesn't close the issue.

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten and @rickystewart)

-- commits line 17 at r1:

Done.

pkg/kv/kvclient/kvcoord/dist_sender.go line 1539 at r1 (raw file):

Added more words here.

pkg/kv/kvclient/kvcoord/dist_sender_test.go line 4604 at r1 (raw file):

I didn't realize this was a new feature. Great advert for GoLand autocomplete though haha

pkg/kv/kvclient/kvcoord/dist_sender_test.go line 4637 at r1 (raw file):

It would just timeout because the maxRetries we set above won't be respected. It's not the greatest test, given we don't configure this in production; maybe we should?