Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: clean up after EndTxn even if client disconnects #64770

Closed
erikgrinaker opened this issue May 6, 2021 · 0 comments · Fixed by #64869
Closed

kvserver: clean up after EndTxn even if client disconnects #64770

erikgrinaker opened this issue May 6, 2021 · 0 comments · Fixed by #64869
Assignees
Labels
A-kv-transactions Relating to MVCC and the transactional model. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented May 6, 2021

The main transaction cleanup (record and intents) is done after the Raft entry has been applied:

if len(propResult.EndTxns) > 0 {
if err := r.store.intentResolver.CleanupTxnIntentsAsync(
ctx, r.RangeID, propResult.EndTxns, true, /* allowSync */
); err != nil {
log.Warningf(ctx, "%v", err)
}
}

However, if the client context is cancelled during Raft processing, we skip this cleanup:

case <-ctxDone:
// If our context was canceled, return an AmbiguousResultError,
// which indicates to the caller that the command may have executed.
abandon()
log.VEventf(ctx, 2, "context cancellation after %0.1fs of attempting command %s",
timeutil.Since(startTime).Seconds(), ba)
return nil, nil, roachpb.NewError(roachpb.NewAmbiguousResultError(ctx.Err().Error()))

We should try to do this cleanup even when the context is cancelled.

As noted in #60585 (comment), this problem is exacerbated by rollbacks following client disconnects during in-flight txns or DML statements getting a 3-second context timeout -- if the request doesn't make it through RPC and Raft in that time, it won't be cleaned up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-transactions Relating to MVCC and the transactional model. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant