Repository navigation
fix: prevent delayed CAS deletes from lowering fences - #284
Open
perfloop[bot] wants to merge 3 commits into
Open
perfloop[bot] wants to merge 3 commits into
perfloop[bot] wants to merge 3 commits into
Conversation
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
|
Signed-off-by: Perfloop Agent <agent@perfloop.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A delayed CAS delete could replace a newer absent-key fence when its merge arrived later than its stamp. The merge operator now keeps the newer tombstone, so an ordered refill based on the earlier absence token is rejected.
The repair covers CAS-only operations on one Storage instance while the tombstone remains retained. Plain/CAS mixing, owner changes, restarts, and the existing fence-retention sweep remain outside this guarantee. The API and stored schema are unchanged.
The storage test sets a one-hour fence retention in
StorageConfigbefore construction starts the cleaner, pauses the earlier delete before its merge, completes the later invalidation, and attempts an inline refill with the intervening token. Merge tests compare all-at-once and successive-base folding; existing tests retain fresh-token, unordered-refill, and advancing-delete behavior. A package-only metadata hook coordinates the pause; it adds no public surface.satisfied
violated
07d0dd86ad1fd6d5dbffc694fbdff9ae33c4b449The runner used for the recorded source comparison is not part of the pull request; the deterministic regression test is. The targeted storage test, full storage suite, targeted race run, CAS integration suite, CAS E2E, lint, and license check passed. The CI's jemalloc-linked CAS E2E variant and the full CI workflow were not run here.
Generated by Perfloop.
Note
Medium Risk
Changes CAS tombstone merge semantics for concurrent/out-of-order deletes, which is correctness-critical for cache invalidation ordering though scoped to the existing fence model and covered by new tests.
Overview
Fixes a CAS fencing bug where a delete whose stamp was issued earlier but merged later could replace a newer absent-key tombstone and wrongly admit a refill that had observed the gap between the two invalidations.
The merge operator now only advances the fence on a repeat delete over a tombstone when
op.Version >= base.Version, so a delayed older delete keeps the newer fence and surfaces a version mismatch instead of lowering it.DeleteIfVersioninvokes the existing test-onlybeforeMetaCommithook before the metadata merge (same as puts), enabling a storage-level regression that pauses the first delete, commits a second invalidation, then rejects a stalePutIfVersion. Test helpers accept optionalStorageConfigtweaks (e.g. fence retention); merge tests assert all-at-once vs split folding still match.Reviewed by Cursor Bugbot for commit dc31114. Bugbot is set up for automated code reviews on this repo. Configure here.