cockroach-go icon indicating copy to clipboard operation
cockroach-go copied to clipboard

ExecuteInTx does not honor context cancellation

Open knz opened this issue 3 years ago • 3 comments

The retry loop in ExecuteInTx does not check ctx.Done to abort the retries if the context is cancelled.

Found while working on adding query cancellation in the crdb SQL shell.

cc @andreimatei @rafiss

knz avatar Feb 11 '22 13:02 knz

Don't the drivers listen for cancellation (and return a non-retryable error) - regardless of whether crdb supports cancellation or not? Do we need this library to react to cancellation?

andreimatei avatar Feb 12 '22 05:02 andreimatei

The context may also have a deadline! We want that to stop the iteration too.

knz avatar Feb 12 '22 09:02 knz

Well if the driver does something for cancellation, it also does something for timeout. But the question is - do the drivers indeed care about the context? I definitely thought they did, but only for the ExecWithContext methods. We used those methods in between the retries, even if the caller's closure doesn't.

andreimatei avatar Feb 12 '22 14:02 andreimatei