Skip to content

fix(lockservice): backport fenced orphan unlock to 4.1-dev - #25652

Merged
XuPeng-SH merged 6 commits into
matrixorigin:4.1-devfrom
LeftHandCold:agent/lockservice-fenced-orphan-unlock-4.1-dev
Jul 13, 2026
Merged

fix(lockservice): backport fenced orphan unlock to 4.1-dev#25652
XuPeng-SH merged 6 commits into
matrixorigin:4.1-devfrom
LeftHandCold:agent/lockservice-fenced-orphan-unlock-4.1-dev

Conversation

@LeftHandCold

Copy link
Copy Markdown
Contributor

What type of PR is this?

  • API-change
  • BUG
  • Improvement
  • Documentation
  • Feature
  • Test and CI
  • Code Refactoring

Which issue(s) this PR fixes:

refs #25553

refs #22115
refs #22791

Backport of #25563 to 4.1-dev.

What this PR does / why we need it:

This PR backports the lockservice orphan-unlock safety fix from #25563. It prevents orphan cleanup from releasing a holder while its TN Commit can still finish, and ensures every exceptional orphan unlock carries a timestamp that fences stale waiters.

Root cause

The incident follows the single-TN-shard 1PC branch and is a race between the TN Commit handler and lockservice orphan cleanup:

  1. TN receives Commit and LockTableAllocator.Valid records the transaction as committing.
  2. Commit remains in flight while CN/TN communication is abnormal.
  3. cleanCommitState can apply an unavailable or stale CN active-transaction view to the allocator state.
  4. A waiter classifies the same holder as orphan. If CannotCommit loses the in-flight Commit state, it can incorrectly report that the holder is safe to unlock.
  5. The holder lock is released, but the original TN Commit subsequently succeeds. A waiter can then proceed without observing that Commit, breaking hotspot-row serialization.

Fix

  1. Track successful Valid calls until their matching FinishCommit, including duplicate or concurrent Commit requests.
  2. Keep in-flight Commit state during cleanup and retain cannotCommit tombstones when the active-transaction query fails.
  3. Make cleaner application generation-aware so an older active-transaction snapshot cannot delete newer state.
  4. Return a strict HLC fence from CannotCommit: (upper.PhysicalTime + 1, 0).
  5. Require both allocator proofs before exceptional orphan unlock: the transaction is not committing and a non-empty fence was returned.
  6. Remove the CN local-clock fallback from waiter, timeout, stale-bind, local, and remote orphan cleanup paths.

Scope and risk

The normal Commit path adds allocator-local state operations only. It adds no RPC and removes state as soon as the final Commit attempt returns. Normal lock acquisition, wait timeout, isolation, and ordinary commit/unlock behavior are unchanged.

The deployment invariant for this hotfix is one TN shard per transaction (len(TNShards) == 1). The retained multi-TN asynchronous Commit branch is outside this PR's supported scope.

If a TN Commit itself never returns, the holder remains protected because releasing a lock while that Commit can still become visible is unsafe. Existing TN recovery remains responsible for resolving that condition.

Validation

  • go test -race ./pkg/lockservice -run 'Test(CannotCommitWaitsForAllCommitAttempts|CleanCommitStateKeepsInflightCommit|CleanerPreservesCannotCommitOnActiveTxnQueryError|CleanerDoesNotDeleteTombstoneCreatedAfterServiceSnapshot|CannotCommitGenerationRefresh|NewFenceTS)' -count=1
  • go test ./pkg/lockservice -count=1
  • CGo-configured go test ./pkg/txn/service -run 'TestCommitWithLockTables' -count=1
  • go vet ./pkg/lockservice
  • CGo-configured go vet ./pkg/txn/service

@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@matrix-meow matrix-meow added the size/L Denotes a PR that changes [500,999] lines label Jul 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Something isn't working kind/test-ci size/L Denotes a PR that changes [500,999] lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants