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: don't PUSH_ABORT encountered intent txns during GC #65777

Closed
erikgrinaker opened this issue May 27, 2021 · 0 comments · Fixed by #65001
Closed

kvserver: don't PUSH_ABORT encountered intent txns during GC #65777

erikgrinaker opened this issue May 27, 2021 · 0 comments · Fixed by #65001
Assignees
Labels
A-kv-transactions Relating to MVCC and the transactional model. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented May 27, 2021

When the GC queue is cleaning up intents, it appears to push their txns with PUSH_ABORT:

intentCount, err := repl.store.intentResolver.
CleanupIntents(ctx, intents, gcTimestamp, roachpb.PUSH_ABORT)

This can be very disruptive, and also block for a significant time. It does have an age threshold:

// IntentAgeThreshold is the threshold after which an extant intent
// will be resolved.
var IntentAgeThreshold = settings.RegisterDurationSetting(
"kv.gc.intent_age_threshold",
"intents older than this threshold will be resolved when encountered by the GC queue",
2*time.Hour,

But aborting txns that have been doing work for >2h seems particularly disruptive.

I suggest we use PUSH_TOUCH here, to leave active txns alone. That will also kick off txn recovery for STAGING txns if the txn liveness has expired:

if reply.PusheeTxn.Status == roachpb.STAGING && (pusherWins || recoverOnFailedPush) {
err := roachpb.NewIndeterminateCommitError(reply.PusheeTxn)
log.VEventf(ctx, 1, "%v", err)
return result.Result{}, err
}

We should also consider simply dropping IntentAgeThreshold and rely solely on the intent txn status if we can efficiently distinguish between active and stale intents via other means.

@erikgrinaker erikgrinaker added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-kv-transactions Relating to MVCC and the transactional model. labels May 27, 2021
@erikgrinaker erikgrinaker self-assigned this May 27, 2021
@erikgrinaker erikgrinaker changed the title kvserver: don't PUSH_ABORT encountered intent txns kvserver: don't PUSH_ABORT encountered intent txns during GC May 27, 2021
@craig craig bot closed this as completed in 034bec9 Jul 7, 2021
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-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant