Blocking pops should remove List/Set objects if entirely popped, just like non-blocking pops do.

Author: prvyk

libs/server/Objects/ItemBroker/CollectionItemBroker.cs

@@ -457,12 +457,12 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                 dstKey = cmdArgs[0];
             }
 
+            var asKey = storageSession.scratchBufferManager.CreateArgSlice(key);
             // Create a transaction if not currently in a running transaction
             if (storageSession.txnManager.state != TxnState.Running)
             {
                 Debug.Assert(storageSession.txnManager.state == TxnState.None);
                 createTransaction = true;
-                var asKey = storageSession.scratchBufferManager.CreateArgSlice(key);
                 if (initial)
                     storageSession.txnManager.SaveKeyEntryToLock(asKey, false, LockType.Exclusive);
                 storageSession.txnManager.SaveKeyEntryToLock(asKey, true, LockType.Exclusive);
@@ -477,6 +477,7 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                 _ = storageSession.txnManager.Run(true);
             }
 
+            var lockableContext = storageSession.txnManager.LockableContext;
             var objectLockableContext = storageSession.txnManager.ObjectStoreLockableContext;
             IGarnetObject dstObj = null;
             byte[] arrDstKey = default;
@@ -490,10 +491,7 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                     if (!initial)
                         return false;
 
-                    var context = storageSession.txnManager.LockableContext;
-
-                    var keySlice = storageSession.scratchBufferManager.CreateArgSlice(key);
-                    statusOp = storageSession.GET(keySlice, out ArgSlice _, ref context);
+                    statusOp = storageSession.GET(asKey, out ArgSlice _, ref lockableContext);
 
                     if (statusOp != GarnetStatus.NOTFOUND)
                     {
@@ -511,7 +509,7 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                             return true;
                         }
 
-                        dstStatusOp = storageSession.GET(dstKey, out ArgSlice _, ref context);
+                        dstStatusOp = storageSession.GET(dstKey, out ArgSlice _, ref lockableContext);
                         if (dstStatusOp != GarnetStatus.NOTFOUND)
                         {
                             result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
@@ -543,13 +541,14 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                         if (currCount == 0)
                             return false;
 
+                        var isSuccessful = false;
                         switch (command)
                         {
                             case RespCommand.BLPOP:
                             case RespCommand.BRPOP:
-                                var isSuccessful = TryGetNextListItem(listObj, command, out var nextItem);
+                                isSuccessful = TryGetNextListItem(listObj, command, out var nextItem);
                                 result = new CollectionItemResult(key, nextItem);
-                                return isSuccessful;
+                                break;
                             case RespCommand.BLMOVE:
                                 ListObject dstList;
                                 var newObj = false;
@@ -577,8 +576,7 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                                     isSuccessful = storageSession.SET(arrDstKey, dstList, ref objectLockableContext) ==
                                                    GarnetStatus.OK;
                                 }
-
-                                return isSuccessful;
+                                break;
                             case RespCommand.BLMPOP:
                                 var popDirection = (OperationDirection)cmdArgs[0].ReadOnlySpan[0];
                                 var popCount = *(int*)(cmdArgs[1].ptr);
@@ -592,11 +590,19 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                                 }
 
                                 result = new CollectionItemResult(key, items);
-                                return true;
+                                isSuccessful = true;
+                                break;
                             default:
                                 result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
                                 return initial;
                         }
+
+                        if (isSuccessful && listObj.LnkList.Count == 0)
+                        {
+                            _ = storageSession.EXPIRE(asKey, TimeSpan.Zero, out _, StoreType.Object, ExpireOption.None,
+                                                      ref lockableContext, ref objectLockableContext);
+                        }
+                        return isSuccessful;
                     case SortedSetObject setObj:
                         currCount = setObj.Count();
                         if (objectType != GarnetObjectType.SortedSet)
@@ -607,8 +613,13 @@ private unsafe bool TryGetResult(byte[] key, StorageSession storageSession,
                         if (currCount == 0)
                             return false;
 
-                        return TryGetNextSetObjects(key, setObj, currCount, command, cmdArgs, out result);
-
+                        isSuccessful = TryGetNextSetObjects(key, setObj, currCount, command, cmdArgs, out result);
+                        if (isSuccessful && setObj.Count() == 0)
+                        {
+                            _ = storageSession.EXPIRE(asKey, TimeSpan.Zero, out _, StoreType.Object, ExpireOption.None,
+                                                      ref lockableContext, ref objectLockableContext);
+                        }
+                        return isSuccessful;
                     default:
                         result = new CollectionItemResult(GarnetStatus.WRONGTYPE);
                         return initial;

test/Garnet.test/RespBlockingCollectionTests.cs

@@ -414,6 +414,11 @@ public void BlmpopBlockingWithCountTest()
             Task.WaitAll([blockingTask, pushingTask], TimeSpan.FromSeconds(10));
             ClassicAssert.IsTrue(blockingTask.IsCompletedSuccessfully);
             ClassicAssert.IsTrue(pushingTask.IsCompletedSuccessfully);
+
+            using var lightClientRequest = TestUtils.CreateRequest();
+            var response = lightClientRequest.SendCommand($"EXISTS {key}");
+            var expectedResponse = ":1\r\n";
+            TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
         }
 
         [Test]
@@ -470,6 +475,9 @@ public void BzmpopReturnTest()
             ClassicAssert.AreEqual(2, pop[1][1].Length);
             ClassicAssert.AreEqual("two", pop[1][1][0].ToString());
             ClassicAssert.AreEqual(2, (int)(RedisValue)pop[1][1][1]);
+
+            ClassicAssert.IsFalse(db.KeyExists("a"));
+            ClassicAssert.IsTrue(db.KeyExists("b"));
         }
 
         [Test]
@@ -685,6 +693,10 @@ public void PopWrongTypeTest()
             Task.WaitAll([blockingLTask, blockingZTask], TimeSpan.FromSeconds(5));
             ClassicAssert.IsTrue(blockingLTask.IsCompletedSuccessfully);
             ClassicAssert.IsTrue(blockingZTask.IsCompletedSuccessfully);
+
+            response = lightClientRequest.SendCommand("EXISTS list");
+            expectedResponse = ":0\r\n";
+            TestUtils.AssertEqualUpToExpectedLength(expectedResponse, response);
         }
     }
 }