Repository navigation
[Objects] Do not create the key when an object RMW leaves the new object empty - #2194
Merged
Badrish Chandramouli (badrishc) merged 1 commit intoOct 2, 2026
Conversation
Federico Laurianti (Laurianti)
force-pushed
the
fix-zadd-xx-missing-key
branch
from
October 2, 2026 03:59
353398c to
5eee8ae
Compare
Federico Laurianti (Laurianti)
marked this pull request as ready for review
October 2, 2026 03:59
Copilot started reviewing on behalf of
Federico Laurianti (Laurianti)
October 2, 2026 04:00
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
A reused ObjectOutput can retain a stale RemoveKey flag and incorrectly reject a later valid object creation.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Prevents empty object keys from being created when an initial RMW leaves the collection empty.
Changes:
- Rejects initial object creation when
RemoveKeyis signaled. - Adds coverage for
ZADD XXandXX CHon missing keys.
| File | Description |
|---|---|
libs/server/Storage/Functions/ObjectStore/RMWMethods.cs |
Skips creation of empty objects. |
test/standalone/Garnet.test.collections/RespSortedSetTests.cs |
Tests missing-key ZADD XX behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Badrish Chandramouli (badrishc)
approved these changes
Oct 2, 2026
Badrish Chandramouli (badrishc)
merged commit Oct 2, 2026
c465175
into
microsoft:main
169 checks passed
x@01 (x-at-01)
added a commit
to webc-fork/garnet
that referenced
this pull request
Oct 4, 2026
…n missing key (microsoft#2192), ZADD XX INCR null (microsoft#2197), no empty object from object RMW (microsoft#2194), fresh ObjectOutput per key HCOLLECT/ZCOLLECT (microsoft#2200), CompletionEvent disposal + LogSizeTracker coalesce (microsoft#2198), dynamic test ports (microsoft#2193)
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.
Root Cause
ZADDwithXXon a missing key creates an empty sorted set:ZADDmust create the object in general, soGarnetObject.NeedToCreatereturns true, andInitialUpdaterstores the new object even thoughXXleft it empty. The key then shows up inEXISTS,TYPEandDBSIZE.Description of Change
libs/server/Storage/Functions/ObjectStore/RMWMethods.cs: inInitialUpdater, afterOperate, if the object signalsRemoveKey(it is empty) the record is not created andInitialUpdaterreturns false. This is the flagInPlaceUpdaterandCopyUpdateralready use to remove a collection that becomes empty, and the same false return the main store'sInitialUpdateruses forSETandPFADD.The reply written by
Operateis kept, soZADD XXstill validates its options and replies 0 as before. Any other operation that would leave a new object empty gets the same treatment.Tests
RespSortedSetTests.AddWithXXOnMissingKeyDoesNotCreateKey: afterZADD XXandZADD XX CHon a missing key, the key does not exist. It fails without the change.AddWithOptionsErrorConditions(option errors on a missing key) still passes. On currentmainwith this change, net10.0:Garnet.test.collections783 passed,Garnet.test1291 passed (9 skipped).Issues Fixed
Fixes #2191