diff --git a/libs/server/Objects/List/ListObject.cs b/libs/server/Objects/List/ListObject.cs index 010f477e33..439bc8430d 100644 --- a/libs/server/Objects/List/ListObject.cs +++ b/libs/server/Objects/List/ListObject.cs @@ -140,7 +140,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou return true; } - var previouseSize = this.Size; + var previousSize = this.Size; switch (header->ListOp) { case ListOperation.LPUSH: @@ -183,7 +183,7 @@ public override unsafe bool Operate(ref SpanByte input, ref SpanByteAndMemory ou throw new GarnetException($"Unsupported operation {(ListOperation)_input[0]} in ListObject.Operate"); } - sizeChange = this.Size - previouseSize; + sizeChange = this.Size - previousSize; } return true; } diff --git a/libs/server/Objects/List/ListObjectImpl.cs b/libs/server/Objects/List/ListObjectImpl.cs index 564d822b63..6080c1199a 100644 --- a/libs/server/Objects/List/ListObjectImpl.cs +++ b/libs/server/Objects/List/ListObjectImpl.cs @@ -4,7 +4,6 @@ using System; using System.Buffers; using System.Collections.Generic; -using System.Diagnostics; using System.Linq; using Garnet.common; using Tsavorite.core; @@ -217,55 +216,55 @@ private void ListRange(byte* input, ref SpanByteAndMemory output) private void ListTrim(byte* input, byte* output) { - var _input = (ObjectInputHeader*)input; - var _output = (ObjectOutputHeader*)output; + var inputHeader = (ObjectInputHeader*)input; + var outputHeader = (ObjectOutputHeader*)output; if (list.Count > 0) { - var start = _input->count < 0 ? list.Count + _input->count : _input->count; - if (start < -(list.Count - 1) || start >= list.Count) start = list.Count - 1; - - var end = _input->done < 0 ? list.Count + _input->done : _input->done; - if (end < -(list.Count - 1) || end >= list.Count) end = list.Count - 1; + var start = inputHeader->count < 0 ? list.Count + inputHeader->count : inputHeader->count; + var end = inputHeader->done < 0 ? list.Count + inputHeader->done : inputHeader->done; - Debug.Assert(end - start <= list.Count); - if (start > end) + if (start > end || start >= list.Count || end < 0) { - _output->opsDone = list.Count; + outputHeader->opsDone = list.Count; list.Clear(); } else { - // Only the first end+1 elements will remain + start = start < 0 ? 0 : start; + end = end >= list.Count ? list.Count : end + 1; + + // Only the first end elements will remain if (start == 0) { - var numDeletes = list.Count - (end + 1); - for (int i = 0; i < numDeletes; i++) + var numDeletes = list.Count - end; + for (var i = 0; i < numDeletes; i++) { - var _value = list.Last.Value; + var value = list.Last!.Value; list.RemoveLast(); - this.UpdateSize(_value, false); + this.UpdateSize(value, false); } - _output->opsDone = numDeletes; + outputHeader->opsDone = numDeletes; } else { - int i = 0; + var i = 0; IList readOnly = new List(list).AsReadOnly(); - foreach (byte[] node in readOnly) + foreach (var node in readOnly) { - if (!(i >= start && i <= end)) + if (!(i >= start && i < end)) { list.Remove(node); this.UpdateSize(node, false); } i++; } - _output->opsDone = i; + outputHeader->opsDone = i; } } - _output->bytesDone = 0; - _output->countDone = _output->opsDone; + + outputHeader->bytesDone = 0; + outputHeader->countDone = outputHeader->opsDone; } } diff --git a/test/Garnet.test/RespListTests.cs b/test/Garnet.test/RespListTests.cs index 6704f487a8..4892a0ecf6 100644 --- a/test/Garnet.test/RespListTests.cs +++ b/test/Garnet.test/RespListTests.cs @@ -64,7 +64,7 @@ public void BasicLPUSHAndLPOP() } [Test] - public void MultiLPUSHAndLTRIM() + public void MultiLPUSHAndLTRIMWithMemoryCheck() { using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); var db = redis.GetDatabase(0); @@ -84,11 +84,10 @@ public void MultiLPUSHAndLTRIM() var expectedResponse = 904; Assert.AreEqual(expectedResponse, actualValue); - long nLen = db.ListLength(key); db.ListTrim(key, 1, 5); - long nLen1 = db.ListLength(key); - Assert.AreEqual(nLen1, 5); + var nLen = db.ListLength(key); + Assert.AreEqual(5, nLen); result = db.Execute("MEMORY", "USAGE", key); actualValue = ResultType.Integer == result.Resp2Type ? Int32.Parse(result.ToString()) : -1; @@ -97,8 +96,8 @@ public void MultiLPUSHAndLTRIM() //all elements remain db.ListTrim(key, 0, -1); - nLen1 = db.ListLength(key); - Assert.AreEqual(nLen1, 5); + nLen = db.ListLength(key); + Assert.AreEqual(5, nLen); result = db.Execute("MEMORY", "USAGE", key); actualValue = ResultType.Integer == result.Resp2Type ? Int32.Parse(result.ToString()) : -1; @@ -106,8 +105,8 @@ public void MultiLPUSHAndLTRIM() Assert.AreEqual(expectedResponse, actualValue); db.ListTrim(key, 0, -3); - nLen1 = db.ListLength(key); - Assert.AreEqual(3, nLen1); + nLen = db.ListLength(key); + Assert.AreEqual(3, nLen); var vals = db.ListRange(key, 0, -1); Assert.AreEqual("val_8", vals[0].ToString()); @@ -115,6 +114,50 @@ public void MultiLPUSHAndLTRIM() Assert.AreEqual("val_6", vals[2].ToString()); } + private static object[] LTrimTestCases = { + new object[] {0, 0, new[] {0} }, + new object[] {-2, -1, new[] {8, 9} }, + new object[] {-2, -2, new[] {8} }, + new object[] {3, 5, new[] {3, 4, 5} }, + new object[] {-12, 0, new[] {0} }, + new object[] {-12, 2, new[] {0, 1, 2} }, + new object[] {-12, -7, new[] {0, 1, 2, 3} }, + new object[] {-15, -11, Array.Empty() }, + new object[] {8, 8, new[] {8} }, + new object[] {8, 12, new[] {8, 9} }, + new object[] {9, 12, new[] {9} }, + new object[] {10, 12, Array.Empty() }, + new object[] {5, 3, Array.Empty()}, + new object[] {-3, -5, Array.Empty()} + }; + + [Test] + [TestCaseSource(nameof(LTrimTestCases))] + public void MultiRPUSHAndLTRIM(int startIdx, int stopIdx, int[] expectedRemainingIdx) + { + using var redis = ConnectionMultiplexer.Connect(TestUtils.GetConfig()); + var db = redis.GetDatabase(0); + + var key = "List_Test"; + var nVals = 10; + var values = new RedisValue[nVals]; + for (var i = 0; i < 10; i++) + { + values[i] = "val_" + i; + } + var nAdded = db.ListRightPush(key, values); + Assert.AreEqual(nVals, nAdded); + + db.ListTrim(key, startIdx, stopIdx); + var nLen = db.ListLength(key); + Assert.AreEqual(expectedRemainingIdx.Length, nLen); + var remainingVals = db.ListRange(key); + for (var i = 0; i < remainingVals.Length; i++) + { + Assert.AreEqual(values[expectedRemainingIdx[i]], remainingVals[i].ToString()); + } + } + [Test] public void MultiLPUSHAndLLENWithPendingStatus() {