Simplify etags [dev]#1739
Merged
Merged
Conversation
Add new SETWITHETAG command as a dedicated top-level command for setting key-value pairs with ETags. This replaces the SET ... WITHETAG option pattern and is the first step in corralling ETag semantics to dedicated commands only. Changes: - Add RespCommand.SETWITHETAG enum value - Add SlowParseCommand parsing and CmdStrings constant - Add NetworkSETWITHETAG handler in BasicEtagCommands.cs - Add dispatch in RespServerSession.ProcessArrayCommands - Add RMW callbacks (NeedInitialUpdate, InitialUpdater, InPlaceUpdater, NeedCopyUpdate, CopyUpdater, PostInitialUpdater) - Add VarLenInputMethods cases (GetRMWInitialFieldInfo, GetRMWModifiedFieldInfo) with HasETag=true - Add SupportedCommand.cs entry - Add GarnetCommandsInfo.json and GarnetCommandsDocs.json entries - Add generated resource JSON entries - Add SetWithEtagACLsAsync ACL test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove WITHETAG option from SET and RENAME/RENAMENX commands. Make all non-ETag commands completely ETag-blind — they do not read, check, update, or remove ETags. Phase 2: Remove WITHETAG from SET - Remove WITHETAG parsing from NetworkSETEXNX - Remove EtagOption enum entirely - SET paths always pass withEtag: false Phase 3: Remove WITHETAG from RENAME/RENAMENX - Remove withEtag parameter from API signatures - Simplify RENAME to strictly 2-arg commands Phase 4: Make RMW methods ETag-blind - Remove hadETagPreMutation/shouldUpdateEtag for non-ETag commands - ETag state only initialized for SETIFMATCH/SETIFGREATER/SETWITHETAG/DELIFGREATER - Remove ETag logic from SET/SETKEEPTTL/SETEXNX InitialUpdater/InPlaceUpdater/CopyUpdater - Remove custom command ETag rejection - Reader only sets ETag state for GETWITHETAG/GETIFNOTMATCH Phase 5: Make UpsertMethods ETag-blind - MainStore/ObjectStore/UnifiedStore UpsertMethods pass false for inputHasETag Phase 6: Make UnifiedStore ETag-blind - Remove shouldUpdateEtag pattern from UnifiedStore RMW/InPlaceUpdater - Clean up VarLenInputMethods to not use CheckWithETagFlag Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove dead ETag constants and error strings: - Remove WITHETAG CmdString (no longer parsed by any command) - Remove RESP_ERR_ETAG_ON_CUSTOM_PROC error string - Remove RESP_ERR_WITHETAG_AND_GETVALUE error string Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace all SET...WITHETAG with SETWITHETAG in tests and samples - Remove tests for removed features: SET WITHETAG NX/XX/KEEPTTL, RENAME WITHETAG, custom command ETag rejection - Remove tests verifying ETag auto-increment on non-ETag commands (APPEND, INCR, DECR, SETRANGE, SETBIT, BITFIELD) - Remove tests verifying SET strips ETags (undefined behavior now) - Fix ETag state initialization for ETag commands on keys without ETags - Update samples/ETag Caching.cs and OccSimulation.cs All 83 ETag tests pass. All 345 RespTests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace SET (WITHETAG) with SETWITHETAG in garnet-specific.md - Rewrite 'Compatibility with Non-ETag Commands' section with prominent key partitioning warning and usage examples - Remove WITHETAG from SET syntax in raw-string.md - Remove WITHETAG from RENAME/RENAMENX in generic-commands.md - Update blog post intro to reference dedicated ETag command set Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…elpers (Phases 6b, 7) Phase 6b: Remove dead ETag weight from Tsavorite - Remove ETag field from RecordMetadata struct (never consumed) - Remove eTag field from PendingContext - Remove ~20 pendingContext.eTag assignments from InternalRead/RMW/Upsert/Delete - Simplify RecordMetadata construction in AllocatorScan, TsavoriteIterator, TsavoriteThread, CompletedOutput, Tsavorite.cs Phase 7: Extract ETag RMW logic to NoInline helpers - Create RMWMethods.Etags.cs with 9 [NoInlining] helper methods - Each ETag case in InPlaceUpdaterWorker/InitialUpdater/CopyUpdater/NeedCopyUpdate becomes a single-line call, keeping hot-path methods compact for JIT - Helpers: HandleDelIfGreater*, HandleSetIfMatch*, HandleSetWithEtag* for InPlaceUpdate, InitialUpdate, CopyUpdate, and NeedCopyUpdate Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move GETWITHETAG and GETIFNOTMATCH handling from Reader into a [NoInlining] HandleEtagReader helper in ReadMethods.Etags.cs. ETag commands are now delegated at the beginning of Reader before any other processing, keeping the hot-path Reader method compact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
All callers pass false — remove the parameter entirely from InPlaceWriterForSpanValue, InPlaceWriterForHeapObjectValue, InPlaceWriterForLogRecordValue, and rename UpdateExpirationAndETag to UpdateExpiration (ETag logic removed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace functionsState.etagState with local long existingEtag passed to each handler. Consolidate ETag command dispatch into single HandleEtagInPlaceUpdateWorker / HandleEtagCopyUpdateWorker / HandleEtagNeedCopyUpdate dispatchers that read the ETag once and delegate to individual NoInline helpers. - Remove ETagState struct and EtagState.cs entirely - Remove etagState field from FunctionsState - Remove PostInitialUpdater ETag reset - CopyRespWithEtagData now takes long etag directly - Remove isEtagCommand/shouldUpdateEtag variables from hot paths - All ETag logic is now stateless — no shared mutable state Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace separate if-checks before switches with cases inside the switch for ETag commands. This lets the JIT generate a single jump table covering both ETag and non-ETag commands, eliminating an extra branch on the hot path. - InPlaceUpdaterWorker: ETag cases in the switch - CopyUpdater: ETag cases in the switch - Reader: restructured to use switch with ETag cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The withEtag parameter is dead — the flag it sets (SetWithETagFlag) is never read by any RMW callback. RMW methods dispatch on the RespCommand enum value instead. Replace the parameter with isEtagCommand derived from the cmd inside the method. Also remove the dead SetWithETagFlag() call from DELIFGREATER handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add SET_ETagConditional and DEL_ETagConditional to IGarnetApi/GarnetApi as dedicated ETag API methods, separate from SET_Conditional - Split NetworkSET_Conditional (non-ETag only) from ExecuteETagSetCommand (shared by SETWITHETAG/SETIFMATCH/SETIFGREATER) - Remove SetWithETagFlag() and CheckWithETagFlag() from RespInputHeader - Remove RespInputFlags.WithEtag enum value - Remove stale ETag comments from ObjectStore VarLenInputMethods - Custom procedure test uses SET_ETagConditional with SETWITHETAG Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
SETWITHETAG without EX/PX now clears any existing expiration, matching SET semantics. Previously it would preserve the old TTL. - InPlaceUpdate: remove expiration when input.arg1 == 0 - CopyUpdate: don't carry forward srcLogRecord.Expiration - VarLenInputMethods: SETWITHETAG doesn't preserve source expiration (split from SETIFMATCH/SETIFGREATER which do preserve) - Fix trailing newline in RMWMethods.Etags.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…mmands Change GetRMWModifiedFieldInfo default from HasETag=srcLogRecord.Info.HasETag to HasETag=false. Non-ETag commands (INCR, APPEND, SETRANGE, etc.) no longer allocate ETag space in copy-updated records. ETag commands explicitly set HasETag=true in their own switch cases. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Garnet’s ETag support to be isolated to a small set of dedicated ETag commands (introducing SETWITHETAG) and removes ETag handling/overhead from all other command hot paths, alongside updating docs, tests, samples, and command metadata accordingly.
Changes:
- Introduces
SETWITHETAGas the only way to initialize/update string keys with ETags (removingWITHETAGoptions fromSET,RENAME,RENAMENX). - Removes Tsavorite/server-side ETag plumbing from non-ETag operations (e.g., removes
ETagState, removesRecordMetadata.ETag, removes custom-command ETag rejection). - Updates docs, blog post, samples, and test suite to use the new command and new “ETag-blind non-ETag commands” contract.
Reviewed changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/commands/raw-string.md | Removes WITHETAG option documentation from SET. |
| website/docs/commands/generic-commands.md | Removes WITHETAG option documentation from RENAME/RENAMENX. |
| website/docs/commands/garnet-specific.md | Updates native ETag docs to SETWITHETAG and adds key-partitioning warning/usage examples. |
| website/blog/2025-01-18-etag-when-and-how.md | Updates blog intro to reflect dedicated ETag commands + key partitioning requirement. |
| test/Garnet.test/TransactionTests.cs | Migrates transaction/watch tests to SETWITHETAG. |
| test/Garnet.test/RespTests.cs | Updates rename-related tests to seed via SETWITHETAG; removes rename-with-etag semantics assertions. |
| test/Garnet.test/RespRevivificationTests.cs | Renames a test to reflect removal of ETag-state behavior. |
| test/Garnet.test/RespEtagTests.cs | Migrates ETag tests to SETWITHETAG and removes tests asserting internal ETag updates from non-ETag commands. |
| test/Garnet.test/RespCustomCommandTests.cs | Updates custom command/proc tests to use SET_ETagConditional/SETWITHETAG and removes expectations of non-ETag commands updating ETags. |
| test/Garnet.test/RespAofTests.cs | Updates AOF test seeding to use SETWITHETAG. |
| test/Garnet.test/Resp/ACL/RespCommandTests.cs | Adds ACL coverage test for SETWITHETAG. |
| samples/ETag/OccSimulation.cs | Updates sample usage from SET ... WITHETAG to SETWITHETAG. |
| samples/ETag/Caching.cs | Updates sample usage from SET ... WITHETAG to SETWITHETAG. |
| playground/CommandInfoUpdater/SupportedCommand.cs | Registers SETWITHETAG in the command info updater. |
| playground/CommandInfoUpdater/GarnetCommandsInfo.json | Adds command metadata for SETWITHETAG. |
| playground/CommandInfoUpdater/GarnetCommandsDocs.json | Adds docs metadata for SETWITHETAG. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/TsavoriteThread.cs | Removes RecordMetadata construction with ETag. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/TsavoriteIterator.cs | Removes ETag from iterator reader metadata. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Tsavorite.cs | Removes ETag from context-returned RecordMetadata. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/InternalUpsert.cs | Removes pending-context ETag tracking on hot paths. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/InternalRead.cs | Removes pending-context ETag tracking on hot paths. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/InternalRMW.cs | Removes pending-context ETag tracking on hot paths. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/InternalDelete.cs | Removes pending-context ETag tracking on hot paths. |
| libs/storage/Tsavorite/cs/src/core/Index/Tsavorite/Implementation/ContinuePending.cs | Removes disk-read ETag propagation into pending context. |
| libs/storage/Tsavorite/cs/src/core/Index/Common/RecordMetadata.cs | Removes RecordMetadata.ETag. |
| libs/storage/Tsavorite/cs/src/core/Index/Common/PendingContext.cs | Removes PendingContext.eTag. |
| libs/storage/Tsavorite/cs/src/core/Index/Common/CompletedOutput.cs | Removes ETag from completed output metadata. |
| libs/storage/Tsavorite/cs/src/core/Allocator/AllocatorScan.cs | Removes ETag from scan reader metadata. |
| libs/server/Storage/Session/UnifiedStore/UnifiedStoreOps.cs | Removes RENAME/RENAMENX “withEtag” plumbing and related record inspection. |
| libs/server/Storage/Functions/UnifiedStore/VarLenInputMethods.cs | Makes unified-store field info ETag-blind for non-ETag commands. |
| libs/server/Storage/Functions/UnifiedStore/UpsertMethods.cs | Removes ETag-setting logic from unified-store upsert writer paths. |
| libs/server/Storage/Functions/UnifiedStore/RMWMethods.cs | Removes transparent ETag updating from unified-store RMW hot paths. |
| libs/server/Storage/Functions/SessionFunctionsUtils.cs | Removes ETag mutation from shared in-place writer helpers. |
| libs/server/Storage/Functions/ObjectStore/VarLenInputMethods.cs | Cleans up TODO comments and confirms no ETag support in object store. |
| libs/server/Storage/Functions/ObjectStore/UpsertMethods.cs | Removes ETag flag flow into shared writers for object store. |
| libs/server/Storage/Functions/MainStore/VarLenInputMethods.cs | Adds SETWITHETAG sizing rules; makes non-ETag ops ETag-blind. |
| libs/server/Storage/Functions/MainStore/UpsertMethods.cs | Removes ETag flag flow into shared writers for main store upsert paths. |
| libs/server/Storage/Functions/MainStore/ReadMethods.cs | Delegates ETag reads to a separate NoInlining file; removes custom-command ETag rejection. |
| libs/server/Storage/Functions/MainStore/ReadMethods.Etags.cs | Adds dedicated handler for GETWITHETAG/GETIFNOTMATCH. |
| libs/server/Storage/Functions/MainStore/RMWMethods.cs | Routes ETag commands to ETag-only dispatcher/helpers. |
| libs/server/Storage/Functions/MainStore/RMWMethods.Etags.cs | Adds NoInlining ETag-only RMW helpers (SETWITHETAG, SETIF*, DELIFGREATER). |
| libs/server/Storage/Functions/MainStore/PrivateMethods.cs | Changes ETag response helper to accept explicit etag value. |
| libs/server/Storage/Functions/FunctionsState.cs | Removes ETagState from shared function state. |
| libs/server/Storage/Functions/EtagState.cs | Deletes ETagState implementation. |
| libs/server/Resp/RespServerSession.cs | Wires SETWITHETAG into command dispatch. |
| libs/server/Resp/RespEnums.cs | Removes EtagOption enum. |
| libs/server/Resp/Parser/RespCommand.cs | Adds SETWITHETAG to enum and parser. |
| libs/server/Resp/KeyAdminCommands.cs | Removes WITHETAG parsing for RENAME/RENAMENX. |
| libs/server/Resp/CmdStrings.cs | Removes WITHETAG token/errors; adds SETWITHETAG token. |
| libs/server/Resp/BasicEtagCommands.cs | Adds SETWITHETAG network handler; routes ETag commands via new ETag-aware API methods. |
| libs/server/Resp/BasicCommands.cs | Removes WITHETAG option parsing from SET variants; simplifies SET conditional path. |
| libs/server/InputHeader.cs | Removes WithEtag input flag and related helpers. |
| libs/server/API/IGarnetApi.cs | Adds SET_ETagConditional/DEL_ETagConditional; removes withEtag from rename APIs. |
| libs/server/API/GarnetApi.cs | Implements new ETag-aware API entrypoints and updated rename signatures. |
| libs/resources/RespCommandsInfo.json | Adds SETWITHETAG command metadata. |
| libs/resources/RespCommandsDocs.json | Adds SETWITHETAG documentation metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix SETWITHETAG CopyUpdate to clear TTL when no EX/PX (match SET semantics) - Fix stale comment in PrivateMethods.cs about ETag value layout - Update doc wording: 'do not preserve' instead of 'do not remove' - Add regression test: SetWithEtagClearsTTLWhenNoExpiryProvided - Keep using Tsavorite.core (needed for PinnedSpanByte in ExecuteETagSetCommand) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.
Summary
Confines ETag semantics to 6 dedicated commands and makes all other commands completely ETag-blind, eliminating ETag overhead from non-ETag hot paths.
ETag commands (the only commands that interact with ETags):
SET ... WITHETAGWhat changed:
New command:
SETWITHETAG key value [EX seconds | PX milliseconds]— sets a key-value pair with an ETag. Returns the ETag as an integer.Removed features:
SET ... WITHETAGoption removed — useSETWITHETAGinsteadRENAME/RENAMENX ... WITHETAGoption removed entirelyEtagOptionenum removedPerformance improvements:
HasETag, manageETagState, or branch onshouldUpdateEtagRecordMetadata.ETagfield andPendingContext.eTagfrom Tsavorite — eliminates ~20srcLogRecord.ETagreads per operation on every Internal* hot pathRMWMethods.Etags.cswith[NoInlining]helpers, keeping hot-path methods compact for JITKey design decision: Non-ETag commands are completely ETag-blind. Users must partition their keys — use only ETag commands on ETag-managed keys. Mixing ETag and non-ETag commands on the same key results in undefined ETag behavior. This is by design for maximum performance.
Files changed: 48 (+757, -1494)
Test results