Skip to content

When clearing shard for snapshot transfer, use temporary dummy shard#9122

Merged
generall merged 1 commit into
devfrom
snapshot-transfer-clear-shard-dummy
May 21, 2026
Merged

When clearing shard for snapshot transfer, use temporary dummy shard#9122
generall merged 1 commit into
devfrom
snapshot-transfer-clear-shard-dummy

Conversation

@timvisee
Copy link
Copy Markdown
Member

@timvisee timvisee commented May 21, 2026 •
edited
Loading

Since #8782 we clear a local shard first when it receives a snapshot based transfer.

The existing implementation completely removed the local shard from in-memory structures. That creates a state inconsistency and is observable by users and other nodes.

This resolves the problem by leaving a dummy shard in place of the cleared shard. The dummy shard is replaced once the snapshot is recovered. A random kill or restart keeps the state intact.

All Submissions:

  • My PR targets the dev branch (not master) and my branch was created from dev.
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you formatted your code locally using cargo +nightly fmt --all command prior to submission?
  3. Have you checked your code using cargo clippy --workspace --all-features command?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@timvisee timvisee added bug Something isn't working release:1.18.1 labels May 21, 2026
@timvisee timvisee marked this pull request as ready for review May 21, 2026 15:14
@timvisee timvisee requested a review from generall May 21, 2026 15:15
@timvisee timvisee mentioned this pull request May 21, 2026
Closed
6 tasks
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 21, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b46b3b0c-fcc9-4f7d-91a3-3499ae5fa964

📥 Commits

Reviewing files that changed from the base of the PR and between b7ae3e8 and 90a24dd.

📒 Files selected for processing (1)
  • lib/collection/src/shards/replica_set/snapshots.rs
📝 Walkthrough

Walkthrough

This change refactors the clear_local_for_snapshot_recovery method in snapshot recovery operations. The method now replaces the in-memory local shard with a dummy shard instance using local.replace() instead of removing it, while clearing on-disk data. This preserves the shard's configuration and replica state, ensuring the shard directory remains a valid empty shard during transfer. The dummy shard is later removed by restore_local_replica_from. Documentation has been updated to explain this new behavior and the lifecycle of the dummy shard.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: introducing a temporary dummy shard during snapshot transfer clearance instead of removing the shard entirely.
Description check ✅ Passed The description clearly explains the problem (state inconsistency from PR 8782), the solution (using a dummy shard), and its benefits (preserving state across restarts).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch snapshot-transfer-clear-shard-dummy

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.

@generall generall merged commit d5773c4 into dev May 21, 2026
15 checks passed
@generall generall deleted the snapshot-transfer-clear-shard-dummy branch May 21, 2026 16:12
generall pushed a commit that referenced this pull request May 22, 2026
@timvisee @generall
When clearing shard for snapshot, put dummy shard back (#9122)
7a52f84
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@generall generall generall approved these changes

Assignees

No one assigned

Labels

bug Something isn't working release:1.18.1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@timvisee @generall