From 021f5c0f25631fe84061991e105ea68160d25b0c Mon Sep 17 00:00:00 2001 From: Hamdaan Khalid <42720645+hamdaankhalid@users.noreply.github.com> Date: Fri, 21 Feb 2025 15:01:52 -0800 Subject: [PATCH] Add SETIFGREATER ETag command (#968) * init impl SETIFGREATER * Redis documentation and tests * Fix issue in RCU route * FMT * Txn and acl test * emphasize link to raw string set * Update command docs * PR feedback * Add NOGET to SETIFMATCH and SETIFGREATRER options * add ability to add key value at fist insertion * Update docs * update test * Rename base etag to no etag --------- Co-authored-by: Hamdaan Khalid --- libs/resources/RespCommandsDocs.json | 66 ++++++++- libs/resources/RespCommandsInfo.json | 24 ++++ libs/server/Resp/BasicCommands.cs | 4 +- libs/server/Resp/BasicEtagCommands.cs | 88 ++++++++++-- libs/server/Resp/CmdStrings.cs | 4 + libs/server/Resp/Parser/RespCommand.cs | 5 + libs/server/Resp/RespServerSession.cs | 1 + libs/server/Storage/Functions/EtagState.cs | 6 +- .../Functions/MainStore/PrivateMethods.cs | 2 +- .../Storage/Functions/MainStore/RMWMethods.cs | 135 ++++++++++++++---- .../Functions/MainStore/ReadMethods.cs | 2 +- .../Functions/MainStore/VarLenInputMethods.cs | 1 + libs/server/Transaction/TxnKeyManager.cs | 1 + .../GarnetCommandsDocs.json | 68 ++++++++- .../GarnetCommandsInfo.json | 26 ++++ .../CommandInfoUpdater/SupportedCommand.cs | 1 + test/Garnet.test/Resp/ACL/RespCommandTests.cs | 19 ++- test/Garnet.test/RespEtagTests.cs | 108 +++++++++++++- website/docs/commands/garnet-specific.md | 57 +++++++- 19 files changed, 564 insertions(+), 54 deletions(-) diff --git a/libs/resources/RespCommandsDocs.json b/libs/resources/RespCommandsDocs.json index 9e217fb06f..a363760349 100644 --- a/libs/resources/RespCommandsDocs.json +++ b/libs/resources/RespCommandsDocs.json @@ -6237,10 +6237,67 @@ } ] }, + { + "Command": "SETIFGREATER", + "Name": "SETIFGREATER", + "Summary": "Sets a key value pair with the given etag only if (1) the etag given in the request is greater than the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was greater than 0.", + "Group": "String", + "Complexity": "O(1)", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandKeyArgument", + "Name": "KEY", + "DisplayText": "key", + "Type": "Key", + "KeySpecIndex": 0 + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "VALUE", + "DisplayText": "value", + "Type": "String" + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "ETAG", + "DisplayText": "etag", + "Type": "Integer" + }, + { + "TypeDiscriminator": "RespCommandContainerArgument", + "Name": "EXPIRATION", + "Type": "OneOf", + "ArgumentFlags": "Optional", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "SECONDS", + "DisplayText": "seconds", + "Type": "Integer", + "Token": "EX" + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "MILLISECONDS", + "DisplayText": "milliseconds", + "Type": "Integer", + "Token": "PX" + } + ] + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "NOGET", + "DisplayText": "noget", + "Type": "PureToken", + "Token": "NOGET" + } + ] + }, { "Command": "SETIFMATCH", "Name": "SETIFMATCH", - "Summary": "Sets the string value of a key, ignoring its type, if the key\u0027s current etag matches the given etag.", + "Summary": "Sets a key value pair with the given etag only if (1) the etag given in the request matches the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was 0.", "Group": "String", "Complexity": "O(1)", "Arguments": [ @@ -6284,6 +6341,13 @@ "Token": "PX" } ] + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "NOGET", + "DisplayText": "noget", + "Type": "PureToken", + "Token": "NOGET" } ] }, diff --git a/libs/resources/RespCommandsInfo.json b/libs/resources/RespCommandsInfo.json index d74adfda93..bf08bc69fd 100644 --- a/libs/resources/RespCommandsInfo.json +++ b/libs/resources/RespCommandsInfo.json @@ -3975,6 +3975,30 @@ } ] }, + { + "Command": "SETIFGREATER", + "Name": "SETIFGREATER", + "Arity": -4, + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Slow, String, Write", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RW, Access, Update, VariableFlags" + } + ] + }, { "Command": "SETIFMATCH", "Name": "SETIFMATCH", diff --git a/libs/server/Resp/BasicCommands.cs b/libs/server/Resp/BasicCommands.cs index 6d0196af1a..132ee53873 100644 --- a/libs/server/Resp/BasicCommands.cs +++ b/libs/server/Resp/BasicCommands.cs @@ -657,8 +657,8 @@ private bool NetworkSET_Conditional(RespCommand cmd, int expiry, ref if (!getValue && !withEtag) { - // the following debug assertion is the catch any edge case leading to SETIFMATCH skipping the above block - Debug.Assert(cmd != RespCommand.SETIFMATCH, "SETIFMATCH should have gone though pointing to right output variable"); + // the following debug assertion is the catch any edge case leading to SETIFMATCH, or SETIFGREATER skipping the above block + Debug.Assert(cmd is not (RespCommand.SETIFMATCH or RespCommand.SETIFGREATER), "SETIFMATCH should have gone though pointing to right output variable"); GarnetStatus status = storageApi.SET_Conditional(ref key, ref input); diff --git a/libs/server/Resp/BasicEtagCommands.cs b/libs/server/Resp/BasicEtagCommands.cs index 23f4ab7d6d..8bcf447636 100644 --- a/libs/server/Resp/BasicEtagCommands.cs +++ b/libs/server/Resp/BasicEtagCommands.cs @@ -77,9 +77,11 @@ private bool NetworkGETIFNOTMATCH(ref TGarnetApi storageApi) return true; } + /// - /// SETIFMATCH key val etag [EX|PX] [expiry] - /// Sets a key value pair only if the already existing etag matches the etag sent as a part of the request. + /// SETIFMATCH key val etag [EX|PX] [expiry] [NOGET] + /// Sets a key value pair with the given etag only if (1) the etag given in the request matches the already existing etag ; + /// or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent Etag was 0. /// /// /// @@ -87,9 +89,33 @@ private bool NetworkGETIFNOTMATCH(ref TGarnetApi storageApi) private bool NetworkSETIFMATCH(ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { - if (parseState.Count < 3 || parseState.Count > 5) + return NetworkSetETagConditional(RespCommand.SETIFMATCH, ref storageApi); + } + + + /// + /// SETIFGREATER key val etag [EX|PX] [expiry] [NOGET] + /// Sets a key value pair with the given etag only if (1) the etag given in the request is greater than the already existing etag ; + /// or (2) there was no existing value; or (3) the existing value was not associated with any etag. + /// + /// + /// + /// + private bool NetworkSETIFGREATER(ref TGarnetApi storageApi) + where TGarnetApi : IGarnetApi + { + return NetworkSetETagConditional(RespCommand.SETIFGREATER, ref storageApi); + } + + private bool NetworkSetETagConditional(RespCommand cmd, ref TGarnetApi storageApi) + where TGarnetApi : IGarnetApi + { + // Currently only supports these two commands + Debug.Assert(cmd is RespCommand.SETIFMATCH or RespCommand.SETIFGREATER); + + if (parseState.Count < 3 || parseState.Count > 6) { - return AbortWithWrongNumberOfArguments(nameof(RespCommand.SETIFMATCH)); + return AbortWithWrongNumberOfArguments(nameof(cmd)); } int expiry = 0; @@ -97,17 +123,59 @@ private bool NetworkSETIFMATCH(ref TGarnetApi storageApi) var tokenIdx = 3; ExpirationOption expOption = ExpirationOption.None; - if (tokenIdx < parseState.Count) + bool noGet = false; + + while (tokenIdx < parseState.Count) { - if (!parseState.TryGetExpirationOption(tokenIdx++, out expOption) || (expOption is not ExpirationOption.EX and not ExpirationOption.PX)) - errorMessage = CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR; - else + // Parse NOGET option + if (parseState.GetArgSliceByRef(tokenIdx).Span.EqualsUpperCaseSpanIgnoringCase(CmdStrings.NOGET)) + { + if (noGet) + { + errorMessage = CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR; + break; + } + + noGet = true; + tokenIdx++; + continue; + } + + // Parse EX | PX expiry combination + if (parseState.TryGetExpirationOption(tokenIdx, out expOption)) { - if (!parseState.TryGetInt(tokenIdx++, out expiry)) + if (expOption is not ExpirationOption.EX and not ExpirationOption.PX) + { + errorMessage = CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR; + break; + } + + // we know that the token is either EX or PX from above and the next value should be the expiry + tokenIdx++; + if (!parseState.TryGetInt(tokenIdx, out expiry)) + { errorMessage = CmdStrings.RESP_ERR_GENERIC_VALUE_IS_NOT_INTEGER; + break; + } else if (expiry <= 0) + { errorMessage = CmdStrings.RESP_ERR_GENERIC_INVALIDEXP_IN_SET; + break; + } + + tokenIdx++; + continue; } + + // neither NOGET nor EX|PX expiry combination + errorMessage = CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR; + break; + } + + bool etagRead = parseState.TryGetLong(2, out long etag); + if (!etagRead || etag < 0) + { + errorMessage = CmdStrings.RESP_ERR_INVALID_ETAG; } if (!errorMessage.IsEmpty) @@ -119,7 +187,7 @@ private bool NetworkSETIFMATCH(ref TGarnetApi storageApi) SpanByte key = parseState.GetArgSliceByRef(0).SpanByte; - NetworkSET_Conditional(RespCommand.SETIFMATCH, expiry, ref key, getValue: true, highPrecision: expOption == ExpirationOption.PX, withEtag: true, ref storageApi); + NetworkSET_Conditional(cmd, expiry, ref key, getValue: !noGet, highPrecision: expOption == ExpirationOption.PX, withEtag: true, ref storageApi); return true; } diff --git a/libs/server/Resp/CmdStrings.cs b/libs/server/Resp/CmdStrings.cs index ed601b9cec..aa9e1b815a 100644 --- a/libs/server/Resp/CmdStrings.cs +++ b/libs/server/Resp/CmdStrings.cs @@ -144,9 +144,11 @@ static partial class CmdStrings public static ReadOnlySpan GETWITHETAG => "GETWITHETAG"u8; public static ReadOnlySpan GETIFNOTMATCH => "GETIFNOTMATCH"u8; public static ReadOnlySpan SETIFMATCH => "SETIFMATCH"u8; + public static ReadOnlySpan SETIFGREATER => "SETIFGREATER"u8; public static ReadOnlySpan FIELDS => "FIELDS"u8; public static ReadOnlySpan TIMEOUT => "TIMEOUT"u8; public static ReadOnlySpan ERROR => "ERROR"u8; + public static ReadOnlySpan NOGET => "NOGET"u8; /// /// Response strings @@ -251,9 +253,11 @@ static partial class CmdStrings public static ReadOnlySpan RESP_COMMAND_HAS_NO_KEY_ARGS => "The command has no key arguments"u8; public static ReadOnlySpan RESP_ERR_INVALID_CLIENT_UNBLOCK_REASON => "ERR CLIENT UNBLOCK reason should be TIMEOUT or ERROR"u8; public static ReadOnlySpan RESP_UNBLOCKED_CLIENT_VIA_CLIENT_UNBLOCK => "UNBLOCKED client unblocked via CLIENT UNBLOCK"u8; + public static ReadOnlySpan RESP_ERR_INVALID_ETAG => "ETAG must be a numerical value greater than or equal to 0"u8; public static ReadOnlySpan RESP_ERR_DEUBG_DISALLOWED => @"ERR DEBUG command not allowed. If the EnableDebugCommand option is set to ""local"", you can run it from a local connection, otherwise you need to set this option in the configuration file, and then restart the server."u8; + /// /// Response string templates /// diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index d3640f1e7f..6ed92c8276 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -174,6 +174,7 @@ public enum RespCommand : ushort SETEXXX, SETNX, SETIFMATCH, + SETIFGREATER, SETKEEPTTL, SETKEEPTTLXX, SETRANGE, @@ -2448,6 +2449,10 @@ private RespCommand SlowParseCommand(ref int count, ref ReadOnlySpan speci { return RespCommand.SETIFMATCH; } + else if (command.SequenceEqual(CmdStrings.SETIFGREATER)) + { + return RespCommand.SETIFGREATER; + } else if (command.SequenceEqual(CmdStrings.GETWITHETAG)) { return RespCommand.GETWITHETAG; diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs index b77e666095..8e42a4c51b 100644 --- a/libs/server/Resp/RespServerSession.cs +++ b/libs/server/Resp/RespServerSession.cs @@ -866,6 +866,7 @@ private bool ProcessOtherCommands(RespCommand command, ref TGarnetAp RespCommand.GETWITHETAG => NetworkGETWITHETAG(ref storageApi), RespCommand.GETIFNOTMATCH => NetworkGETIFNOTMATCH(ref storageApi), RespCommand.SETIFMATCH => NetworkSETIFMATCH(ref storageApi), + RespCommand.SETIFGREATER => NetworkSETIFGREATER(ref storageApi), _ => Process(command, ref storageApi) }; diff --git a/libs/server/Storage/Functions/EtagState.cs b/libs/server/Storage/Functions/EtagState.cs index 266b745a6b..244ff325ff 100644 --- a/libs/server/Storage/Functions/EtagState.cs +++ b/libs/server/Storage/Functions/EtagState.cs @@ -9,7 +9,7 @@ internal static class EtagConstants { public const byte EtagSize = sizeof(long); - public const long BaseEtag = 0; + public const long NoETag = 0; } /// @@ -39,7 +39,7 @@ public EtagState() /// /// Field provides access to getting an Etag from a record, hiding whether it is actually present or not. /// - public long etag { get; private set; } = EtagConstants.BaseEtag; + public long etag { get; set; } = EtagConstants.NoETag; /// /// Sets the values to indicate the presence of an Etag as a part of the payload value @@ -56,7 +56,7 @@ public static void ResetState(ref EtagState curr) { curr.etagOffsetForVarlen = 0; curr.etagSkippedStart = 0; - curr.etag = EtagConstants.BaseEtag; + curr.etag = EtagConstants.NoETag; curr.etagAccountedLength = -1; } } diff --git a/libs/server/Storage/Functions/MainStore/PrivateMethods.cs b/libs/server/Storage/Functions/MainStore/PrivateMethods.cs index 9a41f592e9..4d120e0fe4 100644 --- a/libs/server/Storage/Functions/MainStore/PrivateMethods.cs +++ b/libs/server/Storage/Functions/MainStore/PrivateMethods.cs @@ -663,7 +663,7 @@ static void CopyRespWithEtagData(ref SpanByte value, ref SpanByteAndMemory dst, int desiredLength = 4; ReadOnlySpan etagTruncatedVal; // get etag to write, default etag 0 for when no etag - long etag = hasEtagInVal ? value.GetEtagInPayload() : EtagConstants.BaseEtag; + long etag = hasEtagInVal ? value.GetEtagInPayload() : EtagConstants.NoETag; // remove the length of the ETAG var etagAccountedValueLength = valueLength - etagSkippedStart; if (hasEtagInVal) diff --git a/libs/server/Storage/Functions/MainStore/RMWMethods.cs b/libs/server/Storage/Functions/MainStore/RMWMethods.cs index 11024c7cc7..033d32ddb4 100644 --- a/libs/server/Storage/Functions/MainStore/RMWMethods.cs +++ b/libs/server/Storage/Functions/MainStore/RMWMethods.cs @@ -28,7 +28,6 @@ public bool NeedInitialUpdate(ref SpanByte key, ref RawStringInput input, ref Sp case RespCommand.GETDEL: case RespCommand.GETEX: return false; - case RespCommand.SETIFMATCH: case RespCommand.SETEXXX: // when called withetag all output needs to be placed on the buffer if (input.header.CheckWithEtagFlag()) @@ -37,6 +36,11 @@ public bool NeedInitialUpdate(ref SpanByte key, ref RawStringInput input, ref Sp CopyDefaultResp(CmdStrings.RESP_ERRNOTFOUND, ref output); } return false; + case RespCommand.SETIFGREATER: + case RespCommand.SETIFMATCH: + // add etag on first insertion + this.functionsState.etagState.etagOffsetForVarlen = EtagConstants.EtagSize; + return true; case RespCommand.SET: case RespCommand.SETEXNX: case RespCommand.SETKEEPTTL: @@ -87,8 +91,8 @@ public bool InitialUpdater(ref SpanByte key, ref RawStringInput input, ref SpanB Buffer.MemoryCopy(srcHLL, dstHLL, value.Length, value.Length); break; - case RespCommand.SET: - case RespCommand.SETEXNX: + case RespCommand.SETIFGREATER: + case RespCommand.SETIFMATCH: int spaceForEtag = this.functionsState.etagState.etagOffsetForVarlen; // Copy input to value var newInputValue = input.parseState.GetArgSliceByRef(0).ReadOnlySpan; @@ -97,14 +101,46 @@ public bool InitialUpdater(ref SpanByte key, ref RawStringInput input, ref SpanB value.ExtraMetadata = input.arg1; newInputValue.CopyTo(value.AsSpan(spaceForEtag)); + long clientSentEtag = input.parseState.GetLong(1); + + clientSentEtag++; + + recordInfo.SetHasETag(); + // the increment on initial etag is for satisfying the variant that any key with no etag is the same as a zero'd etag + value.SetEtagInPayload(clientSentEtag); + EtagState.SetValsForRecordWithEtag(ref functionsState.etagState, ref value); + + // write back array of the format [etag, nil] + var nilResponse = CmdStrings.RESP_ERRNOTFOUND; + // *2\r\n: + + \r\n + + WriteValAndEtagToDst( + 4 + 1 + NumUtils.CountDigits(functionsState.etagState.etag) + 2 + nilResponse.Length, + ref nilResponse, + functionsState.etagState.etag, + ref output, + functionsState.memoryPool, + writeDirect: true + ); + + break; + case RespCommand.SET: + case RespCommand.SETEXNX: + spaceForEtag = this.functionsState.etagState.etagOffsetForVarlen; + // Copy input to value + newInputValue = input.parseState.GetArgSliceByRef(0).ReadOnlySpan; + metadataSize = input.arg1 == 0 ? 0 : sizeof(long); + value.ShrinkSerializedLength(newInputValue.Length + metadataSize + spaceForEtag); + value.ExtraMetadata = input.arg1; + newInputValue.CopyTo(value.AsSpan(spaceForEtag)); + if (spaceForEtag != 0) { recordInfo.SetHasETag(); // the increment on initial etag is for satisfying the variant that any key with no etag is the same as a zero'd etag - value.SetEtagInPayload(EtagConstants.BaseEtag + 1); + value.SetEtagInPayload(EtagConstants.NoETag + 1); EtagState.SetValsForRecordWithEtag(ref functionsState.etagState, ref value); // Copy initial etag to output only for SET + WITHETAG and not SET NX or XX - CopyRespNumber(EtagConstants.BaseEtag + 1, ref output); + CopyRespNumber(EtagConstants.NoETag + 1, ref output); } break; @@ -118,10 +154,10 @@ public bool InitialUpdater(ref SpanByte key, ref RawStringInput input, ref SpanB if (spaceForEtag != 0) { recordInfo.SetHasETag(); - value.SetEtagInPayload(EtagConstants.BaseEtag + 1); + value.SetEtagInPayload(EtagConstants.NoETag + 1); EtagState.SetValsForRecordWithEtag(ref functionsState.etagState, ref value); // Copy initial etag to output - CopyRespNumber(EtagConstants.BaseEtag + 1, ref output); + CopyRespNumber(EtagConstants.NoETag + 1, ref output); } break; @@ -247,7 +283,7 @@ public bool InitialUpdater(ref SpanByte key, ref RawStringInput input, ref SpanB public void PostInitialUpdater(ref SpanByte key, ref RawStringInput input, ref SpanByte value, ref SpanByteAndMemory output, ref RMWInfo rmwInfo) { // reset etag state set at need initial update - if (input.header.cmd is (RespCommand.SET or RespCommand.SETEXNX or RespCommand.SETKEEPTTL)) + if (input.header.cmd is (RespCommand.SET or RespCommand.SETEXNX or RespCommand.SETKEEPTTL or RespCommand.SETIFMATCH or RespCommand.SETIFGREATER)) EtagState.ResetState(ref functionsState.etagState); functionsState.watchVersionMap.IncrementVersion(rmwInfo.KeyHash); @@ -311,11 +347,33 @@ private bool InPlaceUpdaterWorker(ref SpanByte key, ref RawStringInput input, re EtagState.ResetState(ref functionsState.etagState); // Nothing is set because being in this block means NX was already violated return true; + case RespCommand.SETIFGREATER: case RespCommand.SETIFMATCH: long etagFromClient = input.parseState.GetLong(1); - if (functionsState.etagState.etag != etagFromClient) + // in IFMATCH we check for equality, in IFGREATER we are checking for sent etag being strictly greater + int comparisonResult = etagFromClient.CompareTo(functionsState.etagState.etag); + int expectedResult = cmd is RespCommand.SETIFMATCH ? 0 : 1; + + if (comparisonResult != expectedResult) { - CopyRespWithEtagData(ref value, ref output, shouldUpdateEtag, functionsState.etagState.etagSkippedStart, functionsState.memoryPool); + if (input.header.CheckSetGetFlag()) + { + CopyRespWithEtagData(ref value, ref output, shouldUpdateEtag, functionsState.etagState.etagSkippedStart, functionsState.memoryPool); + } + else + { + // write back array of the format [etag, nil] + var nilResponse = CmdStrings.RESP_ERRNOTFOUND; + // *2\r\n: + + \r\n + + WriteValAndEtagToDst( + 4 + 1 + NumUtils.CountDigits(functionsState.etagState.etag) + 2 + nilResponse.Length, + ref nilResponse, + functionsState.etagState.etag, + ref output, + functionsState.memoryPool, + writeDirect: true + ); + } // reset etag state after done using EtagState.ResetState(ref functionsState.etagState); return true; @@ -330,19 +388,19 @@ private bool InPlaceUpdaterWorker(ref SpanByte key, ref RawStringInput input, re if (value.Length < inputValue.length + EtagConstants.EtagSize + metadataSize) return false; - if (input.arg1 != 0) - { - value.ExtraMetadata = input.arg1; - } - recordInfo.SetHasETag(); - // Increment the ETag - long newEtag = functionsState.etagState.etag + 1; + long newEtag = cmd is RespCommand.SETIFMATCH ? (functionsState.etagState.etag + 1) : (etagFromClient + 1); + rmwInfo.ClearExtraValueLength(ref recordInfo, ref value, value.TotalSize); + value.UnmarkExtraMetadata(); value.ShrinkSerializedLength(metadataSize + inputValue.Length + EtagConstants.EtagSize); rmwInfo.SetUsedValueLength(ref recordInfo, ref value, value.TotalSize); - rmwInfo.ClearExtraValueLength(ref recordInfo, ref value, value.TotalSize); + + if (input.arg1 != 0) + { + value.ExtraMetadata = input.arg1; + } value.SetEtagInPayload(newEtag); @@ -813,22 +871,42 @@ public bool NeedCopyUpdate(ref SpanByte key, ref RawStringInput input, ref SpanB { switch (input.header.cmd) { + case RespCommand.SETIFGREATER: case RespCommand.SETIFMATCH: - long etagToCheckWith = input.parseState.GetLong(1); - if (functionsState.etagState.etag == etagToCheckWith) + // in IFMATCH we check for equality, in IFGREATER we are checking for sent etag being strictly greater + int comparisonResult = etagToCheckWith.CompareTo(functionsState.etagState.etag); + int expectedResult = input.header.cmd is RespCommand.SETIFMATCH ? 0 : 1; + + if (comparisonResult == expectedResult) { return true; } - else + + if (input.header.CheckSetGetFlag()) { + // Copy value to output for the GET part of the command. CopyRespWithEtagData(ref oldValue, ref output, hasEtagInVal: rmwInfo.RecordInfo.ETag, functionsState.etagState.etagSkippedStart, functionsState.memoryPool); - // reset etag state that may have been initialized earlier - EtagState.ResetState(ref functionsState.etagState); - return false; + } + else + { + // write back array of the format [etag, nil] + var nilResponse = CmdStrings.RESP_ERRNOTFOUND; + // *2\r\n: + + \r\n + + WriteValAndEtagToDst( + 4 + 1 + NumUtils.CountDigits(functionsState.etagState.etag) + 2 + nilResponse.Length, + ref nilResponse, + functionsState.etagState.etag, + ref output, + functionsState.memoryPool, + writeDirect: true + ); } + EtagState.ResetState(ref functionsState.etagState); + return false; + case RespCommand.SETEXNX: // Expired data, return false immediately // ExpireAndResume ensures that we set as new value, since it does not exist @@ -918,7 +996,10 @@ public bool CopyUpdater(ref SpanByte key, ref RawStringInput input, ref SpanByte switch (cmd) { + case RespCommand.SETIFGREATER: case RespCommand.SETIFMATCH: + // By now the comparison for etag against existing etag has already been done in NeedCopyUpdate + shouldUpdateEtag = true; // Copy input to value Span dest = newValue.AsSpan(EtagConstants.EtagSize); @@ -937,6 +1018,12 @@ public bool CopyUpdater(ref SpanByte key, ref RawStringInput input, ref SpanByte newValue.ExtraMetadata = input.arg1; } + long etagFromClient = input.parseState.GetLong(1); + + // change the current etag to the the etag sent from client since rest remains same + if (cmd == RespCommand.SETIFGREATER) + functionsState.etagState.etag = etagFromClient; + long newEtag = functionsState.etagState.etag + 1; recordInfo.SetHasETag(); diff --git a/libs/server/Storage/Functions/MainStore/ReadMethods.cs b/libs/server/Storage/Functions/MainStore/ReadMethods.cs index 7ead9a714c..c7aa8ec694 100644 --- a/libs/server/Storage/Functions/MainStore/ReadMethods.cs +++ b/libs/server/Storage/Functions/MainStore/ReadMethods.cs @@ -148,7 +148,7 @@ private bool handleGetIfNotMatch(ref RawStringInput input, ref SpanByte value, r // Any value without an etag is treated the same as a value with an etag long etagToMatchAgainst = input.parseState.GetLong(0); - long existingEtag = readInfo.RecordInfo.ETag ? value.GetEtagInPayload() : EtagConstants.BaseEtag; + long existingEtag = readInfo.RecordInfo.ETag ? value.GetEtagInPayload() : EtagConstants.NoETag; if (existingEtag == etagToMatchAgainst) { diff --git a/libs/server/Storage/Functions/MainStore/VarLenInputMethods.cs b/libs/server/Storage/Functions/MainStore/VarLenInputMethods.cs index 39b3011bc5..076664954a 100644 --- a/libs/server/Storage/Functions/MainStore/VarLenInputMethods.cs +++ b/libs/server/Storage/Functions/MainStore/VarLenInputMethods.cs @@ -203,6 +203,7 @@ public int GetRMWModifiedValueLength(ref SpanByte t, ref RawStringInput input) return sizeof(int) + input.parseState.GetArgSliceByRef(0).Length + (input.arg1 == 0 ? 0 : sizeof(long)) + functionsState.etagState.etagOffsetForVarlen; case RespCommand.PERSIST: return sizeof(int) + t.LengthWithoutMetadata; + case RespCommand.SETIFGREATER: case RespCommand.SETIFMATCH: var newValue = input.parseState.GetArgSliceByRef(0).ReadOnlySpan; int metadataSize = input.arg1 == 0 ? t.MetadataSize : sizeof(long); diff --git a/libs/server/Transaction/TxnKeyManager.cs b/libs/server/Transaction/TxnKeyManager.cs index 96d3ea68c5..d87f1b9c47 100644 --- a/libs/server/Transaction/TxnKeyManager.cs +++ b/libs/server/Transaction/TxnKeyManager.cs @@ -145,6 +145,7 @@ internal int GetKeys(RespCommand command, int inputCount, out ReadOnlySpan RespCommand.GETWITHETAG => SingleKey(1, false, LockType.Shared), RespCommand.SET => SingleKey(1, false, LockType.Exclusive), RespCommand.SETIFMATCH => SingleKey(1, false, LockType.Exclusive), + RespCommand.SETIFGREATER => SingleKey(1, false, LockType.Exclusive), RespCommand.GETRANGE => SingleKey(1, false, LockType.Shared), RespCommand.SETRANGE => SingleKey(1, false, LockType.Exclusive), RespCommand.PFADD => SingleKey(1, false, LockType.Exclusive), diff --git a/playground/CommandInfoUpdater/GarnetCommandsDocs.json b/playground/CommandInfoUpdater/GarnetCommandsDocs.json index bf0bd62dc9..bebf4f591a 100644 --- a/playground/CommandInfoUpdater/GarnetCommandsDocs.json +++ b/playground/CommandInfoUpdater/GarnetCommandsDocs.json @@ -723,7 +723,7 @@ { "Command": "SETIFMATCH", "Name": "SETIFMATCH", - "Summary": "Sets the string value of a key, ignoring its type, if the key\u0027s current etag matches the given etag.", + "Summary": "Sets a key value pair with the given etag only if (1) the etag given in the request matches the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was 0.", "Group": "String", "Complexity": "O(1)", "Arguments": [ @@ -767,6 +767,72 @@ "Token": "PX" } ] + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "NOGET", + "DisplayText": "noget", + "Type": "PureToken", + "Token": "NOGET", + "ArgumentFlags": "Optional" + } + ] + }, + { + "Command": "SETIFGREATER", + "Name": "SETIFGREATER", + "Summary": "Sets a key value pair with the given etag only if (1) the etag given in the request is greater than the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was greater than 0.", + "Group": "String", + "Complexity": "O(1)", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandKeyArgument", + "Name": "KEY", + "DisplayText": "key", + "Type": "Key", + "KeySpecIndex": 0 + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "VALUE", + "DisplayText": "value", + "Type": "String" + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "ETAG", + "DisplayText": "etag", + "Type": "Integer" + }, + { + "TypeDiscriminator": "RespCommandContainerArgument", + "Name": "EXPIRATION", + "Type": "OneOf", + "ArgumentFlags": "Optional", + "Arguments": [ + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "SECONDS", + "DisplayText": "seconds", + "Type": "Integer", + "Token": "EX" + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "MILLISECONDS", + "DisplayText": "milliseconds", + "Type": "Integer", + "Token": "PX" + } + ] + }, + { + "TypeDiscriminator": "RespCommandBasicArgument", + "Name": "NOGET", + "DisplayText": "noget", + "Type": "PureToken", + "Token": "NOGET", + "ArgumentFlags": "Optional" } ] }, diff --git a/playground/CommandInfoUpdater/GarnetCommandsInfo.json b/playground/CommandInfoUpdater/GarnetCommandsInfo.json index 1cc3b765dd..eeeb0e41be 100644 --- a/playground/CommandInfoUpdater/GarnetCommandsInfo.json +++ b/playground/CommandInfoUpdater/GarnetCommandsInfo.json @@ -805,6 +805,32 @@ } ] }, + { + "Command": "SETIFGREATER", + "Name": "SETIFGREATER", + "IsInternal": false, + "Arity": -4, + "Flags": "NONE", + "FirstKey": 1, + "LastKey": 1, + "Step": 1, + "AclCategories": "Slow, String, Write", + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Flags": "RW, Access, Update, VariableFlags" + } + ] + }, { "Command": "WATCH", "Name": "WATCH", diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index 9f2341398d..c4fcd86e8d 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -261,6 +261,7 @@ public class SupportedCommand new("SETBIT", RespCommand.SETBIT), new("SETEX", RespCommand.SETEX), new("SETIFMATCH", RespCommand.SETIFMATCH), + new("SETIFGREATER", RespCommand.SETIFGREATER), new("SETNX", RespCommand.SETNX), new("SETRANGE", RespCommand.SETRANGE), new("SISMEMBER", RespCommand.SISMEMBER), diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index 412d026bcf..3bf1b74eec 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -5450,8 +5450,23 @@ await CheckCommandsAsync( static async Task DoSetIfMatchAsync(GarnetClient client) { - var res = await client.ExecuteForStringResultAsync("SETIFMATCH", ["foo", "rizz", "0"]); - ClassicAssert.IsNull(res); + var res = await client.ExecuteForStringArrayResultAsync("SETIFMATCH", ["foo", "rizz", "0"]); + ClassicAssert.IsNotNull(res); + } + } + + [Test] + public async Task SetIfGreaterACLsAsync() + { + await CheckCommandsAsync( + "SETIFGREATER", + [DoSetIfGreaterAsync] + ); + + static async Task DoSetIfGreaterAsync(GarnetClient client) + { + var res = await client.ExecuteForStringArrayResultAsync("SETIFGREATER", ["foo", "rizz", "0"]); + ClassicAssert.IsNotNull(res); } } diff --git a/test/Garnet.test/RespEtagTests.cs b/test/Garnet.test/RespEtagTests.cs index 460ab47203..ebc21f9aab 100644 --- a/test/Garnet.test/RespEtagTests.cs +++ b/test/Garnet.test/RespEtagTests.cs @@ -322,9 +322,91 @@ public void SetWithEtagWorksWithMetadata() ClassicAssert.AreEqual(2, etag7); } + [Test] + public void SetIfGreaterWorksWithInitialETag() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + IDatabase db = redis.GetDatabase(0); + + var key = "meow-key"; + var value = "m"; + + RedisResult res = db.Execute("SET", key, value, "WITHETAG"); + ClassicAssert.AreEqual(1, (long)res); + + // not greater etag sent so we expect a higher etag returned + RedisResult[] arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, "diggity", 0); + ClassicAssert.AreEqual(1, (long)arrRes[0]); + ClassicAssert.AreEqual(value, arrRes[1].ToString()); + + // greater etag sent so we expect the same etag returned + var newValue = "meow"; + arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, newValue, 2); + ClassicAssert.AreEqual(3, (long)arrRes[0]); + ClassicAssert.IsTrue(arrRes[1].IsNull); + + // shrink value size and send greater etag + newValue = "m"; + arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, newValue, 5); + ClassicAssert.AreEqual(6, (long)arrRes[0]); + ClassicAssert.IsTrue(arrRes[1].IsNull); + } + + [Test] + public void SetIfGreaterWorksWithoutInitialETag() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + IDatabase db = redis.GetDatabase(0); + + var key = "meow-key"; + var value = "m"; + + RedisResult res = db.Execute("SET", key, value); + ClassicAssert.AreEqual("OK", res.ToString()); + + // not greater etag sent so we expect a higher etag returned + RedisResult[] arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, "check", 0); + ClassicAssert.AreEqual(0, (long)arrRes[0]); + ClassicAssert.AreEqual(value, arrRes[1].ToString()); + + // greater etag sent so we expect the same etag returned + var newValue = "meow"; + arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, newValue, 2); + ClassicAssert.AreEqual(3, (long)arrRes[0]); + ClassicAssert.IsTrue(arrRes[1].IsNull); + + // shrink value size and send greater etag + newValue = "m"; + arrRes = (RedisResult[])db.Execute("SETIFGREATER", key, newValue, 5); + ClassicAssert.AreEqual(6, (long)arrRes[0]); + ClassicAssert.IsTrue(arrRes[1].IsNull); + } + #endregion # region Edgecases + [Test] + public void SetIfMatchSetsKeyValueOnNonExistingKey() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + IDatabase db = redis.GetDatabase(0); + + RedisResult[] result = (RedisResult[])db.Execute("SETIFMATCH", "key", "valueanother", 1, "EX", 3); + ClassicAssert.AreEqual(2, (long)result[0]); + ClassicAssert.IsTrue(result[1].IsNull); + } + + + [Test] + public void SetIfGreaterSetsKeyValueOnNonExistingKey() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + IDatabase db = redis.GetDatabase(0); + + RedisResult[] result = (RedisResult[])db.Execute("SETIFGREATER", "key", "valueanother", 1, "EX", 3); + ClassicAssert.AreEqual(2, (long)result[0]); + ClassicAssert.IsTrue(result[1].IsNull); + } [Test] public void SETOnAlreadyExistingSETDataOverridesItWithInitialEtag() @@ -454,16 +536,34 @@ public void SETOnAlreadyExistingNonEtagDataOverridesIt() [Test] - public void SetIfMatchOnNonEtagDataReturnsNewEtagAndValue() + public void SetIfMatchOnNonEtagDataReturnsNewEtagAndNoValue() { using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); IDatabase db = redis.GetDatabase(0); var _ = db.StringSet("h", "k"); - var res = (RedisResult[])db.Execute("SETIFMATCH", ["h", "t", "2"]); - ClassicAssert.AreEqual("0", res[0].ToString()); - ClassicAssert.AreEqual("k", res[1].ToString()); + var res = (RedisResult[])db.Execute("SETIFMATCH", ["h", "t", "0"]); + ClassicAssert.AreEqual("1", res[0].ToString()); + ClassicAssert.IsTrue(res[1].IsNull); + } + + [Test] + public void SetIfMatchReturnsNewEtagButNoValueWhenUsingNoGet() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + IDatabase db = redis.GetDatabase(0); + + var _ = db.StringSet("h", "k"); + + var res = (RedisResult[])db.Execute("SETIFMATCH", "h", "t", "0", "NOGET"); + ClassicAssert.AreEqual("1", res[0].ToString()); + ClassicAssert.IsTrue(res[1].IsNull); + + // ETag mismatch + res = (RedisResult[])db.Execute("SETIFMATCH", "h", "t", "2", "NOGET"); + ClassicAssert.AreEqual("1", res[0].ToString()); + ClassicAssert.IsTrue(res[1].IsNull); } [Test] diff --git a/website/docs/commands/garnet-specific.md b/website/docs/commands/garnet-specific.md index 8b3a71e646..699db52412 100644 --- a/website/docs/commands/garnet-specific.md +++ b/website/docs/commands/garnet-specific.md @@ -148,7 +148,32 @@ Garnet provides support for ETags on raw strings. By using the ETag-related comm Compatibility with non-ETag commands and the behavior of data inserted with ETags are detailed at the end of this document. -To initialize a key value pair with an ETag you can use either the SET command with the newly added "WITHETAG" optional flag, or you can take any existing Key value pair and call SETIFMATCH with the ETag argument as 0 (Any key value pair without an explicit ETag has an ETag of 0 implicitly). You can read more about setting an initial ETag via SET [here](../commands/raw-string#set) +To initialize a key value pair with an ETag you can use either the SET command with the newly added "WITHETAG" optional flag, or you can take any existing Key value pair and call SETIFMATCH with the ETag argument as 0 (Any key value pair without an explicit ETag has an ETag of 0 implicitly). **You can read more about setting an initial ETag via SET [here](../commands/raw-string#set)** +--- + +### **SET (WITHETAG)** + +#### **Syntax** + +```bash + SET key value [NX | XX] [EX seconds | PX milliseconds] [KEEPTTL] WITHETAG +``` + +Set **key** to hold the string value along with an ETag. If key already holds a value, it is overwritten, regardless of its type. Any previous time to live associated with the **key** is discarded on successful SET operation. + +**Options:** + +* EX seconds -- Set the specified expire time, in seconds (a positive integer). +* PX milliseconds -- Set the specified expire time, in milliseconds (a positive integer). +* NX -- Only set the key if it does not already exist. +* XX -- Only set the key if it already exists. +* KEEPTTL -- Retain the time to live associated with the key. +* WITHETAG -- **Adding this sets the Key Value pair with an initial ETag**, if called on an existing key value pair with an ETag, this command will update the ETag transparently. + +#### Resp Reply + +* Integer reply: WITHETAG given: The ETag associated with the value. + --- @@ -176,21 +201,43 @@ One of the following: #### **Syntax** ```bash -SETIFMATCH key value etag [EX seconds | PX milliseconds] +SETIFMATCH key value etag [EX seconds | PX milliseconds] [NOGET] ``` -Updates the value of a key if the provided ETag matches the current ETag of the key. +Sets/updates a key value pair with the given etag only if (1) the etag given in the request matches the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was 0. **Options:** * EX seconds -- Set the specified expire time, in seconds (a positive integer). * PX milliseconds -- Set the specified expire time, in milliseconds (a positive integer). +* NOGET -- The value is not returned even if the etag mismatched #### **Response** One of the following: -- **Array reply**: If etags match an array where the first item is the updated etag, and the second value is nil. If the etags do not match the array will hold the latest etag, and the latest value in order. -- **Nil reply**: If the key does not exist. +- **Array reply**: If the sent etag matches the existing etag the reponse will be an array where the first item is the updated etag, and the second value is nil. If the etags do not match then the response array will hold the latest etag, and the latest value in order. + +--- + +### **SETIFGREATER** + +#### **Syntax** + +```bash +SETIFGREATER key value etag [EX seconds | PX milliseconds] [NOGET] +``` +Sets/updates a key value pair with the given etag only if (1) the etag given in the request is greater than the already existing etag ; or (2) there was no existing value; or (3) the existing value was not associated with any etag and the sent etag was greater than 0. + +**Options:** +* EX seconds -- Set the specified expire time, in seconds (a positive integer). +* PX milliseconds -- Set the specified expire time, in milliseconds (a positive integer). +* NOGET -- The value is not returned even if the sent etag was not greater to existing etag. + +#### **Response** + +One of the following: + +- **Array reply**: If the sent etag is greater than the existing etag then an array where the first item is the updated etag, and the second value is nil is returned. If the sentEtag is less than or equal to the existing etag then the response array will hold the latest etag, and the latest value in order. ---