transaction: bound async-commit commit ts and handle TiKV's fallbacks

[dina-kar]

Motivation

Committer::prewrite sends async-commit and 1PC prewrites with min_commit_ts = 0 and no max_commit_ts (// FIXME set max_commit_ts and min_commit_ts). Committer::commit also assumes TiKV always used the protocol it asked for (// FIXME: min_commit_ts == 0 => fallback to normal 2PC, then min_commit_ts.unwrap()). Three problems follow:

1. Unbounded commit timestamp. TiKV computes the commit ts of an async-commit or 1PC transaction from max_ts, the largest timestamp any reader has used on the keys. Without max_commit_ts, a reader with a timestamp far in the future pushes the commit ts arbitrarily far. client-go bounds it with calculateMaxCommitTS (start ts + elapsed + async-commit.safe-window, 2 s by default).
2. TiKV's 2PC fallback was ignored. Where TiKV cannot use async commit in a region (with a max_commit_ts, because the commit ts would exceed it), it writes an ordinary 2PC lock and answers min_commit_ts = 0. The committer took the maximum over all regions and committed every key at that timestamp. For the fallen-back region that timestamp is not valid: it can be below a read that TiKV already allowed there. If every region fell back, the commit ts was 0.
3. 1PC fallback left locks. If a single-region 1PC prewrite did not commit in one phase (one_pc_commit_ts = 0), TiKV had still written ordinary locks, but commit returned Error::OnePcFailure and left them for readers to resolve.

What is changed and how it works

• Async-commit and 1PC prewrites carry min_commit_ts = max(start_ts, for_update_ts) + 1 and max_commit_ts = start_ts + ((elapsed + 2 s) << 18), as in client-go's buildPrewriteRequest and calculateMaxCommitTS. 2PC prewrites are unchanged.
• If any region answers min_commit_ts = 0 for an async-commit prewrite, the transaction falls back to 2PC, as client-go does (c.setAsyncCommit(false)). The primary is committed with a fresh timestamp (the same retry and undetermined handling as any 2PC primary commit), then the secondaries.
• A 1PC prewrite that did not commit in one phase goes on to async commit or 2PC instead of failing. commit no longer returns Error::OnePcFailure; the variant stays for compatibility.
• A missing min_commit_ts is an error instead of an unwrap() panic.

The safe window is a constant (ASYNC_COMMIT_SAFE_WINDOW, 2 s). Making it a TransactionOptions setting is easy if maintainers want it.

Tests

Unit tests in transaction::transaction::tests, with a mock TiKV that records prewrite and commit requests:

• async_commit_prewrite_bounds_the_commit_ts: min_commit_ts = start + 1, max_commit_ts ≥ start + 2 s, and the commit ts is the max of the regions' min_commit_ts.
• two_phase_prewrite_carries_no_commit_ts_bounds
• async_commit_falls_back_to_2pc_when_a_region_did: one of two regions answers 0, and the primary alone is committed before commit returns.
• one_pc_fallback_commits_the_locks_instead_of_failing
• one_pc_commit_returns_the_one_pc_commit_ts

Three of these fail on master. The integration suites (including the async-commit and 1PC tests) pass on a tiup playground v8.5.8. cargo test --lib, clippy with -D clippy::all and cargo fmt --check are clean.

Notes for reviewers

• This interacts with lock resolution. After a 2PC fallback, some locks are async-commit locks and some are not. Readers then take the "not an async-commit lock" path, where the primary decides. See the companion PR "resolve expired async-commit locks on the read path", which adds that path to resolve_locks (cleanup_locks already had it).
• Error::OnePcFailure is still in the public enum but is no longer produced by commit.

Check list

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

Release note: async-commit and 1PC prewrites set max_commit_ts. The client falls back to 2PC when TiKV does, and commits a 1PC transaction that fell back instead of failing with OnePcFailure.

[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 cfzjywxk 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: c5815059-8b41-474b-9e3a-084836c0229c

📥 Commits

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

📒 Files selected for processing (1)

• src/transaction/transaction.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.}