From ab961c1d0ff95fd9b4dfec99247e7e712ba64d9f Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sat, 13 Apr 2024 10:43:02 +0800 Subject: [PATCH 01/11] fix: operation direction parse error --- libs/server/Objects/List/ListObject.cs | 1 + libs/server/Resp/Objects/ListCommands.cs | 8 +++++-- libs/server/Resp/Objects/ObjectStoreUtils.cs | 23 ++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/libs/server/Objects/List/ListObject.cs b/libs/server/Objects/List/ListObject.cs index 358e635fd9..23012385f5 100644 --- a/libs/server/Objects/List/ListObject.cs +++ b/libs/server/Objects/List/ListObject.cs @@ -47,6 +47,7 @@ public enum OperationDirection /// Right or tail /// Right, + Unknown, } diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index d9749ec508..2f889bc164 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -679,8 +679,12 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) return false; - var sourceDirection = param1.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left; - var destinationDirection = param2.ReadOnlySpan.SequenceEqual("RIGHT"u8) ? OperationDirection.Right : OperationDirection.Left; + OperationDirection sourceDirection = GetOperationDirection(param1.ReadOnlySpan); + OperationDirection destinationDirection = GetOperationDirection(param2.ReadOnlySpan); + if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) + { + return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); + } result = ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, ref storageApi); if (node != null) diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index fe93930e4a..3cf5a57009 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -65,5 +65,28 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) return true; } + + /// + /// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection. + /// If parsing fails, returns OperationDirection.Unknown. + /// + /// The input to parse. + /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. + public OperationDirection GetOperationDirection(ReadOnlySpan input) + { + string inputString = Encoding.UTF8.GetString(input).ToUpperInvariant(); + if (inputString == "RIGHT") + { + return OperationDirection.Right; + } + else if (inputString == "LEFT") + { + return OperationDirection.Left; + } + else + { + return OperationDirection.Unknown; + } + } } } \ No newline at end of file From ca41892f1216e266a2a2fa11f89a4dec840744d3 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sat, 13 Apr 2024 10:53:51 +0800 Subject: [PATCH 02/11] fix format --- libs/server/Resp/Objects/ListCommands.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index 2f889bc164..23f3cc4412 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -684,7 +684,7 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) { return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); - } + } result = ListMove(count, sourceKey, destinationKey, sourceDirection, destinationDirection, out var node, ref storageApi); if (node != null) From caf3eb3573e8c0b5362a9d03c2e1f8015ea16fbd Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sun, 14 Apr 2024 09:39:39 +0800 Subject: [PATCH 03/11] optimize logic --- libs/server/Resp/Objects/ListCommands.cs | 29 ++++++++++++++++++-- libs/server/Resp/Objects/ObjectStoreUtils.cs | 23 ---------------- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index 23f3cc4412..95be68096e 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -679,8 +679,33 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) return false; - OperationDirection sourceDirection = GetOperationDirection(param1.ReadOnlySpan); - OperationDirection destinationDirection = GetOperationDirection(param2.ReadOnlySpan); + OperationDirection sourceDirection, destinationDirection; + if (Ascii.EqualsIgnoreCase("RIGHT", param1.ReadOnlySpan)) + { + sourceDirection = OperationDirection.Right; + } + else if (Ascii.EqualsIgnoreCase("LEFT", param1.ReadOnlySpan)) + { + sourceDirection = OperationDirection.Left; + } + else + { + sourceDirection = OperationDirection.Unknown; + } + + if (Ascii.EqualsIgnoreCase("RIGHT", param2.ReadOnlySpan)) + { + destinationDirection = OperationDirection.Right; + } + else if (Ascii.EqualsIgnoreCase("LEFT", param2.ReadOnlySpan)) + { + destinationDirection = OperationDirection.Left; + } + else + { + destinationDirection = OperationDirection.Unknown; + } + if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) { return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index 3cf5a57009..fe93930e4a 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -65,28 +65,5 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) return true; } - - /// - /// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection. - /// If parsing fails, returns OperationDirection.Unknown. - /// - /// The input to parse. - /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. - public OperationDirection GetOperationDirection(ReadOnlySpan input) - { - string inputString = Encoding.UTF8.GetString(input).ToUpperInvariant(); - if (inputString == "RIGHT") - { - return OperationDirection.Right; - } - else if (inputString == "LEFT") - { - return OperationDirection.Left; - } - else - { - return OperationDirection.Unknown; - } - } } } \ No newline at end of file From 26eabd5011f72b94bf7bad715d7347bf5aa46744 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sun, 14 Apr 2024 09:46:28 +0800 Subject: [PATCH 04/11] fix format --- libs/server/Resp/Objects/ListCommands.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index 95be68096e..8caf885e7f 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -692,7 +692,7 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st { sourceDirection = OperationDirection.Unknown; } - + if (Ascii.EqualsIgnoreCase("RIGHT", param2.ReadOnlySpan)) { destinationDirection = OperationDirection.Right; From b7366c4b2be0bdc1365c65653d5f0bbd08c5292c Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sun, 14 Apr 2024 13:45:23 +0800 Subject: [PATCH 05/11] optimize string parse logic --- libs/server/Resp/Objects/ListCommands.cs | 29 ++--------------- libs/server/Resp/Objects/ObjectStoreUtils.cs | 34 ++++++++++++++++++++ 2 files changed, 36 insertions(+), 27 deletions(-) diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index 8caf885e7f..23f3cc4412 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -679,33 +679,8 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) return false; - OperationDirection sourceDirection, destinationDirection; - if (Ascii.EqualsIgnoreCase("RIGHT", param1.ReadOnlySpan)) - { - sourceDirection = OperationDirection.Right; - } - else if (Ascii.EqualsIgnoreCase("LEFT", param1.ReadOnlySpan)) - { - sourceDirection = OperationDirection.Left; - } - else - { - sourceDirection = OperationDirection.Unknown; - } - - if (Ascii.EqualsIgnoreCase("RIGHT", param2.ReadOnlySpan)) - { - destinationDirection = OperationDirection.Right; - } - else if (Ascii.EqualsIgnoreCase("LEFT", param2.ReadOnlySpan)) - { - destinationDirection = OperationDirection.Left; - } - else - { - destinationDirection = OperationDirection.Unknown; - } - + OperationDirection sourceDirection = GetOperationDirection(param1.ReadOnlySpan); + OperationDirection destinationDirection = GetOperationDirection(param2.ReadOnlySpan); if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) { return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index fe93930e4a..85d564cab5 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -65,5 +65,39 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) return true; } + + /// + /// Tries to parse the input as "LEFT" or "RIGHT" and returns the corresponding OperationDirection. + /// If parsing fails, returns OperationDirection.Unknown. + /// + /// The input to parse. + /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. + public OperationDirection GetOperationDirection(ReadOnlySpan input) + { +#if NET8_0_OR_GREATER + if (Ascii.EqualsIgnoreCase(input, "RIGHT")) + { + return OperationDirection.Right; + } + else if (Ascii.EqualsIgnoreCase(input, "LEFT")) + { + return OperationDirection.Left; + } +#else + string inputString = Encoding.UTF8.GetString(input); + if (inputString.Equals("RIGHT", StringComparison.OrdinalIgnoreCase)) + { + return OperationDirection.Right; + } + else if (inputString.Equals("LEFT", StringComparison.OrdinalIgnoreCase)) + { + return OperationDirection.Left; + } +#endif + else + { + return OperationDirection.Unknown; + } + } } } \ No newline at end of file From d7ae807790c472a37dc2e9d1702a507e84a921e7 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Sun, 14 Apr 2024 13:46:51 +0800 Subject: [PATCH 06/11] optimize string parse logic --- libs/server/Resp/Objects/ObjectStoreUtils.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index 85d564cab5..cdae28fed3 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. using System; +using System.Runtime.CompilerServices; using System.Text; using Garnet.common; @@ -72,6 +73,7 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) /// /// The input to parse. /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. + [MethodImpl(MethodImplOptions.AggressiveInlining)] public OperationDirection GetOperationDirection(ReadOnlySpan input) { #if NET8_0_OR_GREATER From c2362992622ebdc83732e0d46dab0dd3e1416fd9 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Mon, 15 Apr 2024 09:45:07 +0800 Subject: [PATCH 07/11] optimize string parse logic --- libs/server/Resp/Objects/ListCommands.cs | 4 +-- libs/server/Resp/Objects/ObjectStoreUtils.cs | 29 ++++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/libs/server/Resp/Objects/ListCommands.cs b/libs/server/Resp/Objects/ListCommands.cs index 23f3cc4412..1bdee2ea94 100644 --- a/libs/server/Resp/Objects/ListCommands.cs +++ b/libs/server/Resp/Objects/ListCommands.cs @@ -679,8 +679,8 @@ private unsafe bool ListMove(int count, byte* ptr, ref TGarnetApi st if (!RespReadUtils.ReadPtrWithLengthHeader(ref param2.ptr, ref param2.length, ref ptr, recvBufferPtr + bytesRead)) return false; - OperationDirection sourceDirection = GetOperationDirection(param1.ReadOnlySpan); - OperationDirection destinationDirection = GetOperationDirection(param2.ReadOnlySpan); + OperationDirection sourceDirection = GetOperationDirection(param1); + OperationDirection destinationDirection = GetOperationDirection(param2); if (sourceDirection == OperationDirection.Unknown || destinationDirection == OperationDirection.Unknown) { return AbortWithErrorMessage(count, CmdStrings.RESP_ERR_GENERIC_SYNTAX_ERROR); diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index cdae28fed3..25a20e37f2 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -74,32 +74,31 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) /// The input to parse. /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. [MethodImpl(MethodImplOptions.AggressiveInlining)] - public OperationDirection GetOperationDirection(ReadOnlySpan input) + public OperationDirection GetOperationDirection(ArgSlice input) { #if NET8_0_OR_GREATER - if (Ascii.EqualsIgnoreCase(input, "RIGHT")) + if (Ascii.EqualsIgnoreCase(input.ReadOnlySpan, "RIGHT")) { return OperationDirection.Right; } - else if (Ascii.EqualsIgnoreCase(input, "LEFT")) + else if (Ascii.EqualsIgnoreCase(input.ReadOnlySpan, "LEFT")) { return OperationDirection.Left; } #else - string inputString = Encoding.UTF8.GetString(input); - if (inputString.Equals("RIGHT", StringComparison.OrdinalIgnoreCase)) - { + // Optimize for the common case + if (input.ReadOnlySpan.SequenceEqual("LEFT"u8)) + return OperationDirection.Left; + if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8)) return OperationDirection.Right; - } - else if (inputString.Equals("LEFT", StringComparison.OrdinalIgnoreCase)) - { + // Rare case: try making upper case and retry + MakeUpperCase(input.ptr); + if (input.ReadOnlySpan.SequenceEqual("LEFT"u8)) return OperationDirection.Left; - } -#endif - else - { - return OperationDirection.Unknown; - } + if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8)) + return OperationDirection.Right; +#endif + return OperationDirection.Unknown; } } } \ No newline at end of file From 54d1366db6459b5cfceeb9512cd532053dc12b2c Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Mon, 15 Apr 2024 09:47:03 +0800 Subject: [PATCH 08/11] optimize string parse logic --- libs/server/Resp/Objects/ObjectStoreUtils.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index 25a20e37f2..30d196e25e 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -2,7 +2,6 @@ // Licensed under the MIT license. using System; -using System.Runtime.CompilerServices; using System.Text; using Garnet.common; @@ -73,7 +72,6 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) /// /// The input to parse. /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. - [MethodImpl(MethodImplOptions.AggressiveInlining)] public OperationDirection GetOperationDirection(ArgSlice input) { #if NET8_0_OR_GREATER From 4c0b83746ad662afc8ea58a4319a20443d1cf97e Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Tue, 16 Apr 2024 09:05:06 +0800 Subject: [PATCH 09/11] optimize string parse logic --- libs/server/Resp/Objects/ObjectStoreUtils.cs | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/libs/server/Resp/Objects/ObjectStoreUtils.cs b/libs/server/Resp/Objects/ObjectStoreUtils.cs index 30d196e25e..44dc2f4c1d 100644 --- a/libs/server/Resp/Objects/ObjectStoreUtils.cs +++ b/libs/server/Resp/Objects/ObjectStoreUtils.cs @@ -74,16 +74,6 @@ private bool AbortWithErrorMessage(int count, ReadOnlySpan errorMessage) /// The parsed OperationDirection, or OperationDirection.Unknown if parsing fails. public OperationDirection GetOperationDirection(ArgSlice input) { -#if NET8_0_OR_GREATER - if (Ascii.EqualsIgnoreCase(input.ReadOnlySpan, "RIGHT")) - { - return OperationDirection.Right; - } - else if (Ascii.EqualsIgnoreCase(input.ReadOnlySpan, "LEFT")) - { - return OperationDirection.Left; - } -#else // Optimize for the common case if (input.ReadOnlySpan.SequenceEqual("LEFT"u8)) return OperationDirection.Left; @@ -95,7 +85,6 @@ public OperationDirection GetOperationDirection(ArgSlice input) return OperationDirection.Left; if (input.ReadOnlySpan.SequenceEqual("RIGHT"u8)) return OperationDirection.Right; -#endif return OperationDirection.Unknown; } } From dde8ce7c73daa042e57639c3bcfa6edbf0ac1239 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Tue, 23 Apr 2024 09:48:52 +0800 Subject: [PATCH 10/11] add test case --- test/Garnet.test/RespListTests.cs | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/Garnet.test/RespListTests.cs b/test/Garnet.test/RespListTests.cs index af038f9bf7..73cc10af9f 100644 --- a/test/Garnet.test/RespListTests.cs +++ b/test/Garnet.test/RespListTests.cs @@ -681,6 +681,39 @@ public async Task CanUseLMoveGC() Assert.AreEqual(expectedResponseArray, responseArray); } + [Test] + public async Task CanUseLMoveWithCaseInsensitiveDirectionGC() + { + using var db = TestUtils.GetGarnetClient(); + db.Connect(); + + await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "one" }); + await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "two" }); + await db.ExecuteForStringResultAsync("RPUSH", new string[] { "mylist", "three" }); + + var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "left" }); + Assert.AreEqual("three", response); + + var responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" }); + var expectedResponseArray = new string[] { "one", "two" }; + Assert.AreEqual(expectedResponseArray, responseArray); + + responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" }); + expectedResponseArray = new string[] { "three" }; + Assert.AreEqual(expectedResponseArray, responseArray); + + response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "LeFT", "RIghT" }); + Assert.AreEqual("one", response); + + responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "mylist", "0", "-1" }); + expectedResponseArray = new string[] { "two" }; + Assert.AreEqual(expectedResponseArray, responseArray); + + responseArray = await db.ExecuteForStringArrayResultAsync("LRANGE", new string[] { "myotherlist", "0", "-1" }); + expectedResponseArray = new string[] { "three", "one" }; + Assert.AreEqual(expectedResponseArray, responseArray); + } + [Test] public async Task CanUseLMoveWithCancellationTokenGC() { From 0241ea801fa6dff28d5c89a3ae13cc4cc60a3e93 Mon Sep 17 00:00:00 2001 From: Zzhiter <2631992879@qq.com> Date: Tue, 23 Apr 2024 09:56:55 +0800 Subject: [PATCH 11/11] add test case --- test/Garnet.test/RespListTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/Garnet.test/RespListTests.cs b/test/Garnet.test/RespListTests.cs index 73cc10af9f..b7170956e4 100644 --- a/test/Garnet.test/RespListTests.cs +++ b/test/Garnet.test/RespListTests.cs @@ -638,6 +638,13 @@ public async Task CanUseLMoveGC() using var db = TestUtils.GetGarnetClient(); db.Connect(); + // Test for Operation direction error. + var exception = Assert.ThrowsAsync(async () => + { + await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "right", "lef" }); + }); + Assert.AreEqual("ERR syntax error", exception.Message); + //If source does not exist, the value nil is returned and no operation is performed. var response = await db.ExecuteForStringResultAsync("LMOVE", new string[] { "mylist", "myotherlist", "RIGHT", "LEFT" }); Assert.AreEqual(null, response);