kvserver: log a trace for slow splits

[miraradeva]

This change enables logging a trace of a split request if it exceeds a certain duration. The duration is controlled by a new cluster setting: kv.split.slow_split_tracing_threshold, which defaults to 2s.

Fixes: #81152

Release note: None

[cockroach-teamcity]

This change is 

[miraradeva]

Looking for feedback on:

1. What should the default value for the tracing threshold duration be? I have it at 2s now. Should it be a property of the range that's being split, similarly to the replicate queue's threshold.
2. Do we want to refactor processOneChangeWithTracing as a function of the base queue? E.g. as a wrapper around process. That way any specific queue that wants tracing for slow operations can just set its own tracing threshold. I didn't do this right away because the different queues implement process differently: some have retries built into process, and others don't. I'm assuming we don't want a trace across multiple retries, but happy to discuss.

[kvoli]

What should the default value for the tracing threshold duration be? I have it at 2s now. Should it be a property of the range that's being split, similarly to the replicate queue's threshold.

2s sounds reasonable. Re-using something similar to the replicate queue would be a bit rough to define an expected rate based off the range size given different hardware/conditions. In comparison, the rate for the replicate queue is mostly bounded by the configured snapshot rate.

Do we want to refactor processOneChangeWithTracing as a function of the base queue? E.g. as a wrapper around process. That way any specific queue that wants tracing for slow operations can just set its own tracing threshold. I didn't do this right away because the different queues implement process differently: some have retries built into process, and others don't. I'm assuming we don't want a trace across multiple retries, but happy to discuss.

I didn't take a look at other queues but if this is doable without too much added effort, then that sounds good. We wouldn't want to trace across multiple retries, since a trace on the last retry would be sufficient.

[miraradeva]

We wouldn't want to trace across multiple retries, since a trace on the last retry would be sufficient.

I looked at the various queues again. I think if we want to add optional tracing for slow queue processing in the base queue, we'd wrap the timing and tracing logic around this call to process(...). I think that makes sense since the timeout logic is there as well. Almost all queues implement the processing logic as a single attempt, except the replicate queue, which retries within the process(...) function. So I see two options:

1. Do the above refactor, as long as we're ok with tracing the process in replicate queue with all its retries.
2. Do this kind of change one queue at a time as needed. I can try to refactor a helper function to do the actual tracing to avoid code duplication.

Edited to add: the refactor in option 2 doesn't seem particularly easy because the internal processing functions (e.g. processAttempt for the split queue, and processOneChange for the replicate queue) don't have the same signatures. So, probably either do option 1 or leave it as is, unless you have another suggestion.

[kvoli]

I'd prefer to leave as is for now, unless we won't be backporting this to the 24.1 release branch.

[kvoli]

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

---

pkg/kv/kvserver/client_split_test.go line 1070 at r1 (raw file):

// TestStoreRangeSplitWithTracing tests that the split queue logs traces for
// slow splits.
func TestStoreRangeSplitWithTracing(t *testing.T) {

Nice test!

---

pkg/kv/kvserver/client_split_test.go line 1074 at r1 (raw file):

	defer log.Scope(t).Close(t)
	l := log.ScopeWithoutShowLogs(t)
	_ = log.SetVModule("split_queue=2")

nit: change this to split_queue=1 to match the expensive log check exactly inprocessAttemptWithTracing.

[miraradeva]

bors r=kvoli

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)
• check_generated_code
• docker_image_amd64
• examples_orms
• linux_amd64_build
• linux_amd64_fips_build
• linux_arm64_build
• macos_amd64_build
• macos_arm64_build
• windows_build