transaction: resolve expired async-commit locks on the read path

[dina-kar]

Motivation

When an async-commit writer fails after its prewrite (a crash, a lost connection, or the after-prewrite failpoint), readers cannot get past its locks until GC runs.

resolve_locks, which every read plan's ResolveLock step uses, sends only CheckTxnStatus. TiKV never rolls back an expired async-commit primary through CheckTxnStatus (unless force_sync_commit is set), because the transaction may already be committed through its secondaries. The status comes back Locked, the lock is treated as live, and the reader backs off until it fails with ResolveLockError. Only LockResolver::cleanup_locks, the GC path, calls CheckSecondaryLocks, and it runs only below the GC safe point: about ten minutes later with the default gc_life_time. A 1PC prewrite that falls back to async commit leaves the same locks.

Found while building a metadata store on tikv-client. A write that failed after its prewrite left its keys unreadable and unwritable for more than a minute (until GC in our test). We switched that component to plain 2PC as a workaround.

Reproduction (new test, below): 16 async-commit transactions × 32 keys, after-prewrite returning an error. After the 3 s lock TTL, a snapshot read fails on master:

ResolveLockError([LockInfo { ..., lock_ttl: 3006, use_async_commit: true, min_commit_ts: ..., secondaries: [...] }])

What is changed and how it works

In resolve_locks, an expired async-commit primary (is_expired && use_async_commit) is now resolved the way client-go's resolveAsyncCommitLock / checkAllSecondaries do. CheckSecondaryLocks is sent for the primary's secondaries, and the outcome follows the async commit protocol:

secondaries	outcome
one is committed	committed at its commit_ts
one is rolled back, or was never locked (TiKV writes a rollback record for it, so it can no longer be prewritten)	rolled back
all still locked	committed at the largest min_commit_ts of the primary and all secondaries
a lock is not an async-commit lock (TiKV fell back to 2PC in that region)	the primary decides: CheckTxnStatus with force_sync_commit

Then the primary is resolved as well as the lock that was read, so later readers of the transaction get its final status from CheckTxnStatus directly. An unexpired async-commit primary is still a live lock, as before. The decision is SecondaryLocksStatus::async_commit_version, shared with cleanup_locks.

Two fixes in code cleanup_locks already used

1. Merging CheckSecondaryLocks answers ignored a rolled-back region. TiKV answers a region in one of three ways: every key locked, commit_ts set, or no locks and no commit_ts (a key rolled back, or a missing lock it just rolled back). The merge ignored the third answer. With another region still locked, cleanup_locks then committed the transaction at that region's min_commit_ts, although one of its keys was rolled back and can never be committed: a partial commit. The merge now records rolled_back, returns an error instead of hitting assert_eq! on two different commit timestamps, and rejects "committed and rolled back" as contradictory. The commit version now also includes the primary's min_commit_ts, as client-go's does (it starts from status.primaryLock.MinCommitTs).
2. The forced re-check after a 2PC fallback returned the cached result of the first check. check_txn_status caches an expired Locked status, and the second call with force_sync_commit = true returned that cached Locked, so cleanup_locks failed with "cleanup_locks fail to clean locks". A cached Locked status now answers only unforced checks.

Tests

• Unit (transaction::lock::tests, mock TiKV):

  • expired_async_commit_with_every_secondary_locked_commits_at_the_max_min_commit_ts
  • expired_async_commit_with_a_rolled_back_secondary_is_rolled_back
  • expired_async_commit_with_a_committed_secondary_commits_at_its_ts
  • async_commit_that_fell_back_to_2pc_is_resolved_by_its_primary
  • unexpired_async_commit_primary_stays_live
  • secondary_locks_status_decides_the_async_commit_outcome
  • merging_secondary_locks_refuses_contradictions

  The first four fail without the change.

• Integration: txn_read_resolves_expired_async_commit_locks (tests/failpoint_tests.rs). It leaves 512 async-commit locks with after-prewrite, waits for the TTL, reads every key through a snapshot, and expects no locks afterwards. On master it fails with ResolveLockError. With the change the reads take about 0.4 s and leave no locks. The existing txn_cleanup_async_commit_locks, txn_cleanup_range_async_commit_locks and txn_resolve_locks still pass.

• All integration suites were run on a tiup playground v8.5.8 (API v2, one TiKV). cargo test --lib, clippy with -D clippy::all and cargo fmt --check are clean.

Notes for reviewers

• Resolving the primary adds one ResolveLock per resolved async-commit transaction on the read path. client-go resolves every key of the transaction there. This change stays closer to this crate's existing per-region resolution and resolves only the primary and the region that was read.
• The rollback case needs a region whose prewrite never arrived, which is hard to produce with the existing failpoints, so it is covered by unit tests only.

Check list

• Unit tests
• Integration test
• DCO: the commit is signed off.

Release note: readers resolve the locks of a failed async-commit or 1PC transaction once its primary lock expires, instead of waiting for GC. cleanup_locks no longer commits an async-commit transaction that has a rolled-back secondary.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign marsishandsome for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[coderabbitai]

Warning

Review limit reached

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 2 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: ded40d6d-4c2e-403a-bd97-9897dd628fbc

📥 Commits

Reviewing files that changed from the base of the PR and between ab4be1c and f037ebb.

📒 Files selected for processing (3)

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

---

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.}