transaction: clear prewrite locks on rollback of a failed pessimistic commit

[eduralph]

After a 2PC commit fails at prewrite, the already-placed prewrite (2PC) locks must be rolled back. The terminal pessimistic rollback sent PessimisticRollback, which only removes LockType::Pessimistic locks, so it silently left the prewrite locks behind while still returning Ok — leaving keys locked until TTL expiry and lock resolution.

Roll back a pessimistic transaction that has started committing with BatchRollback (as the optimistic path and client-go's commit cleanup do), which clears locks by start_ts regardless of type. A pessimistic transaction rolled back before commit (locks still pessimistic) continues to use the narrower PessimisticRollback.

Adds a failpoint regression test.

Closes #545

Signed-off-by: Eduard Ralph eduard@ralphovi.net

Summary by CodeRabbit

• Bug Fixes
  • Improved rollback handling for pessimistic transactions when commit() fails after the prewrite/2PC step, ensuring proper cleanup of any remaining 2PC and pessimistic locks.
  • Rollback now consistently bases its cleanup strategy on the commit path state reached during the initial commit, including retry situations.
• Tests
  • Added regression coverage for pessimistic transactions failing after prewrite, with and without a retry, verifying rollback() succeeds and the client ends with zero remaining locks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c5ee2ea-7cb4-45e2-824c-0f15e69b3678

📥 Commits

Reviewing files that changed from the base of the PR and between 173895e and a7bda9e.

📒 Files selected for processing (2)

• src/transaction/transaction.rs
• tests/failpoint_tests.rs

🚧 Files skipped from review as they are similar to previous changes (2)

• tests/failpoint_tests.rs
• src/transaction/transaction.rs

---

📝 Walkthrough

Walkthrough

Transaction rollback now preserves whether commit reached prewrite, then uses that state to choose the rollback RPC. New failpoint tests cover lock cleanup after failed pessimistic commits and a rollback retry.

Changes

Pessimistic Rollback Lock Cleanup Fix

Layer / File(s)	Summary
Rollback entry point tracks prewritten state src/transaction/transaction.rs	Transaction initializes a prewritten flag, sets it when commit enters the commit path, and preserves it during rollback before calling Committer::rollback(prewritten).
Committer rollback branches on prewritten src/transaction/transaction.rs	Committer::rollback accepts prewritten: bool, updates its documentation, binds shared rollback values before branching, and uses PessimisticRollback only when the transaction is pessimistic and not prewritten; otherwise it uses BatchRollback.
Regression tests for rollback cleanup tests/failpoint_tests.rs	Two failpoint-based tests force commit to fail after prewrite, then verify rollback clears locks in the normal path and after a retrying rollback attempt.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Transaction
  participant Committer
  participant TiKV

  Transaction->>Transaction: mark commit path as prewritten
  Transaction->>Committer: commit
  Committer->>TiKV: prewrite locks
  TiKV-->>Committer: commit failure
  Transaction->>Committer: rollback(prewritten=true)
  Committer->>TiKV: BatchRollback by start_ts
  TiKV-->>Committer: locks cleared

Loading

Suggested reviewers: pingyu, iosmanthus

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main fix: clearing prewrite locks on rollback after a failed pessimistic commit.
Linked Issues check	✅ Passed	The changes match #545: rollback switches to BatchRollback after prewrite, preserves that state across retries, and adds a regression test.
Out of Scope Changes check	✅ Passed	The refactorings stay within the rollback/cleanup fix and test coverage; no unrelated changes are evident.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/transaction/transaction.rs`:
- Around line 704-708: The rollback path in transaction logic recomputes
`prewritten` from `get_status()` after the status has already moved to
`StartedRollback`, so retrying `rollback()` can lose the fact that the txn had
started committing. Update `transaction::Transaction`/`rollback()` to persist
this rollback mode across retries, either by storing a dedicated flag or
preserving the commit-start state before the transition, and use that saved
state when choosing between `BatchRollback` and `PessimisticRollback`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 42469290-3135-4f7c-aa8c-2cbf4a56d457

📥 Commits

Reviewing files that changed from the base of the PR and between e53837d and 03bf9d3.

📒 Files selected for processing (2)

• src/transaction/transaction.rs
• tests/failpoint_tests.rs

[pingyu]

LGTM~

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: pingyu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [pingyu]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2026-07-18 08:03:48.686503222 +0000 UTC m=+1046414.722598268: ☑️ agreed by pingyu.