txn_types: add shrink-only flag to shared locks#19280
txn_types: add shrink-only flag to shared locks#19280ti-chi-bot[bot] merged 2 commits intotikv:masterfrom
Conversation
Signed-off-by: zyguan <zhongyangguan@gmail.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a shrink-only flag to shared locks as part of the broader effort to support shared locks for foreign key constraints (issue #19087). The shrink-only flag prevents new shared locks from being added to a SharedLocks instance once set, enabling specific lock management patterns needed for foreign key constraint handling.
Changes:
- Added a
SharedLocksFlagsbitflags type with aSHRINK_ONLYflag to control whether new shared locks can be added - Modified
put_shared_lockmethod to returnResult<()>and fail when attempting to add locks to a shrink-only instance - Added a new
SharedLocksAreShrinkOnlyerror type with appropriate error handling and conversion logic - Updated serialization/deserialization to persist the flags field with forward compatibility support
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| components/txn_types/src/lock.rs | Core implementation of shrink-only flag including bitflags definition, SharedLocks struct modification, put_shared_lock API change, serialization/deserialization updates, and comprehensive unit tests |
| components/txn_types/src/lib.rs | New error type definition with appropriate error code mapping and cloning support |
| src/storage/mvcc/mod.rs | Error conversion logic to handle SharedLocksAreShrinkOnly as an internal error |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
components/txn_types/src/lock.rs
Outdated
| @@ -774,12 +812,15 @@ impl SharedLocks { | |||
| } | |||
|
|
|||
| #[inline] | |||
There was a problem hiding this comment.
The method signature has changed to return a Result, but there is no documentation explaining when this method can fail. Consider adding a doc comment that explains the method can return an error when attempting to add a lock to a shrink-only SharedLocks instance.
| #[inline] | |
| #[inline] | |
| /// Inserts a shared lock into this `SharedLocks` collection. | |
| /// | |
| /// # Errors | |
| /// | |
| /// Returns an error if this `SharedLocks` instance is marked as shrink-only | |
| /// and cannot accept new locks, in which case `ErrorInner::SharedLocksAreShrinkOnly` | |
| /// is returned. |
| #[inline] | ||
| pub fn is_shrink_only(&self) -> bool { | ||
| self.flags.contains(SharedLocksFlags::SHRINK_ONLY) | ||
| } | ||
|
|
||
| #[inline] |
There was a problem hiding this comment.
These public methods lack documentation. Consider adding doc comments explaining what the shrink-only flag means and its intended usage, especially since this is a public API that will be used by other parts of the codebase.
| #[inline] | |
| pub fn is_shrink_only(&self) -> bool { | |
| self.flags.contains(SharedLocksFlags::SHRINK_ONLY) | |
| } | |
| #[inline] | |
| #[inline] | |
| /// Returns whether this shared-lock set is marked as *shrink-only*. | |
| /// | |
| /// When a `SharedLocks` instance is in shrink-only mode, it is only expected | |
| /// to have entries removed (i.e. the set of held shared locks may shrink) | |
| /// and should not grow with new shared-lock entries. Callers can use this | |
| /// flag to coordinate flows where shared locks are being cleaned up or | |
| /// finalized and no new shared locks should be appended. | |
| pub fn is_shrink_only(&self) -> bool { | |
| self.flags.contains(SharedLocksFlags::SHRINK_ONLY) | |
| } | |
| #[inline] | |
| /// Marks this shared-lock set as *shrink-only*. | |
| /// | |
| /// After calling this, the `SharedLocks` value should only be modified by | |
| /// removing existing shared-lock entries, not by inserting new ones. This | |
| /// is typically used in code paths that are shrinking or cleaning up the | |
| /// shared locks and want to prevent further expansion of the lock set. |
Signed-off-by: zyguan <zhongyangguan@gmail.com>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cfzjywxk, you06 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ref tikv#19087 Introduce shrink-only flag to shared locks. Signed-off-by: zyguan <zhongyangguan@gmail.com>
|
In response to a cherrypick label: new pull request created to branch |
What is changed and how it works?
Issue Number: ref #19087
What's Changed:
Related changes
pingcap/docs/pingcap/docs-cn:Check List
Tests
Side effects
Release note