core: intent_resolver_ir_batcher.sendBatch timed out after 30s

[vivekmenezes]

E190418 14:30:01.900300 1 workload/cli/run.go:428  error in delivery: ERROR: operation "intent_resolver_ir_batcher.sendBatch" timed out after 30s (SQLSTATE XXUUU)

I'm seeing the above in https://teamcity.cockroachdb.com/viewLog.html?buildId=1247383&tab=buildLog

[ajwerner]

I just added back this timeout #36845.

I'm curious if the problem is that the returned error's cause is not context.DeadlineExceeded or if we would have hit this before in a sorely overloaded cluster. Should this failure be retried internally?

I'll investigate further to figure out how to error makes its way back to the client.

[ajwerner]

Had a conversation with @nvanbenschoten and @andreimatei about this. It seems inconsistent to return an error to a client due to an internal timeout, we certainly don't do that for general range unavailability. This seems to be a long-running conversation.

Interestingly we had these timeouts on intent resolution for a good long while and I suspect if you were to run this overloaded situation in 2.1 (see https://github.com/ajwerner/cockroach/blame/295b6ae3142518f04a5771c79ce171043697ee1f/pkg/storage/intent_resolver.go#L991) you would see the same timeout.

One proposed solution is to only add a timeout to intent resolution batches we know to consist only of asynchronous requests. Another option is to revert the timeout change. We should ask ourselves why we care about hung intent resolution. On some level they represent a leak but we do have some mechanisms in the intent resolver to push back the total amount of async intent resolution. @jordanlewis you specifically were concerned about them in #36806 (comment). I'm happy to type up the first but want to reach consensus on the right course of action before making a PR.

[jordanlewis]

I like the idea of adding the timeout only in the case of asynchronous intent resolution, but I don't have a serious horse in the race. I only was doing some drive by observation on those suspiciously hung goroutines.

[vivekmenezes]

I agree that we shouldn't be returning an error to the client here. How about you discuss this with other core folks and come up with a solution. Thanks

[ajwerner]

Closed by #37103.