[CodeRabbit review] upstream #10613: KVStore: ignore non-exist IStorage instance in releasePreHandledSnapshot (#10607)

[jebter]

Original upstream pull request: pingcap#10613

This is an immutable review-only mirror of the exact upstream base/head commits. Both branches add the same .coderabbit.yaml solely to enable CodeRabbit; it is review-enabling metadata and not an upstream code change. The branches and this PR must not be modified, rebased, merged, or closed.

Summary by CodeRabbit

• Bug Fixes

  • Improved snapshot cancellation handling when no external files are present.
  • Added safeguards for situations where snapshot storage is unavailable, preventing invalid cleanup attempts and improving operational stability.

• Tests

  • Added coverage for snapshot release scenarios involving empty external-file sets and unavailable storage, including SST-backed data.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: e2dfe991-a021-40a8-9739-e1086df9593d

📥 Commits

Reviewing files that changed from the base of the PR and between c716736 and 05a3461.

📒 Files selected for processing (4)

• .coderabbit.yaml
• dbms/src/Common/FailPoint.cpp
• dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp
• dbms/src/Storages/KVStore/tests/gtest_raftstore_v2.cpp

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds automatic review configuration, registers a snapshot-release failpoint, updates snapshot release handling for empty files and null storage, and adds two V2 tests for these cases.

Changes

Snapshot release handling

Layer / File(s)	Summary
Release path and failpoint wiring dbms/src/Common/FailPoint.cpp, dbms/src/Storages/KVStore/MultiRaft/PrehandleSnapshot.cpp	Registers and declares force_release_snap_meet_null_storage. Snapshot release now returns for empty external files or null storage.
Snapshot release test coverage dbms/src/Storages/KVStore/tests/gtest_raftstore_v2.cpp	Adds tests for releasing snapshots without external files and with failpoint-forced null storage.

Review configuration

Layer / File(s)	Summary
Automatic review branch scope .coderabbit.yaml	Enables automatic reviews for matching coderabbit/release-8.5-base-* branches.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 05a34

Snapshot release now safely handles empty external files and missing storage, with coverage for both paths. No actionable merge risk remains.

Sequence Diagram(s)

sequenceDiagram
  participant SnapshotReleaseCallback
  participant releasePreHandledSnapshot
  participant RegionTableStorage
  SnapshotReleaseCallback->>releasePreHandledSnapshot: trigger snapshot release
  releasePreHandledSnapshot->>releasePreHandledSnapshot: check external files
  releasePreHandledSnapshot->>RegionTableStorage: fetch storage
  RegionTableStorage-->>releasePreHandledSnapshot: storage or null
  releasePreHandledSnapshot-->>SnapshotReleaseCallback: return on empty files or null storage

Loading

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description only explains the immutable review mirror. It omits the required problem summary, issue number, change details, test checklist, side effects, documentation impact, and release note.	Complete the repository template with the problem summary, issue number, implementation details, applicable test checkbox and results, side effects, documentation impact, and release note.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the main change: KVStore ignores a non-existent IStorage instance during releasePreHandledSnapshot. The upstream review prefix does not make the title misleading.
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.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch coderabbit/release-8.5-pr-10613-config

---

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.}