fix(storages): reject shared locks at disaggregated resolver

[wfxr]

What problem does this PR solve?

Issue Number: ref #10902 ref #10704

Problem Summary:

TiFlash read paths should treat shared locks as read-compatible and skip them before reporting locked errors. If a shared-lock wrapper reaches the disaggregated lock resolver, resolving it would hide an upstream invariant violation and could interpret invalid wrapper transaction fields as a real lock.

What is changed and how it works?

fix(storages): reject shared locks at disaggregated resolver

TiFlash read paths should skip shared locks before emitting locked errors. Treat a shared-lock wrapper at the disaggregated resolver as an invariant violation so it is not retried or resolved with invalid wrapper txn fields.

Keep ordinary lock conversion covered so existing lock-resolution behavior remains intact.

This PR documents the shared-lock test coverage design, rejects shared-lock wrappers at the two disaggregated lock resolver boundaries, and adds unit coverage for both ordinary lock conversion and shared-lock wrapper rejection.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Unit test:

release-linux-llvm/build-unit-tests-shared-lock/dbms/gtests_dbms --gtest_filter=StorageDisaggregatedHelpersTest.MakeLocksForDisaggResolve*'

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

None

Summary by CodeRabbit

Summary

• Refactor

  • Standardized conversion of disaggregated lock information into lock objects used during lock resolution.

• Bug Fixes

  • Improved locked-error handling for disaggregated reads with clearer “locked error” wording while retaining the same lock details for debugging.

• Tests

  • Added unit tests covering lock conversion behavior (key/primary/transaction/TTL/lock type preservation) and verification that unsupported shared-lock inputs are rejected with a logical error.

[pingcap-cla-assistant]

All committers have signed the CLA.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Two inline helpers (makeLockForDisaggResolve, makeLocksForDisaggResolve) are added to StorageDisaggregatedHelpers.h to convert kvrpcpb::LockInfo protobuf objects into pingcap::kv::LockPtr objects, throwing LOGICAL_ERROR on SharedLock wrappers. Both disaggregated LockedError call sites in the remote and columnar paths are updated to use these helpers, and two unit tests verify the field-preservation and rejection behaviors.

Changes

Disaggregated Lock Resolution Guard

Layer / File(s)	Summary
Lock-conversion helpers with SharedLock guard dbms/src/Storages/StorageDisaggregatedHelpers.h	Adds includes for kvrpcpb.pb.h, pingcap/kv/LockResolver.h, Common/RedactHelpers.h, and standard library headers. Introduces makeLockForDisaggResolve that throws LOGICAL_ERROR on SharedLock wrappers and constructs pingcap::kv::Lock otherwise; makeLocksForDisaggResolve batches conversion over a RepeatedPtrField<LockInfo>.
Call-site updates in remote and columnar paths dbms/src/Storages/StorageDisaggregatedRemote.cpp, dbms/src/Storages/StorageDisaggregatedColumnar.cpp	StorageDisaggregatedRemote.cpp replaces manual lock-vector construction with makeLocksForDisaggResolve and updates log wording from "retryable error" to "locked error"; StorageDisaggregatedColumnar.cpp replaces std::make_shared<pingcap::kv::Lock>(lock_info) with makeLockForDisaggResolve.
Unit tests for lock-conversion helpers dbms/src/Storages/tests/gtest_disagg_remote.cpp	Adds include for kvproto/kvrpcpb.pb.h. Implements two StorageDisaggregatedHelpersTest cases: one verifies field preservation (key, primary, txn_id, ttl, lock_type) for a normal LockInfo, and one verifies LOGICAL_ERROR with "Unexpected SharedLock" message for a SharedLock-typed LockInfo.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Harden shared-lock handling at disaggregated resolver boundary #10902: This PR directly implements the defensive hardening requirement by adding makeLockForDisaggResolve and makeLocksForDisaggResolve helpers that explicitly reject SharedLock types with LOGICAL_ERROR at the disaggregated resolver boundary.

Suggested reviewers

• JinheLin
• Lloyd-Pottiger
• yongman

Poem

🐇 A SharedLock hops in, the code cries "No way!"
With LOGICAL_ERROR I send it away.
Two helpers now guard the lock-resolving gate,
Converting each LockInfo — none arrive late.
The tests hop in circles confirming it's right,
My burrow is safe through the disaggregated night! 🌙

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: rejecting shared locks at the disaggregated resolver, which is the core objective of this PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description check	✅ Passed	The PR description is comprehensive and follows the required template structure with all major sections completed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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 and usage tips.}

[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 `@docs/superpowers/specs/2026-06-15-enhance-shared-lock-tests-design.md`:
- Around line 17-18: The worktree paths in the TiFlash and Cloud Storage Engine
lines contain a spelling error where "enchance-shared-lock-tests" should be
"enhance-shared-lock-tests" (missing the 'a'). Correct this typo in both
worktree path lines to align with the correct specification naming and ensure
readers are directed to the proper paths.

🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9e88cf67-3f43-455d-8241-b8cb2ae136e8

📥 Commits

Reviewing files that changed from the base of the PR and between 50e6dea and 848420d.

📒 Files selected for processing (5)

• dbms/src/Storages/StorageDisaggregatedColumnar.cpp
• dbms/src/Storages/StorageDisaggregatedHelpers.h
• dbms/src/Storages/StorageDisaggregatedRemote.cpp
• dbms/src/Storages/tests/gtest_disagg_remote.cpp
• docs/superpowers/specs/2026-06-15-enhance-shared-lock-tests-design.md

[wfxr]

/retest

[wfxr]

/retest

[wfxr]

/retest

[JaySon-Huang]

/test pull-unit-test

[JaySon-Huang]

LGTM

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JaySon-Huang, yongman

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 [JaySon-Huang,yongman]

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-06-16 06:35:06.740228636 +0000 UTC m=+1460207.810546026: ☑️ agreed by JaySon-Huang.
• 2026-06-16 06:47:55.25986837 +0000 UTC m=+1460976.330185760: ☑️ agreed by yongman.

[wfxr]

/retest