add locks functions

[daviszhen]

What type of PR is this?

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

Which issue(s) this PR fixes:

issue #24486

What this PR does / why we need it:

增加user level locks函数: is_used_lock, release_all_locks.

• IS_USED_LOCK：

• 新增 lockservice 点查接口 GetLockHolder(tableID, row, options)
• 复用现有 GetTxnLock 远程 RPC，并在 response 中增加 Holder / Found，让 holder 查询能路由到真正持有 lock table 的 CN，支持跨 CN。
• user lock 的 row key 保持为 \0<lock_name>；synthetic txn id 中编码 \0<connection_id>\0<lock_name>。
• IS_USED_LOCK 从 holder 的 synthetic txn id 中解析 connection_id；如果锁不存在或 holder txn id 不是 user lock 格式，则返回 NULL。

RELEASE_ALL_LOCKS：

• 只维护本 session 的本地状态：counts 记录可重入计数，byOwner 记录当前 session 持有的 lock name。
• 只释放当前 session 自己持有的 user locks，通过当前 session 的 owner 和 connection id 生成 synthetic txn id 执行 unlock。
• 返回释放的本地可重入 lock 次数。
• 不再维护本地全局 holders map，避免错误报告或释放其他 session / 其他 CN 持有的锁。

[qodo-code-review]

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 →

[XuPeng-SH]

I re-checked the latest head. The earlier txn-id compatibility and function-id stability concerns look addressed now, but I still see one real correctness issue in the new holder-lookup path.\n\npkg/lockservice/lock_table_remote.go:245-257 refreshes the bind on error via handleError, but if that refresh succeeds (handleError returns nil) it immediately returns (false, nil) instead of retrying doGetLockHolder on the new bind. That means a stale remote binding is surfaced to SQL as "no holder" / NULL even though the lock may still exist on the new CN. With IS_USED_LOCK() now relying on GetLockHolder, this becomes a user-visible false negative during bind migration / CN movement, not just an internal observability gap.\n\nPlease retry the holder lookup after a successful bind refresh, the same way the caller needs the post-refresh result rather than a silent "not found", and add a targeted test that exercises GetLockHolder across a bind-change path.

[daviszhen]

I re-checked the latest head. The earlier txn-id compatibility and function-id stability concerns look addressed now, but I still see one real correctness issue in the new holder-lookup path.\n\npkg/lockservice/lock_table_remote.go:245-257 refreshes the bind on error via handleError, but if that refresh succeeds (handleError returns nil) it immediately returns (false, nil) instead of retrying doGetLockHolder on the new bind. That means a stale remote binding is surfaced to SQL as "no holder" / NULL even though the lock may still exist on the new CN. With IS_USED_LOCK() now relying on GetLockHolder, this becomes a user-visible false negative during bind migration / CN movement, not just an internal observability gap.\n\nPlease retry the holder lookup after a successful bind refresh, the same way the caller needs the post-refresh result rather than a silent "not found", and add a targeted test that exercises GetLockHolder across a bind-change path.

已经修改

[XuPeng-SH]

I re-checked the new head. The previous stale-bind false-NULL issue is not fully fixed yet.\n\npkg/lockservice/lock_table_remote.go:245-257 now does continue after handleError() refreshes the bind, but that loop is still running on the same remoteLockTable instance. doGetLockHolder() always sends req.LockTable = l.bind, and on bind change service.handleBindChanged() replaces the tableGroups entry with a newly created lock-table object (it does not mutate this in-flight l). So after a bind refresh you can still keep retrying on the stale remote table pointer instead of actually querying through the new bind / new local-vs-remote table object.\n\nThe new test does not pin the real production behavior here because its fake bind-changed callback never swaps the lock-table object, and the mock GetLockHolder handler returns success on the second call regardless of which bind was used. Please make the retry reacquire the lock table after bind change (or bubble ErrLockTableBindChanged back so the caller reacquires and retries), and add a test that proves the retry goes through the refreshed bind/object rather than the stale one.

[XuPeng-SH]

I re-checked the latest head and still see two correctness issues.

1. IS_USED_LOCK() can still ignore statement cancellation when the holder CN is unreachable.
   service.GetLockHolder() now re-checks ctx.Err() while reacquiring after ErrLockTableBindChanged, but remoteLockTable.getLockHolder() itself still loops forever on transient RPC failures with backoff and no context parameter. In the unhappy path where the remote RPC keeps failing but the allocator keeps returning the same bind, IS_USED_LOCK() can still hang until the process/session is interrupted instead of respecting timeout/cancel.

   Suggestion: thread the caller context through getLockHolder / doGetLockHolder, and break the retry loop as soon as ctx.Done() fires.

2. Remote GetLockHolder still does not carry enough data for a cold lookup.
   The RPC only sends GetLockHolder.Row, but the server-side fallback path in getLocalLockTable() recreates a missing table from req.Lock.Rows and req.Lock.Options.Sharding. Those fields are unset for Method_GetLockHolder, so a cold remote lookup can still fail with ErrLockTableNotFound / stale-bind behavior even when a holder exists.

   Suggestion: either make getLocalLockTable() method-aware for GetLockHolder (use req.GetLockHolder.Row and req.LockTable.Sharding), or extend the GetLockHolder request payload so the handler has the same lookup inputs as Lock / GetTxnLock.

Non-blocking coverage note: I still do not see a direct transient-retry test for remoteLockTable.getLockHolder() where RPC fails once, bind does not change, and the retry later succeeds.

[daviszhen]

I re-checked the latest head and still see two correctness issues.

1. IS_USED_LOCK() can still ignore statement cancellation when the holder CN is unreachable.
   service.GetLockHolder() now re-checks ctx.Err() while reacquiring after ErrLockTableBindChanged, but remoteLockTable.getLockHolder() itself still loops forever on transient RPC failures with backoff and no context parameter. In the unhappy path where the remote RPC keeps failing but the allocator keeps returning the same bind, IS_USED_LOCK() can still hang until the process/session is interrupted instead of respecting timeout/cancel.
   Suggestion: thread the caller context through getLockHolder / doGetLockHolder, and break the retry loop as soon as ctx.Done() fires.
2. Remote GetLockHolder still does not carry enough data for a cold lookup.
   The RPC only sends GetLockHolder.Row, but the server-side fallback path in getLocalLockTable() recreates a missing table from req.Lock.Rows and req.Lock.Options.Sharding. Those fields are unset for Method_GetLockHolder, so a cold remote lookup can still fail with ErrLockTableNotFound / stale-bind behavior even when a holder exists.
   Suggestion: either make getLocalLockTable() method-aware for GetLockHolder (use req.GetLockHolder.Row and req.LockTable.Sharding), or extend the GetLockHolder request payload so the handler has the same lookup inputs as Lock / GetTxnLock.

Non-blocking coverage note: I still do not see a direct transient-retry test for remoteLockTable.getLockHolder() where RPC fails once, bind does not change, and the retry later succeeds.

已经修改

[XuPeng-SH]

I re-checked the latest head. The previous remote-holder blockers are fixed, but I still see one remaining local stale-bind race in the holder-lookup path.

service.GetLockHolder() and handleRemoteGetLockHolder() still do:

1. get/recreate a lock table,
2. then call l.getLockHolder(...)
   without holding bindChangeMu across those two steps.

If a bind change closes/replaces that local table in between, localLockTable.getLockHolder() returns (false, nil) for mu.closed, which is indistinguishable from “lock does not exist”. The caller therefore does not retry against the new authoritative table and can transiently report NULL / no owner even though the bind has moved and the new table may still have a holder.

You can see the pieces in the current head:

• service.handleBindChanged() swaps the table under bindChangeMu and closes the old one
• localLockTable.getLockHolder() still returns pb.WaitTxn{}, false, nil when l.mu.closed
• unlike Lock, the holder-lookup path does not hold bindChangeMu.RLock() across table acquisition + lookup

Concrete suggestions

1. Mirror the Lock path and hold bindChangeMu.RLock() across getLockTableWithCreate/getLocalLockTable and getLockHolder.
2. Or, if you prefer the retry pattern already used elsewhere, make localLockTable.getLockHolder() surface ErrLockTableBindChanged when it is closed by rebinding so the caller can reacquire and retry on the current table.
3. Add one focused test for the local race (not the remote bind-changed case already covered): acquire a local table, rebind/close it before getLockHolder, and assert the lookup retries on the new table instead of returning (false, nil).

[daviszhen]

I re-checked the latest head. The previous remote-holder blockers are fixed, but I still see one remaining local stale-bind race in the holder-lookup path.

service.GetLockHolder() and handleRemoteGetLockHolder() still do:

1. get/recreate a lock table,
2. then call l.getLockHolder(...)
   without holding bindChangeMu across those two steps.

If a bind change closes/replaces that local table in between, localLockTable.getLockHolder() returns (false, nil) for mu.closed, which is indistinguishable from “lock does not exist”. The caller therefore does not retry against the new authoritative table and can transiently report NULL / no owner even though the bind has moved and the new table may still have a holder.

You can see the pieces in the current head:

• service.handleBindChanged() swaps the table under bindChangeMu and closes the old one
• localLockTable.getLockHolder() still returns pb.WaitTxn{}, false, nil when l.mu.closed
• unlike Lock, the holder-lookup path does not hold bindChangeMu.RLock() across table acquisition + lookup

Concrete suggestions

1. Mirror the Lock path and hold bindChangeMu.RLock() across getLockTableWithCreate/getLocalLockTable and getLockHolder.
2. Or, if you prefer the retry pattern already used elsewhere, make localLockTable.getLockHolder() surface ErrLockTableBindChanged when it is closed by rebinding so the caller can reacquire and retry on the current table.
3. Add one focused test for the local race (not the remote bind-changed case already covered): acquire a local table, rebind/close it before getLockHolder, and assert the lookup retries on the new table instead of returning (false, nil).

已经按照建议修改了

[XuPeng-SH]

Re-checked the latest head. The remaining local stale-bind holder-lookup race is now fixed: GetLockHolder() holds bindChangeMu.RLock() across table acquisition and getLockHolder(...), so a rebinding-close can no longer be misreported as a clean "no holder" result. Together with the earlier remote ctx/sharding/bind-refresh fixes and the new lockservice tests, I do not see a remaining correctness issue that should keep this blocked.

[mergify]

Queued — the merge queue status continues in this comment ↓.

[mergify]

Merge Queue Status

• ✅ Entered queue — 2026-06-23 09:08 UTC · Rule: release-4.0 · triggered by rule Automatic queue on approval for release-4.0
• ✅ Checks skipped · PR is already up-to-date
• ✅ Merged — 2026-06-23 09:08 UTC · at 94b970a7b9c8fdeb9a42032c5d924fc777701567 · squash

This pull request spent 22 seconds in the queue, including 2 seconds running CI.

Required conditions to merge

• #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • add locks functions #25038
• #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • add locks functions #25038
• github-review-decision = APPROVED [🛡 GitHub branch protection]
  • add locks functions #25038
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
  • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(PESSIMISTIC)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • check-neutral = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
  • check-skipped = Matrixone Standlone CI / Multi-CN e2e BVT Test on Linux/x64(LAUNCH, PROXY)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
  • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH, PESSIMISTIC)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone CI / SCA Test on Ubuntu/x86
  • check-neutral = Matrixone CI / SCA Test on Ubuntu/x86
  • check-skipped = Matrixone CI / SCA Test on Ubuntu/x86
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • check-neutral = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
  • check-skipped = Matrixone Compose CI / multi cn e2e bvt test docker compose(Optimistic/PUSH)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • check-neutral = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
  • check-skipped = Matrixone Standlone CI / e2e BVT Test on Linux/x64(LAUNCH,Optimistic)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • check-neutral = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
  • check-skipped = Matrixone Upgrade CI / Compatibility Test With Target on Linux/x64(LAUNCH)
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone Utils CI / Coverage
  • check-neutral = Matrixone Utils CI / Coverage
  • check-skipped = Matrixone Utils CI / Coverage
• any of [🛡 GitHub branch protection]:
  • check-success = Matrixone CI / UT Test on Ubuntu/x86
  • check-neutral = Matrixone CI / UT Test on Ubuntu/x86
  • check-skipped = Matrixone CI / UT Test on Ubuntu/x86