Stabilize disk-persisted enum values - #1943
Merged
Merged
Conversation
…zes) RespCommand and several other enums have their numeric values persisted to disk (AOF, checkpoints, cluster config) and streamed to replicas. Because they were auto-numbered by declaration order, adding a member (even a read-only command) shifted every subsequent value, so data written by an older Garnet version could replay/recover as the wrong member on a newer version. Changes: - RespCommand: move write (mutating) commands into a dense, explicitly-numbered block (APPEND=1 .. BITOP_DIFF=120) immediately after NONE. Writes are the only persisted RespCommand values; reads/scripts/admin follow and may renumber freely. Boundary checks become positional ranges (IsReadOnly is now a branchless range check); RespCommandsInfo fast arrays re-anchor to FirstDataCommand. - Object sub-op enums (Hash/List/Set/SortedSet Operation), GarnetObjectType, NodeRole and SlotState: freeze with explicit values + append-only guidance. - GarnetObjectType: pin the custom-object-type base to a fixed 0x40 so adding a built-in type never shifts persisted custom-object ids; reserve 0xFC..0xFF for a future object-format version byte with a fail-fast read guard. - RespInputHeader: give the object sub-operation id its own header byte (256/type instead of 5 bits) by dispatching COSCAN via type==All instead of header.cmd, freeing byte 1. Header stays 3 bytes; string/unified layout is unchanged. - AOF: bump header version 3->4. Replay remaps legacy (v3) RespCommand values by name (LegacyRespCommand, fail-fast on rename) and relocates the v3 object sub-op id; a max-version guard rejects unknown-higher versions loudly. - Add PersistedEnumStabilityTests (golden values, write-block invariant, sub-op capacity, custom-range, legacy mapping). Update the add-command skill and docs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9a558bfe-f632-4abc-b95b-10d78a9c85bc
Copilot started reviewing on behalf of
Badrish Chandramouli (badrishc)
July 18, 2026 15:25
View session
Contributor
There was a problem hiding this comment.
Pull request overview
This PR stabilizes several enum numeric values that are persisted to disk (AOF/checkpoints/cluster config) or streamed to replicas, by explicitly freezing values and introducing compatibility handling for the AOF format. It also reorganizes RespCommand into a writes-first, explicitly-numbered persisted block to prevent unrelated additions from shifting persisted IDs.
Changes:
- Reordered
RespCommandto a dense, explicitly-numbered write block (APPEND = 1 .. BITOP_DIFF = 120), updated read/write classification to use explicit ranges, and re-anchored fast command-info arrays toFirstDataCommand. - Expanded object sub-operation capacity by moving
RespInputHeader’s object sub-op id into a dedicated header byte, plus AOF v3→v4 replay remapping for legacyRespCommandnumbering and legacy object sub-op packing. - Froze additional persisted enums (object sub-ops, object types, cluster roles/states), and added golden stability tests + docs/skill guidance updates.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/docs/dev/range-index-resp-api.md | Updates developer docs to reflect writes-first RespCommand layout and range-based read/write classification. |
| test/standalone/Garnet.test/PersistedEnumStabilityTests.cs | Adds golden-value tests guarding persisted enum stability and legacy v3 mapping invariants. |
| libs/server/Storage/Functions/ObjectStore/ReadMethods.cs | Routes COSCAN via header.type == All to the stored object’s Operate for scan dispatch. |
| libs/server/Resp/RespCommandsInfo.cs | Re-anchors fast lookup arrays and offsets to FirstDataCommand. |
| libs/server/Resp/Parser/RespCommand.cs | Introduces explicitly-numbered write block, updates range constants, and makes IsReadOnly a two-sided range check. |
| libs/server/Resp/Objects/SharedObjectCommands.cs | Stops overriding cmd for COSCAN; relies on header.type == All signaling instead. |
| libs/server/Objects/Types/GarnetObjectType.cs | Freezes built-in object type values, documents reserved bands, and defines reserved object-format marker range. |
| libs/server/Objects/Types/GarnetObjectSerializer.cs | Adds fail-fast guard for reserved object-format marker bytes (0xFC..0xFF). |
| libs/server/Objects/SortedSet/SortedSetObject.cs | Freezes SortedSetOperation values and documents append-only persistence. |
| libs/server/Objects/Set/SetObject.cs | Freezes SetOperation values and documents append-only persistence. |
| libs/server/Objects/List/ListObject.cs | Freezes ListOperation values and documents append-only persistence. |
| libs/server/Objects/Hash/HashObject.cs | Freezes HashOperation values and documents append-only persistence. |
| libs/server/InputHeader.cs | Refactors RespInputHeader layout: dedicated subId byte + flags-only byte; updates accessors and docs. |
| libs/server/Custom/CustomObjectCommandWrapper.cs | Raises max custom-object subcommand id to 255 due to full-byte subId. |
| libs/server/Custom/CustomObjectBase.cs | Changes COSCAN detection to header.type == All instead of header.cmd. |
| libs/server/Custom/CustomCommandManager.cs | Pins custom object type id base to fixed 0x40 (reserved built-in band ends at 0x3F). |
| libs/server/AOF/LegacyRespCommand.cs | Adds name-based v3→current RespCommand mapping table for replay compatibility. |
| libs/server/AOF/AofProcessor.cs | Adds max-version guard and legacy replay translation for v3 RespCommand + legacy object sub-op packing. |
| libs/server/AOF/AofHeader.cs | Bumps AOF header version to 4 and defines max supported version. |
| libs/cluster/Server/Worker.cs | Freezes NodeRole numeric values and documents persistence requirements. |
| libs/cluster/Server/HashSlot.cs | Freezes SlotState numeric values and documents persistence requirements. |
| .github/skills/add-garnet-command/SKILL.md | Updates command-adding guidance for writes-first, explicitly-numbered persisted write block + persisted sub-op enums. |
The sub-command id (subId) now occupies its own header byte, so each custom object type supports 256 sub-commands instead of 32. Update the registration capacity test accordingly (MYDICTGET consumes one slot, so 256 more attempts yield exactly one 'Out of registration space' failure). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9a558bfe-f632-4abc-b95b-10d78a9c85bc
When a VSIM reply during migration is the transient single-element 'Key has MOVED to ...' response, the array has length 1, so the message is at index 0. The check read index 1, throwing IndexOutOfRangeException whenever the migration window was hit (an intermittent CI flake). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9a558bfe-f632-4abc-b95b-10d78a9c85bc
Addresses PR review feedback: pinning the custom object type base to 0x40 means a custom object persisted by an older build (whose custom ids started at LastObjectType+1 = 5) now falls in the reserved built-in band. CustomDeserialize previously returned null for any type below the custom base, so such a record was silently dropped on recovery/checkpoint load (data loss with no indication). Split the guard so a type id in the reserved built-in band throws a GarnetException (consistent with the existing 0xFC..0xFF reserved-marker guard), while the pre-existing unregistered-custom-factory case still returns null. Added a regression test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9a558bfe-f632-4abc-b95b-10d78a9c85bc
kevin-montrose
approved these changes
Jul 20, 2026
Badrish Chandramouli (badrishc)
merged commit Jul 20, 2026
e267b62
into
main
315 of 317 checks passed
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.
Also writes-first RespCommand + freezes for format stability
RespCommand and several other enums have their numeric values persisted to disk (AOF, checkpoints, cluster config) and streamed to replicas. Because they were auto-numbered by declaration order, adding a member (even a read-only command) shifted every subsequent value, so data written by an older Garnet version could replay/recover as the wrong member on a newer version.
Changes: