Skip to content

Conversation

@you06
Copy link

@you06 you06 commented Jan 19, 2026

What problem does this PR solve?

Issue Number: ref tikv/tikv#19087

Problem Summary:

What is changed and how it works?

This PR support parsing the TiKV's shared lock and bypass them in conflict check.

Check List

Tests

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

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

Release Notes

  • New Features

    • Added support for shared locks in the KVStore layer, enhancing concurrent transaction management and lock isolation capabilities.
  • Tests

    • Expanded test coverage to validate detection and handling of the new shared lock type.

✏️ Tip: You can customize this high-level summary in your review settings.

Signed-off-by: you06 <you1474600@gmail.com>
@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. labels Jan 19, 2026
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 19, 2026

[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 calvinneo 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:

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 ti-chi-bot bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jan 19, 2026
@pingcap-cla-assistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

📝 Walkthrough

Walkthrough

This change adds support for a new "Shared" lock type to the TiKV storage layer. It introduces a new Shared = 'H' enumerator to the LockType enum, updates lock decoding and retrieval logic to handle the new type, adds test coverage, and updates the kvproto submodule reference.

Changes

Cohort / File(s) Summary
Lock Type Definition
dbms/src/Storages/KVStore/TiKVHelpers/TiKVRecordFormat.h
Added new enumerator Shared = 'H' to the LockType enum in the DB::RecordKVFormat namespace
Lock Handling Logic
dbms/src/Storages/KVStore/TiKVHelpers/DecodedLockCFValue.cpp
Updated decodeLockCfValue to recognize and early-return for SharedLock; broadened Inner::getLockInfoPtr early-return condition to treat SharedLock same as Lock and PessimisticLock (returns nullptr)
Test Coverage
dbms/src/Storages/KVStore/tests/gtest_kvstore.cpp
Added test block in RegionKVStoreOldTest::RegionReadWrite that inserts a SharedLock entry, creates a committed scanner, and verifies lock info returns nullptr
Submodule Reference
contrib/kvproto
Updated kvproto submodule reference from commit db74bf0 to commit 6db24b6

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A SharedLock hops into the fray,
New enum value joins the play,
'H' marks it in the KV store,
Decoded swift, returns as before,
Tests confirm it's working right! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'support bypassing shared lock' accurately reflects the main objective of the PR, which adds support for parsing and bypassing TiKV's shared locks in conflict checks.
Description check ✅ Passed The PR description follows the required template with all main sections completed: problem statement, changes explained, tests checked (unit test), side effects assessed, and release note provided.

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

✨ Finishing touches
  • 📝 Generate docstrings

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jan 19, 2026

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-linked-issue label, please provide the linked issue number on one line in the PR body, for example: Issue Number: close #123 or Issue Number: ref #456.

📖 For more info, you can check the "Contribute Code" section in the development guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/needs-linked-issue release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant