From 4c0b8080124c8e959f3df889f56182ed59c76f8b Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Sun, 8 Sep 2024 17:01:35 +0530 Subject: [PATCH 01/11] Inital changes (Doesn't build) --- libs/server/API/GarnetApi.cs | 4 + libs/server/API/IGarnetApi.cs | 9 ++ libs/server/Resp/KeyAdminCommands.cs | 24 ++++ libs/server/Resp/Parser/RespCommand.cs | 2 + .../Storage/Session/MainStore/MainStoreOps.cs | 131 ++++++++++++++++++ 5 files changed, 170 insertions(+) diff --git a/libs/server/API/GarnetApi.cs b/libs/server/API/GarnetApi.cs index 4aec2f705d..9fb3e706f4 100644 --- a/libs/server/API/GarnetApi.cs +++ b/libs/server/API/GarnetApi.cs @@ -154,6 +154,10 @@ public GarnetStatus APPEND(ArgSlice key, ArgSlice value, ref ArgSlice output) /// public GarnetStatus RENAME(ArgSlice oldKey, ArgSlice newKey, StoreType storeType = StoreType.All) => storageSession.RENAME(oldKey, newKey, storeType); + + /// + public GarnetStatus RENAMENX(ArgSlice oldKey, ArgSlice newKey, StoreType storeType = StoreType.All) + => storageSession.RENAMENX(oldKey, newKey, storeType); #endregion #region EXISTS diff --git a/libs/server/API/IGarnetApi.cs b/libs/server/API/IGarnetApi.cs index 7a84d8603d..67403cbb02 100644 --- a/libs/server/API/IGarnetApi.cs +++ b/libs/server/API/IGarnetApi.cs @@ -118,6 +118,15 @@ public interface IGarnetApi : IGarnetReadApi, IGarnetAdvancedApi /// /// GarnetStatus RENAME(ArgSlice oldKey, ArgSlice newKey, StoreType storeType = StoreType.All); + + /// + /// Renames key to newkey if newkey does not yet exist. It returns an error when key does not exist. + /// + /// + /// + /// + /// + GarnetStatus RENAMENX(ArgSlice oldKey, ArgSlice newKey, StoreType storeType = StoreType.All); #endregion #region EXISTS diff --git a/libs/server/Resp/KeyAdminCommands.cs b/libs/server/Resp/KeyAdminCommands.cs index 2365e9bb27..8a09deba0a 100644 --- a/libs/server/Resp/KeyAdminCommands.cs +++ b/libs/server/Resp/KeyAdminCommands.cs @@ -40,6 +40,30 @@ private bool NetworkRENAME(ref TGarnetApi storageApi) return true; } + /// + /// TryRENAMENX + /// + private bool NetworkRENAMENX(ref TGarnetApi storageApi) + where TGarnetApi : IGarnetApi + { + // TODO: (Nirmal) If need combine the RENAME and RENAMENX commands by passing RespCommand command. Refer: HashSet, HashCommands.cs + if (parseState.Count != 2) + { + return AbortWithWrongNumberOfArguments(nameof(RespCommand.RENAMENX)); + } + + var oldKeySlice = parseState.GetArgSliceByRef(0); + var newKeySlice = parseState.GetArgSliceByRef(1); + // TODO: (Nirmal) If need combine the RENAME and RENAMENX commands by using out param. Refer: HashSet, HashCommands.cs + var status = storageApi.RENAMENX(oldKeySlice, newKeySlice); + + // TODO: (Nirmal) Add int response for the RENAMENX command by adding first param + while (!RespWriteUtils.WriteInteger(1, ref dcurr, dend)) + SendAndReset(); + + return true; + } + /// /// GETDEL command processor /// diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index 83595b1cb0..fd912a555a 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -117,6 +117,7 @@ public enum RespCommand : byte PFMERGE, PSETEX, RENAME, + RENAMENX, RPOP, RPOPLPUSH, RPUSH, @@ -620,6 +621,7 @@ private RespCommand FastParseCommand(out int count) (2 << 4) | 6 when lastWord == MemoryMarshal.Read("INCRBY\r\n"u8) => RespCommand.INCRBY, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("DECRBY\r\n"u8) => RespCommand.DECRBY, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("RENAME\r\n"u8) => RespCommand.RENAME, + (2 << 4) | 8 when lastWord == MemoryMarshal.Read("RENAMENX\r\n"u8) => RespCommand.RENAMENX, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("GETBIT\r\n"u8) => RespCommand.GETBIT, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("APPEND\r\n"u8) => RespCommand.APPEND, (2 << 4) | 7 when lastWord == MemoryMarshal.Read("UBLISH\r\n"u8) && ptr[8] == 'P' => RespCommand.PUBLISH, diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index f979bcc0f6..6b8d5feccb 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -616,6 +616,137 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St return returnStatus; } + public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, StoreType storeType) + { + GarnetStatus returnStatus = GarnetStatus.NOTFOUND; + + // If same name check return early. + if (oldKeySlice.ReadOnlySpan.SequenceEqual(newKeySlice.ReadOnlySpan)) + return GarnetStatus.OK; + + bool createTransaction = false; + if (txnManager.state != TxnState.Running) + { + createTransaction = true; + txnManager.SaveKeyEntryToLock(oldKeySlice, false, LockType.Exclusive); + txnManager.SaveKeyEntryToLock(newKeySlice, false, LockType.Exclusive); + txnManager.Run(true); + } + + var context = txnManager.LockableContext; + var objectContext = txnManager.ObjectStoreLockableContext; + + if (storeType == StoreType.Main || storeType == StoreType.All) + { + try + { + SpanByte oldKey = oldKeySlice.SpanByte; + SpanByte newKey = newKeySlice.SpanByte; + + SpanByte input = default; + var o = new SpanByteAndMemory(); + var status = GET(ref oldKey, ref input, ref o, ref context); + + if (status == GarnetStatus.OK) + { + Debug.Assert(!o.IsSpanByte); + var memoryHandle = o.Memory.Memory.Pin(); + var ptrVal = (byte*)memoryHandle.Pointer; + + RespReadUtils.ReadUnsignedLengthHeader(out var headerLength, ref ptrVal, ptrVal + o.Length); + var value = SpanByte.FromPinnedPointer(ptrVal, headerLength); + + // TODO: (Nirmal) Find the right way to get the inputPtr (pointer to input value). + byte* inputPtr = null; + // Make space for RespCommand in input + inputPtr -= RespInputHeader.Size; + var expiryBytes = new SpanByteAndMemory(); + var expiryStatus = TTL(ref oldKey, storeType, ref expiryBytes, ref context, ref objectContext, true); + + if (expiry == 0) // no expiration provided + { + *(int*)inputPtr = RespInputHeader.Size + isize; + ((RespInputHeader*)(inputPtr + sizeof(int)))->cmd = cmd; + ((RespInputHeader*)(inputPtr + sizeof(int)))->flags = 0; + if (getValue) + ((RespInputHeader*)(inputPtr + sizeof(int)))->SetSetGetFlag(); + } + else + { + // Move payload forward to make space for metadata + Buffer.MemoryCopy(inputPtr + sizeof(int) + RespInputHeader.Size, + inputPtr + sizeof(int) + sizeof(long) + RespInputHeader.Size, isize, isize); + *(int*)inputPtr = sizeof(long) + RespInputHeader.Size + isize; + ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->cmd = cmd; + ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->flags = 0; + if (getValue) + ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->SetSetGetFlag(); + SpanByte.Reinterpret(inputPtr).ExtraMetadata = DateTimeOffset.UtcNow.Ticks + + (highPrecision + ? TimeSpan.FromMilliseconds(expiry).Ticks + : TimeSpan.FromSeconds(expiry).Ticks); + } + + SET_Conditional(ref newKey, ref Unsafe.AsRef(inputPtr), ref context); + + memoryHandle.Dispose(); + o.Memory.Dispose(); + + // Delete the old key + DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); + + returnStatus = GarnetStatus.OK; + } + } + finally + { + if (createTransaction) + txnManager.Commit(true); + } + } + + if ((storeType == StoreType.Object || storeType == StoreType.All) && !objectStoreBasicContext.IsNull) + { + createTransaction = false; + if (txnManager.state != TxnState.Running) + { + txnManager.SaveKeyEntryToLock(oldKeySlice, true, LockType.Exclusive); + txnManager.SaveKeyEntryToLock(newKeySlice, true, LockType.Exclusive); + txnManager.Run(true); + createTransaction = true; + } + + try + { + // TODO: (Nirmal) Use span if possible + byte[] oldKeyArray = oldKeySlice.ToArray(); + byte[] newKeyArray = newKeySlice.ToArray(); + + var status = GET(oldKeyArray, out var value, ref objectContext); + + if (status == GarnetStatus.OK) + { + var valObj = value.garnetObject; + + // TODO: (Nirmal) Change this to SETNX as well + SET(newKeyArray, valObj, ref objectContext); + + // Delete the old key + DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); + + returnStatus = GarnetStatus.OK; + } + } + finally + { + if (createTransaction) + txnManager.Commit(true); + } + } + + return returnStatus; + } + /// /// Returns if key is an existing one in the store. /// From 57e76191b341e765773a687056973caa2f08ae36 Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Tue, 10 Sep 2024 12:41:54 +0530 Subject: [PATCH 02/11] New changes from rename --- .../Storage/Session/MainStore/MainStoreOps.cs | 103 +++++++++--------- 1 file changed, 54 insertions(+), 49 deletions(-) diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index 6b8d5feccb..a00316599f 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -635,14 +635,12 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, var context = txnManager.LockableContext; var objectContext = txnManager.ObjectStoreLockableContext; + SpanByte oldKey = oldKeySlice.SpanByte; if (storeType == StoreType.Main || storeType == StoreType.All) { try { - SpanByte oldKey = oldKeySlice.SpanByte; - SpanByte newKey = newKeySlice.SpanByte; - SpanByte input = default; var o = new SpanByteAndMemory(); var status = GET(ref oldKey, ref input, ref o, ref context); @@ -654,48 +652,38 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, var ptrVal = (byte*)memoryHandle.Pointer; RespReadUtils.ReadUnsignedLengthHeader(out var headerLength, ref ptrVal, ptrVal + o.Length); - var value = SpanByte.FromPinnedPointer(ptrVal, headerLength); - // TODO: (Nirmal) Find the right way to get the inputPtr (pointer to input value). - byte* inputPtr = null; - // Make space for RespCommand in input - inputPtr -= RespInputHeader.Size; - var expiryBytes = new SpanByteAndMemory(); - var expiryStatus = TTL(ref oldKey, storeType, ref expiryBytes, ref context, ref objectContext, true); + // Find expiration time of the old key + var expireSpan = new SpanByteAndMemory(); + var ttlStatus = TTL(ref oldKey, storeType, ref expireSpan, ref context, ref objectContext, true); - if (expiry == 0) // no expiration provided - { - *(int*)inputPtr = RespInputHeader.Size + isize; - ((RespInputHeader*)(inputPtr + sizeof(int)))->cmd = cmd; - ((RespInputHeader*)(inputPtr + sizeof(int)))->flags = 0; - if (getValue) - ((RespInputHeader*)(inputPtr + sizeof(int)))->SetSetGetFlag(); - } - else + if (ttlStatus == GarnetStatus.OK && !expireSpan.IsSpanByte) { - // Move payload forward to make space for metadata - Buffer.MemoryCopy(inputPtr + sizeof(int) + RespInputHeader.Size, - inputPtr + sizeof(int) + sizeof(long) + RespInputHeader.Size, isize, isize); - *(int*)inputPtr = sizeof(long) + RespInputHeader.Size + isize; - ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->cmd = cmd; - ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->flags = 0; - if (getValue) - ((RespInputHeader*)(inputPtr + sizeof(int) + sizeof(long)))->SetSetGetFlag(); - SpanByte.Reinterpret(inputPtr).ExtraMetadata = DateTimeOffset.UtcNow.Ticks + - (highPrecision - ? TimeSpan.FromMilliseconds(expiry).Ticks - : TimeSpan.FromSeconds(expiry).Ticks); + using var expireMemoryHandle = expireSpan.Memory.Memory.Pin(); + var expirePtrVal = (byte*)expireMemoryHandle.Pointer; + RespReadUtils.TryRead64Int(out var expireTimeMs, ref expirePtrVal, expirePtrVal + expireSpan.Length, out var _); + + // If the key has an expiration, set the new key with the expiration + if (expireTimeMs > 0) + { + SETEX(newKeySlice, new ArgSlice(ptrVal, headerLength), TimeSpan.FromMilliseconds(expireTimeMs), ref context); + } + else if (expireTimeMs == -1) // Its possible to have expireTimeMs as 0 (Key expired or will be expired now) or -2 (Key does not exist), in those cases we don't SET the new key + { + SpanByte newKey = newKeySlice.SpanByte; + var value = SpanByte.FromPinnedPointer(ptrVal, headerLength); + SET_Conditional(ref newKey, ref value, ref context); + } + + expireSpan.Memory.Dispose(); + memoryHandle.Dispose(); + o.Memory.Dispose(); + + // Delete the old key + DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); + + returnStatus = GarnetStatus.OK; } - - SET_Conditional(ref newKey, ref Unsafe.AsRef(inputPtr), ref context); - - memoryHandle.Dispose(); - o.Memory.Dispose(); - - // Delete the old key - DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); - - returnStatus = GarnetStatus.OK; } } finally @@ -718,23 +706,40 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, try { - // TODO: (Nirmal) Use span if possible byte[] oldKeyArray = oldKeySlice.ToArray(); - byte[] newKeyArray = newKeySlice.ToArray(); - var status = GET(oldKeyArray, out var value, ref objectContext); if (status == GarnetStatus.OK) { var valObj = value.garnetObject; + byte[] newKeyArray = newKeySlice.ToArray(); - // TODO: (Nirmal) Change this to SETNX as well - SET(newKeyArray, valObj, ref objectContext); + var expireSpan = new SpanByteAndMemory(); + var ttlStatus = TTL(ref oldKey, StoreType.Object, ref expireSpan, ref context, ref objectContext, true); - // Delete the old key - DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); + if (ttlStatus == GarnetStatus.OK && !expireSpan.IsSpanByte) + { + using var expireMemoryHandle = expireSpan.Memory.Memory.Pin(); + var expirePtrVal = (byte*)expireMemoryHandle.Pointer; + RespReadUtils.TryRead64Int(out var expireTimeMs, ref expirePtrVal, expirePtrVal + expireSpan.Length, out var _); + expireSpan.Memory.Dispose(); + + if (expireTimeMs > 0) + { + SET(newKeyArray, valObj, ref objectContext); + EXPIRE(newKeySlice, TimeSpan.FromMilliseconds(expireTimeMs), out _, StoreType.Object, ExpireOption.None, ref context, ref objectContext, true); + } + else if (expireTimeMs == -1) // Its possible to have expire as 0 or -2, in those cases we don't SET the new key + { + SET(newKeyArray, valObj, ref objectContext); + } + + // Delete the old key + DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); + + returnStatus = GarnetStatus.OK; + } - returnStatus = GarnetStatus.OK; } } finally From cae61314b7073d5c8d822b073ec9b485183df8a3 Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Wed, 11 Sep 2024 12:22:35 +0530 Subject: [PATCH 03/11] Temp changes for calling SET_Conditional --- libs/server/Storage/Session/MainStore/MainStoreOps.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index a00316599f..5050a777c3 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -670,9 +670,13 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, } else if (expireTimeMs == -1) // Its possible to have expireTimeMs as 0 (Key expired or will be expired now) or -2 (Key does not exist), in those cases we don't SET the new key { - SpanByte newKey = newKeySlice.SpanByte; - var value = SpanByte.FromPinnedPointer(ptrVal, headerLength); - SET_Conditional(ref newKey, ref value, ref context); + var setValue = scratchBufferManager.FormatScratch(RespInputHeader.Size, new ArgSlice(ptrVal, headerLength)); + var setValuePtr = setValue.SpanByte.ToPointerWithMetadata(); + ((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; + ((RespInputHeader*)setValuePtr)->flags = 0; + var newKey = newKeySlice.SpanByte; + var setValueSpan = setValue.SpanByte; // TODO: need to add length in the start and create a new span + SET_Conditional(ref newKey, ref setValueSpan, ref context); } expireSpan.Memory.Dispose(); From ad8feeb6702de58b3ff858ca274b89ee884886bf Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Thu, 12 Sep 2024 00:50:57 +0530 Subject: [PATCH 04/11] Added basic test --- libs/server/Resp/Parser/RespCommand.cs | 2 +- libs/server/Resp/RespServerSession.cs | 1 + test/Garnet.test/RespTests.cs | 134 ++++++++++++++++++++++++- 3 files changed, 134 insertions(+), 3 deletions(-) diff --git a/libs/server/Resp/Parser/RespCommand.cs b/libs/server/Resp/Parser/RespCommand.cs index fd912a555a..cf087de3db 100644 --- a/libs/server/Resp/Parser/RespCommand.cs +++ b/libs/server/Resp/Parser/RespCommand.cs @@ -621,7 +621,7 @@ private RespCommand FastParseCommand(out int count) (2 << 4) | 6 when lastWord == MemoryMarshal.Read("INCRBY\r\n"u8) => RespCommand.INCRBY, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("DECRBY\r\n"u8) => RespCommand.DECRBY, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("RENAME\r\n"u8) => RespCommand.RENAME, - (2 << 4) | 8 when lastWord == MemoryMarshal.Read("RENAMENX\r\n"u8) => RespCommand.RENAMENX, + (2 << 4) | 8 when lastWord == MemoryMarshal.Read("NAMENX\r\n"u8) => RespCommand.RENAMENX, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("GETBIT\r\n"u8) => RespCommand.GETBIT, (2 << 4) | 6 when lastWord == MemoryMarshal.Read("APPEND\r\n"u8) => RespCommand.APPEND, (2 << 4) | 7 when lastWord == MemoryMarshal.Read("UBLISH\r\n"u8) && ptr[8] == 'P' => RespCommand.PUBLISH, diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs index c0ffdce91c..7a1df7fa22 100644 --- a/libs/server/Resp/RespServerSession.cs +++ b/libs/server/Resp/RespServerSession.cs @@ -505,6 +505,7 @@ private bool ProcessBasicCommands(RespCommand cmd, ref TGarnetApi st RespCommand.SETEXNX => NetworkSETEXNX(ref storageApi), RespCommand.DEL => NetworkDEL(ref storageApi), RespCommand.RENAME => NetworkRENAME(ref storageApi), + RespCommand.RENAMENX => NetworkRENAMENX(ref storageApi), RespCommand.EXISTS => NetworkEXISTS(ref storageApi), RespCommand.EXPIRE => NetworkEXPIRE(RespCommand.EXPIRE, ref storageApi), RespCommand.PEXPIRE => NetworkEXPIRE(RespCommand.PEXPIRE, ref storageApi), diff --git a/test/Garnet.test/RespTests.cs b/test/Garnet.test/RespTests.cs index 0ff2e60ba8..fae9d4d6f2 100644 --- a/test/Garnet.test/RespTests.cs +++ b/test/Garnet.test/RespTests.cs @@ -1178,7 +1178,7 @@ public void SingleRenameWithExpiry() var ttl = db.KeyTimeToLive("key2"); ClassicAssert.IsTrue(ttl.HasValue); ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); - ClassicAssert.Less(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); } [Test] @@ -1266,9 +1266,139 @@ public void SingleRenameObjectStoreWithExpiry() var ttl = db.KeyTimeToLive("lkey2"); ClassicAssert.IsTrue(ttl.HasValue); ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); - ClassicAssert.Less(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); } + #region RENAMENX + + [Test] + public void SingleRenameNx() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + string origValue = "test1"; + db.StringSet("key1", origValue); + + db.KeyRename("key1", "key2", When.NotExists); + string retValue = db.StringGet("key2"); + + ClassicAssert.AreEqual(origValue, retValue); + + origValue = db.StringGet("key1"); + ClassicAssert.AreEqual(null, origValue); + } + + [Test] + public void SingleRenameNxWithExpiry() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var origValue = "test1"; + db.StringSet("key1", origValue, TimeSpan.FromMinutes(1)); + + db.KeyRename("key1", "key2", When.NotExists); + string retValue = db.StringGet("key2"); + + ClassicAssert.AreEqual(origValue, retValue); + + var ttl = db.KeyTimeToLive("key2"); + ClassicAssert.IsTrue(ttl.HasValue); + ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + } + + [Test] + public void SingleRenameNxKeyEdgeCase([Values] bool withoutObjectStore) + { + if (withoutObjectStore) + { + TearDown(); + TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); + server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, DisableObjects: true); + server.Start(); + } + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + //1. Key rename does not exist + try + { + var res = db.KeyRename("key1", "key2", When.NotExists); + } + catch (Exception ex) + { + ClassicAssert.AreEqual("ERR no such key", ex.Message); + } + + //2. Key rename oldKey.Equals(newKey) + string origValue = "test1"; + db.StringSet("key1", origValue); + bool renameRes = db.KeyRename("key1", "key1", When.NotExists); + ClassicAssert.IsTrue(renameRes); + string retValue = db.StringGet("key1"); + ClassicAssert.AreEqual(origValue, retValue); + } + + [Test] + public void SingleRenameNxObjectStore() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var origList = new RedisValue[] { "a", "b", "c", "d" }; + var key1 = "lkey1"; + var count = db.ListRightPush(key1, origList); + ClassicAssert.AreEqual(4, count); + + var result = db.ListRange(key1); + ClassicAssert.AreEqual(origList, result); + + var key2 = "lkey2"; + var rb = db.KeyRename(key1, key2, When.NotExists); + ClassicAssert.IsTrue(rb); + result = db.ListRange(key1); + ClassicAssert.AreEqual(Array.Empty(), result); + + result = db.ListRange(key2); + ClassicAssert.AreEqual(origList, result); + } + + [Test] + public void SingleRenameNxObjectStoreWithExpiry() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var origList = new RedisValue[] { "a", "b", "c", "d" }; + var key1 = "lkey1"; + var count = db.ListRightPush(key1, origList); + ClassicAssert.AreEqual(4, count); + + var result = db.ListRange(key1); + ClassicAssert.AreEqual(origList, result); + + var expirySet = db.KeyExpire("lkey1", TimeSpan.FromMinutes(1)); + ClassicAssert.IsTrue(expirySet); + + var key2 = "lkey2"; + var rb = db.KeyRename(key1, key2, When.NotExists); + ClassicAssert.IsTrue(rb); + result = db.ListRange(key1); + ClassicAssert.AreEqual(Array.Empty(), result); + + result = db.ListRange(key2); + ClassicAssert.AreEqual(origList, result); + + var ttl = db.KeyTimeToLive("lkey2"); + ClassicAssert.IsTrue(ttl.HasValue); + ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + } + + #endregion + [Test] public void CanSelectCommand() { From dac162ca2f60465def24054e55007d9f12c95d62 Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Thu, 12 Sep 2024 01:49:18 +0530 Subject: [PATCH 05/11] Updated CommandsInfo --- libs/server/Resp/RespCommandsInfo.json | 43 +++++++++++++++++++ .../CommandInfoUpdater/SupportedCommand.cs | 1 + 2 files changed, 44 insertions(+) diff --git a/libs/server/Resp/RespCommandsInfo.json b/libs/server/Resp/RespCommandsInfo.json index 65204e8205..a0f85d937e 100644 --- a/libs/server/Resp/RespCommandsInfo.json +++ b/libs/server/Resp/RespCommandsInfo.json @@ -3573,6 +3573,49 @@ ], "SubCommands": null }, + { + "Command": "RENAMENX", + "Name": "RENAMENX", + "IsInternal": false, + "Arity": 3, + "Flags": "Fast, Write", + "FirstKey": 1, + "LastKey": 2, + "Step": 1, + "AclCategories": "Fast, KeySpace, Write", + "Tips": null, + "KeySpecifications": [ + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 1 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Notes": null, + "Flags": "RW, Access, Delete" + }, + { + "BeginSearch": { + "TypeDiscriminator": "BeginSearchIndex", + "Index": 2 + }, + "FindKeys": { + "TypeDiscriminator": "FindKeysRange", + "LastKey": 0, + "KeyStep": 1, + "Limit": 0 + }, + "Notes": null, + "Flags": "OW, Insert" + } + ], + "SubCommands": null + }, { "Command": "REPLICAOF", "Name": "REPLICAOF", diff --git a/playground/CommandInfoUpdater/SupportedCommand.cs b/playground/CommandInfoUpdater/SupportedCommand.cs index db39c684fd..81115f74a6 100644 --- a/playground/CommandInfoUpdater/SupportedCommand.cs +++ b/playground/CommandInfoUpdater/SupportedCommand.cs @@ -194,6 +194,7 @@ public class SupportedCommand new("READONLY", RespCommand.READONLY), new("READWRITE", RespCommand.READWRITE), new("RENAME", RespCommand.RENAME), + new("RENAMENX", RespCommand.RENAMENX), new("REPLICAOF", RespCommand.REPLICAOF), new("RPOP", RespCommand.RPOP), new("RPOPLPUSH", RespCommand.RPOPLPUSH), From 5263ed17634ca555babbf759e7386612c688886d Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Fri, 13 Sep 2024 01:32:15 +0530 Subject: [PATCH 06/11] Changes done for non object keys --- .../Storage/Session/MainStore/MainStoreOps.cs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index b3fb8f8448..277e8a3ea1 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -653,6 +653,10 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St } } + // TODO: (Nirmal) + // Integer reply: 1 if key was renamed to newkey. + // Integer reply: 0 if newkey already exists. + return returnStatus; } @@ -706,16 +710,27 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, // If the key has an expiration, set the new key with the expiration if (expireTimeMs > 0) { + // Move payload forward to make space for RespInputHeader and Metadata + var setValue = scratchBufferManager.FormatScratch(RespInputHeader.Size + sizeof(long), new ArgSlice(ptrVal, headerLength)); + var setValueSpan = setValue.SpanByte; + var setValuePtr = setValueSpan.ToPointerWithMetadata(); + setValueSpan.ExtraMetadata = DateTimeOffset.UtcNow.Ticks + TimeSpan.FromMilliseconds(expireTimeMs).Ticks; + ((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; + ((RespInputHeader*)setValuePtr)->flags = 0; + var newKey = newKeySlice.SpanByte; + GC.Collect(2, GCCollectionMode.Forced, true, true); SETEX(newKeySlice, new ArgSlice(ptrVal, headerLength), TimeSpan.FromMilliseconds(expireTimeMs), ref context); } else if (expireTimeMs == -1) // Its possible to have expireTimeMs as 0 (Key expired or will be expired now) or -2 (Key does not exist), in those cases we don't SET the new key { + // Move payload forward to make space for RespInputHeader var setValue = scratchBufferManager.FormatScratch(RespInputHeader.Size, new ArgSlice(ptrVal, headerLength)); - var setValuePtr = setValue.SpanByte.ToPointerWithMetadata(); + var setValueSpan = setValue.SpanByte; + var setValuePtr = setValueSpan.ToPointerWithMetadata(); ((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; ((RespInputHeader*)setValuePtr)->flags = 0; var newKey = newKeySlice.SpanByte; - var setValueSpan = setValue.SpanByte; // TODO: need to add length in the start and create a new span + GC.Collect(2, GCCollectionMode.Forced, true, true); // TODO: (Nirmal) Romove it, added to check if any memory need to be pinned SET_Conditional(ref newKey, ref setValueSpan, ref context); } From 33e9b8c0ee45a6a40a820ac5273671a8cd93d20b Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Sat, 14 Sep 2024 12:34:44 +0530 Subject: [PATCH 07/11] Final commit --- libs/server/Resp/KeyAdminCommands.cs | 26 ++- .../Storage/Session/MainStore/MainStoreOps.cs | 97 ++++----- test/Garnet.test/RespTests.cs | 189 ++++++++++++++---- website/docs/commands/api-compatibility.md | 2 +- website/docs/commands/generic-commands.md | 20 ++ 5 files changed, 231 insertions(+), 103 deletions(-) diff --git a/libs/server/Resp/KeyAdminCommands.cs b/libs/server/Resp/KeyAdminCommands.cs index 8a09deba0a..f542fea944 100644 --- a/libs/server/Resp/KeyAdminCommands.cs +++ b/libs/server/Resp/KeyAdminCommands.cs @@ -46,7 +46,6 @@ private bool NetworkRENAME(ref TGarnetApi storageApi) private bool NetworkRENAMENX(ref TGarnetApi storageApi) where TGarnetApi : IGarnetApi { - // TODO: (Nirmal) If need combine the RENAME and RENAMENX commands by passing RespCommand command. Refer: HashSet, HashCommands.cs if (parseState.Count != 2) { return AbortWithWrongNumberOfArguments(nameof(RespCommand.RENAMENX)); @@ -54,12 +53,29 @@ private bool NetworkRENAMENX(ref TGarnetApi storageApi) var oldKeySlice = parseState.GetArgSliceByRef(0); var newKeySlice = parseState.GetArgSliceByRef(1); - // TODO: (Nirmal) If need combine the RENAME and RENAMENX commands by using out param. Refer: HashSet, HashCommands.cs var status = storageApi.RENAMENX(oldKeySlice, newKeySlice); - // TODO: (Nirmal) Add int response for the RENAMENX command by adding first param - while (!RespWriteUtils.WriteInteger(1, ref dcurr, dend)) - SendAndReset(); + // Integer reply: 1 if key was renamed to newkey. + // Integer reply: 0 if newkey already exists. + switch (status) + { + // GarnetStatus OK means new key already exists + case GarnetStatus.OK: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_1, ref dcurr, dend)) + SendAndReset(); + break; + + // GarnetStatus NOTFOUND means remane operation is successful + case GarnetStatus.NOTFOUND: + while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_GENERIC_NOSUCHKEY, ref dcurr, dend)) + SendAndReset(); + break; + + case GarnetStatus.MOVED: + while (!RespWriteUtils.WriteDirect(CmdStrings.RESP_RETURN_VAL_0, ref dcurr, dend)) + SendAndReset(); + break; + } return true; } diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index 277e8a3ea1..d77857e13c 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -618,32 +618,13 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St var valObj = value.garnetObject; byte[] newKeyArray = newKeySlice.ToArray(); - var expireSpan = new SpanByteAndMemory(); - var ttlStatus = TTL(ref oldKey, StoreType.Object, ref expireSpan, ref context, ref objectContext, true); + // valObj already has expiration time, so no need to write expiration logic here + SET(newKeyArray, valObj, ref objectContext); - if (ttlStatus == GarnetStatus.OK && !expireSpan.IsSpanByte) - { - using var expireMemoryHandle = expireSpan.Memory.Memory.Pin(); - var expirePtrVal = (byte*)expireMemoryHandle.Pointer; - RespReadUtils.TryRead64Int(out var expireTimeMs, ref expirePtrVal, expirePtrVal + expireSpan.Length, out var _); - expireSpan.Memory.Dispose(); - - if (expireTimeMs > 0) - { - SET(newKeyArray, valObj, ref objectContext); - EXPIRE(newKeySlice, TimeSpan.FromMilliseconds(expireTimeMs), out _, StoreType.Object, ExpireOption.None, ref context, ref objectContext, true); - } - else if (expireTimeMs == -1) // Its possible to have expire as 0 or -2, in those cases we don't SET the new key - { - SET(newKeyArray, valObj, ref objectContext); - } - - // Delete the old key - DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); - - returnStatus = GarnetStatus.OK; - } + // Delete the old key + DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); + returnStatus = GarnetStatus.OK; } } finally @@ -653,13 +634,20 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St } } - // TODO: (Nirmal) - // Integer reply: 1 if key was renamed to newkey. - // Integer reply: 0 if newkey already exists. - return returnStatus; } + /// + /// Renames a key if the new key does not already exist. + /// + /// The old key to be renamed. + /// The new key name. + /// The type of store to perform the operation on. + /// + /// GarnetStatus OK means rename operation is successful + /// GarnetStatus NOTFOUND means old key does not exists + /// GarnetStatus MOVED means new key already exists + /// public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, StoreType storeType) { GarnetStatus returnStatus = GarnetStatus.NOTFOUND; @@ -715,11 +703,13 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, var setValueSpan = setValue.SpanByte; var setValuePtr = setValueSpan.ToPointerWithMetadata(); setValueSpan.ExtraMetadata = DateTimeOffset.UtcNow.Ticks + TimeSpan.FromMilliseconds(expireTimeMs).Ticks; - ((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; - ((RespInputHeader*)setValuePtr)->flags = 0; + ((RespInputHeader*)(setValuePtr + sizeof(long)))->cmd = RespCommand.SETEXNX; + ((RespInputHeader*)(setValuePtr + sizeof(long)))->flags = 0; var newKey = newKeySlice.SpanByte; - GC.Collect(2, GCCollectionMode.Forced, true, true); - SETEX(newKeySlice, new ArgSlice(ptrVal, headerLength), TimeSpan.FromMilliseconds(expireTimeMs), ref context); + var setStatus = SET_Conditional(ref newKey, ref setValueSpan, ref context); + + // For SET NX `NOTFOUND` means the operation succeeded + returnStatus = setStatus == GarnetStatus.NOTFOUND ? GarnetStatus.OK : GarnetStatus.MOVED; } else if (expireTimeMs == -1) // Its possible to have expireTimeMs as 0 (Key expired or will be expired now) or -2 (Key does not exist), in those cases we don't SET the new key { @@ -730,18 +720,21 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, ((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; ((RespInputHeader*)setValuePtr)->flags = 0; var newKey = newKeySlice.SpanByte; - GC.Collect(2, GCCollectionMode.Forced, true, true); // TODO: (Nirmal) Romove it, added to check if any memory need to be pinned - SET_Conditional(ref newKey, ref setValueSpan, ref context); + var setStatus = SET_Conditional(ref newKey, ref setValueSpan, ref context); + + // For SET NX `NOTFOUND` means the operation succeeded + returnStatus = setStatus == GarnetStatus.NOTFOUND ? GarnetStatus.OK : GarnetStatus.MOVED; } expireSpan.Memory.Dispose(); memoryHandle.Dispose(); o.Memory.Dispose(); - // Delete the old key - DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); - - returnStatus = GarnetStatus.OK; + // Delete the old key only when SET NX succeeded + if (returnStatus == GarnetStatus.OK) + { + DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); + } } } } @@ -773,32 +766,23 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, var valObj = value.garnetObject; byte[] newKeyArray = newKeySlice.ToArray(); - var expireSpan = new SpanByteAndMemory(); - var ttlStatus = TTL(ref oldKey, StoreType.Object, ref expireSpan, ref context, ref objectContext, true); + // Not using EXISTS method to avoid new allocation of Array for key + var getNewStatus = GET(newKeyArray, out _, ref objectContext); - if (ttlStatus == GarnetStatus.OK && !expireSpan.IsSpanByte) + if (getNewStatus == GarnetStatus.NOTFOUND) { - using var expireMemoryHandle = expireSpan.Memory.Memory.Pin(); - var expirePtrVal = (byte*)expireMemoryHandle.Pointer; - RespReadUtils.TryRead64Int(out var expireTimeMs, ref expirePtrVal, expirePtrVal + expireSpan.Length, out var _); - expireSpan.Memory.Dispose(); - - if (expireTimeMs > 0) - { - SET(newKeyArray, valObj, ref objectContext); - EXPIRE(newKeySlice, TimeSpan.FromMilliseconds(expireTimeMs), out _, StoreType.Object, ExpireOption.None, ref context, ref objectContext, true); - } - else if (expireTimeMs == -1) // Its possible to have expire as 0 or -2, in those cases we don't SET the new key - { - SET(newKeyArray, valObj, ref objectContext); - } + // valObj already has expiration time, so no need to write expiration logic here + SET(newKeyArray, valObj, ref objectContext); // Delete the old key DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext); returnStatus = GarnetStatus.OK; } - + else + { + returnStatus = GarnetStatus.MOVED; + } } } finally @@ -807,7 +791,6 @@ public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, txnManager.Commit(true); } } - return returnStatus; } diff --git a/test/Garnet.test/RespTests.cs b/test/Garnet.test/RespTests.cs index fae9d4d6f2..9d6436fa7d 100644 --- a/test/Garnet.test/RespTests.cs +++ b/test/Garnet.test/RespTests.cs @@ -1269,6 +1269,22 @@ public void SingleRenameObjectStoreWithExpiry() ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); } + [Test] + public void SingleRenameWithOldKeyAndNewKeyAsSame() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var origValue = "test1"; + var key = "key1"; + db.StringSet(key, origValue); + + var result = db.KeyRename(key, key); + + ClassicAssert.IsTrue(result); + string retValue = db.StringGet(key); + ClassicAssert.AreEqual(origValue, retValue); + } + #region RENAMENX [Test] @@ -1280,15 +1296,37 @@ public void SingleRenameNx() string origValue = "test1"; db.StringSet("key1", origValue); - db.KeyRename("key1", "key2", When.NotExists); - string retValue = db.StringGet("key2"); + var result = db.KeyRename("key1", "key2", When.NotExists); + ClassicAssert.IsTrue(result); + string retValue = db.StringGet("key2"); ClassicAssert.AreEqual(origValue, retValue); origValue = db.StringGet("key1"); ClassicAssert.AreEqual(null, origValue); } + [Test] + public void SingleRenameNxWithNewKeyAlreadyExist() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + string origValue = "test1"; + string origValue2 = "test2"; + db.StringSet("key1", origValue); + db.StringSet("key2", origValue2); + + var result = db.KeyRename("key1", "key2", When.NotExists); + ClassicAssert.IsFalse(result); + + string retValue2 = db.StringGet("key2"); + ClassicAssert.AreEqual(origValue2, retValue2); + + string retValue1 = db.StringGet("key1"); + ClassicAssert.AreEqual(origValue, retValue1); + } + [Test] public void SingleRenameNxWithExpiry() { @@ -1298,9 +1336,10 @@ public void SingleRenameNxWithExpiry() var origValue = "test1"; db.StringSet("key1", origValue, TimeSpan.FromMinutes(1)); - db.KeyRename("key1", "key2", When.NotExists); - string retValue = db.StringGet("key2"); + var result = db.KeyRename("key1", "key2", When.NotExists); + ClassicAssert.IsTrue(result); + string retValue = db.StringGet("key2"); ClassicAssert.AreEqual(origValue, retValue); var ttl = db.KeyTimeToLive("key2"); @@ -1310,35 +1349,29 @@ public void SingleRenameNxWithExpiry() } [Test] - public void SingleRenameNxKeyEdgeCase([Values] bool withoutObjectStore) + public void SingleRenameNxWithExpiryAndNewKeyAlreadyExist() { - if (withoutObjectStore) - { - TearDown(); - TestUtils.DeleteDirectory(TestUtils.MethodTestDir, wait: true); - server = TestUtils.CreateGarnetServer(TestUtils.MethodTestDir, DisableObjects: true); - server.Start(); - } using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); - //1. Key rename does not exist - try - { - var res = db.KeyRename("key1", "key2", When.NotExists); - } - catch (Exception ex) - { - ClassicAssert.AreEqual("ERR no such key", ex.Message); - } + var origValue = "test1"; + string origValue2 = "test2"; + db.StringSet("key1", origValue, TimeSpan.FromMinutes(1)); + db.StringSet("key2", origValue2, TimeSpan.FromMinutes(1)); - //2. Key rename oldKey.Equals(newKey) - string origValue = "test1"; - db.StringSet("key1", origValue); - bool renameRes = db.KeyRename("key1", "key1", When.NotExists); - ClassicAssert.IsTrue(renameRes); - string retValue = db.StringGet("key1"); - ClassicAssert.AreEqual(origValue, retValue); + var result = db.KeyRename("key1", "key2", When.NotExists); + ClassicAssert.IsFalse(result); + + string retValue = db.StringGet("key2"); + ClassicAssert.AreEqual(origValue2, retValue); + + var ttl = db.KeyTimeToLive("key2"); + ClassicAssert.IsTrue(ttl.HasValue); + ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + + string retValue1 = db.StringGet("key1"); + CollectionAssert.AreEqual(origValue, retValue1); } [Test] @@ -1350,51 +1383,127 @@ public void SingleRenameNxObjectStore() var origList = new RedisValue[] { "a", "b", "c", "d" }; var key1 = "lkey1"; var count = db.ListRightPush(key1, origList); - ClassicAssert.AreEqual(4, count); - var result = db.ListRange(key1); - ClassicAssert.AreEqual(origList, result); - var key2 = "lkey2"; + var rb = db.KeyRename(key1, key2, When.NotExists); ClassicAssert.IsTrue(rb); + result = db.ListRange(key1); - ClassicAssert.AreEqual(Array.Empty(), result); + CollectionAssert.AreEqual(Array.Empty(), result); result = db.ListRange(key2); - ClassicAssert.AreEqual(origList, result); + CollectionAssert.AreEqual(origList, result); } [Test] - public void SingleRenameNxObjectStoreWithExpiry() + public void SingleRenameNxObjectStoreWithNewKeyAlreadyExist() { using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); var origList = new RedisValue[] { "a", "b", "c", "d" }; + var origList2 = new RedisValue[] { "z", "y", "z" }; var key1 = "lkey1"; - var count = db.ListRightPush(key1, origList); - ClassicAssert.AreEqual(4, count); + var key2 = "lkey2"; + db.ListRightPush(key1, origList); + db.ListRightPush(key2, origList2); + + var rb = db.KeyRename(key1, key2, When.NotExists); + ClassicAssert.IsFalse(rb); var result = db.ListRange(key1); ClassicAssert.AreEqual(origList, result); - var expirySet = db.KeyExpire("lkey1", TimeSpan.FromMinutes(1)); - ClassicAssert.IsTrue(expirySet); + result = db.ListRange(key2); + ClassicAssert.AreEqual(origList2, result); + } + + [Test] + public void SingleRenameNxObjectStoreWithExpiry() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var origList = new RedisValue[] { "a", "b", "c", "d" }; + var key1 = "lkey1"; + var count = db.ListRightPush(key1, origList); + var result = db.ListRange(key1); + var expirySet = db.KeyExpire("lkey1", TimeSpan.FromMinutes(1)); var key2 = "lkey2"; + var rb = db.KeyRename(key1, key2, When.NotExists); ClassicAssert.IsTrue(rb); + result = db.ListRange(key1); ClassicAssert.AreEqual(Array.Empty(), result); result = db.ListRange(key2); ClassicAssert.AreEqual(origList, result); - var ttl = db.KeyTimeToLive("lkey2"); + var ttl = db.KeyTimeToLive(key2); + ClassicAssert.IsTrue(ttl.HasValue); + ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); + ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + } + + [Test] + public void SingleRenameNxObjectStoreWithExpiryAndNewKeyAlreadyExist() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var origList = new RedisValue[] { "a", "b", "c", "d" }; + var origList2 = new RedisValue[] { "x", "y", "z" }; + var key1 = "lkey1"; + var key2 = "lkey2"; + db.ListRightPush(key1, origList); + db.ListRightPush(key2, origList2); + var result = db.ListRange(key1); + var expirySet = db.KeyExpire(key1, TimeSpan.FromMinutes(1)); + + var rb = db.KeyRename(key1, key2, When.NotExists); + ClassicAssert.IsFalse(rb); + + result = db.ListRange(key1); + ClassicAssert.AreEqual(origList, result); + + result = db.ListRange(key2); + ClassicAssert.AreEqual(origList2, result); + + var ttl = db.KeyTimeToLive(key1); ClassicAssert.IsTrue(ttl.HasValue); ClassicAssert.Greater(ttl.Value.TotalMilliseconds, 0); ClassicAssert.LessOrEqual(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds); + + var ttl2 = db.KeyTimeToLive(key2); + ClassicAssert.IsFalse(ttl2.HasValue); + } + + [Test] + public void SingleRenameNxWithKeyNotExist() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var exception = Assert.Throws(() => db.KeyRename("key1", "key2", When.NotExists)); + ClassicAssert.AreEqual("ERR no such key", exception.Message); + } + + [Test] + public void SingleRenameNxWithOldKeyAndNewKeyAsSame() + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + var origValue = "test1"; + var key = "key1"; + db.StringSet(key, origValue); + + var result = db.KeyRename(key, key, When.NotExists); + + ClassicAssert.IsTrue(result); + string retValue = db.StringGet(key); + ClassicAssert.AreEqual(origValue, retValue); } #endregion diff --git a/website/docs/commands/api-compatibility.md b/website/docs/commands/api-compatibility.md index af95155613..00ed34db2d 100644 --- a/website/docs/commands/api-compatibility.md +++ b/website/docs/commands/api-compatibility.md @@ -99,7 +99,7 @@ Note that this list is subject to change as we continue to expand our API comman | | [PTTL](generic-commands.md#pttl) | ➕ | | | | RANDOMKEY | ➖ | | | | [RENAME](generic-commands.md#rename) | ➕ | | -| | RENAMENX | ➖ | | +| | [RENAMENX](generic-commands.md#renamenx) | ➕ | | | | RESTORE | ➖ | | | | [SCAN](generic-commands.md#scan) | ➕ | | | | SORT | ➖ | | diff --git a/website/docs/commands/generic-commands.md b/website/docs/commands/generic-commands.md index 8eecbfdede..2e4a330047 100644 --- a/website/docs/commands/generic-commands.md +++ b/website/docs/commands/generic-commands.md @@ -263,6 +263,26 @@ Renames key to newkey. It returns an error when key does not exist. If newkey al Simple string reply: OK. --- + +### RENAMENX + +#### Syntax + +```bash + RENAME key newkey +``` + +Renames key to newkey if newkey does not yet exist. It returns an error when key does not exist. + +#### Resp Reply + +One of the following: + +* Integer reply: 1 if key was renamed to newkey. +* Integer reply: 0 if newkey already exists. + +--- + ### SCAN #### Syntax From 669fb34ef4b141b60a58f9937520dc586138855c Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Sat, 14 Sep 2024 12:41:21 +0530 Subject: [PATCH 08/11] Added proper comments --- libs/server/API/IGarnetApi.cs | 12 ++++++++---- .../server/Storage/Session/MainStore/MainStoreOps.cs | 2 +- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/libs/server/API/IGarnetApi.cs b/libs/server/API/IGarnetApi.cs index 67403cbb02..146c303483 100644 --- a/libs/server/API/IGarnetApi.cs +++ b/libs/server/API/IGarnetApi.cs @@ -122,10 +122,14 @@ public interface IGarnetApi : IGarnetReadApi, IGarnetAdvancedApi /// /// Renames key to newkey if newkey does not yet exist. It returns an error when key does not exist. /// - /// - /// - /// - /// + /// The old key to be renamed. + /// The new key name. + /// The type of store to perform the operation on. + /// + /// GarnetStatus OK means rename operation is successful + /// GarnetStatus NOTFOUND means old key does not exists + /// GarnetStatus MOVED means new key already exists + /// GarnetStatus RENAMENX(ArgSlice oldKey, ArgSlice newKey, StoreType storeType = StoreType.All); #endregion diff --git a/libs/server/Storage/Session/MainStore/MainStoreOps.cs b/libs/server/Storage/Session/MainStore/MainStoreOps.cs index d77857e13c..4520bb4867 100644 --- a/libs/server/Storage/Session/MainStore/MainStoreOps.cs +++ b/libs/server/Storage/Session/MainStore/MainStoreOps.cs @@ -638,7 +638,7 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St } /// - /// Renames a key if the new key does not already exist. + /// Renames key to newkey if newkey does not yet exist. It returns an error when key does not exist. /// /// The old key to be renamed. /// The new key name. From c9d3aff1b641adb5504d0b070538aaf22b1124ca Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Sat, 14 Sep 2024 18:48:58 +0530 Subject: [PATCH 09/11] Code format fix --- libs/server/Resp/KeyAdminCommands.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/server/Resp/KeyAdminCommands.cs b/libs/server/Resp/KeyAdminCommands.cs index f542fea944..b24596ca1f 100644 --- a/libs/server/Resp/KeyAdminCommands.cs +++ b/libs/server/Resp/KeyAdminCommands.cs @@ -51,7 +51,7 @@ private bool NetworkRENAMENX(ref TGarnetApi storageApi) return AbortWithWrongNumberOfArguments(nameof(RespCommand.RENAMENX)); } - var oldKeySlice = parseState.GetArgSliceByRef(0); + var oldKeySlice = parseState.GetArgSliceByRef(0); var newKeySlice = parseState.GetArgSliceByRef(1); var status = storageApi.RENAMENX(oldKeySlice, newKeySlice); From 22581e6a0a2dbdb8b208f42975c34c1d0bd4f6b2 Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Sat, 14 Sep 2024 19:31:59 +0530 Subject: [PATCH 10/11] Added ACL test for renamenx --- test/Garnet.test/Resp/ACL/RespCommandTests.cs | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index e616dd5888..259f548a20 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -4300,6 +4300,33 @@ static async Task DoPTTLAsync(GarnetClient client) } } + [Test] + public async Task RenameNxACLsAsync() + { + await CheckCommandsAsync( + "RENAMENX", + [DoPTTLAsync] + ); + + static async Task DoPTTLAsync(GarnetClient client) + { + try + { + await client.ExecuteForStringResultAsync("RENAMENX", ["foo", "bar"]); + Assert.Fail("Shouldn't succeed, key doesn't exist"); + } + catch (Exception e) + { + if (e.Message == "ERR no such key") + { + return; + } + + throw; + } + } + } + [Test] public async Task ReplicaOfACLsAsync() { From ce3aa4360537943401b10ea9c73a609528b13e16 Mon Sep 17 00:00:00 2001 From: Vijay-Nirmal Date: Mon, 16 Sep 2024 00:28:18 +0530 Subject: [PATCH 11/11] Corrected the function name --- test/Garnet.test/Resp/ACL/RespCommandTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Garnet.test/Resp/ACL/RespCommandTests.cs b/test/Garnet.test/Resp/ACL/RespCommandTests.cs index 259f548a20..c79e075f68 100644 --- a/test/Garnet.test/Resp/ACL/RespCommandTests.cs +++ b/test/Garnet.test/Resp/ACL/RespCommandTests.cs @@ -4278,10 +4278,10 @@ public async Task RenameACLsAsync() { await CheckCommandsAsync( "RENAME", - [DoPTTLAsync] + [DoRENAMEAsync] ); - static async Task DoPTTLAsync(GarnetClient client) + static async Task DoRENAMEAsync(GarnetClient client) { try { @@ -4305,10 +4305,10 @@ public async Task RenameNxACLsAsync() { await CheckCommandsAsync( "RENAMENX", - [DoPTTLAsync] + [DoRENAMENXAsync] ); - static async Task DoPTTLAsync(GarnetClient client) + static async Task DoRENAMENXAsync(GarnetClient client) { try {