Skip to content

Skip Value Expiration Check When Scanning a Tombstoned Record#1612

Open
vazois wants to merge 12 commits intomainfrom
vazois/migrate-fix
Open

Skip Value Expiration Check When Scanning a Tombstoned Record#1612
vazois wants to merge 12 commits intomainfrom
vazois/migrate-fix

Conversation

@vazois
Copy link
Contributor

@vazois vazois commented Mar 6, 2026

This PR attempts to fix #1560. It also includes the following:

  • Skip expiration check when SingleReader pushes a tombstoned record.
    This is safe since expiration validation will occur when reading the actual value during TransmitKeys. If the key has expired at that point, it will be skipped.
  • Ensure pause reviv signal is observed under epoch protection.
  • Fix ClusterMigrateTest.
    Use GarnetClientSession to ensure the correct redirection messages are generated.
  • Release epoch when acquiring SuspendConfigMerge lock to prevent deadlock/livelock.

@vazois vazois force-pushed the vazois/migrate-fix branch from 423b177 to d6a7896 Compare March 6, 2026 05:28
@vazois vazois marked this pull request as ready for review March 6, 2026 05:28
Copilot AI review requested due to automatic review settings March 6, 2026 05:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes cluster slot migration failures when scans include tombstoned records (issue #1560) by avoiding expiration checks on tombstones, strengthening revivification pausing semantics, and updating/adding cluster migration tests to validate behavior.

Changes:

  • Skip expiration validation in migration scan readers when the scanned record is a tombstone.
  • Update TsavoriteKV.PauseRevivification() to coordinate pause visibility via epoch bump + signaling.
  • Fix and extend cluster migration tests, including a new tombstone-focused migration test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
test/Garnet.test.cluster/ClusterMigrateTests.cs Adjusts migrate test flow/expectations and adds a new tombstone migration test.
libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Tsavorite.cs Implements epoch-coordinated revivification pause signaling and disposes the new event.
libs/cluster/Server/Migration/MigrateScanFunctions.cs Skips expiration checks for tombstone records during scan to avoid null/value issues.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

vazois and others added 2 commits March 5, 2026 21:35
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@vazois vazois requested a review from badrishc March 10, 2026 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants