This change is 

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.

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.

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.

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

bors r=kvoli

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