Skip to content

Commit

Permalink
[Bug] Fixed RENAME command removing the expiration of the renamed key (…
Browse files Browse the repository at this point in the history 
…#652)

* Fixed RENAME removes expiration of the renamed key

* Added missed Dispose

* Adding a couple of tests to this PR

* dotnet format

* Fixed review comments

---------

Co-authored-by: Tal Zaccai <[email protected]>
  • Loading branch information
@Vijay-Nirmal @TalZaccai
Vijay-Nirmal and TalZaccai authored Sep 10, 2024
1 parent 3492813 commit 485bcf5
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 19 deletions.
78 changes: 59 additions & 19 deletions libs/server/Storage/Session/MainStore/MainStoreOps.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -539,14 +539,12 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St

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);
Expand All @@ -558,16 +556,38 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St
var ptrVal = (byte*)memoryHandle.Pointer;

RespReadUtils.ReadUnsignedLengthHeader(out var headerLength, ref ptrVal, ptrVal + o.Length);
var value = SpanByte.FromPinnedPointer(ptrVal, headerLength);
SET(ref newKey, ref value, ref context);

memoryHandle.Dispose();
o.Memory.Dispose();

// Delete the old key
DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext);

returnStatus = GarnetStatus.OK;
// 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 (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 _);

// 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(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;
}
}
}
finally
Expand All @@ -591,19 +611,39 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St
try
{
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;
SET(newKeyArray, valObj, ref objectContext);

// Delete the old key
DELETE(oldKeyArray, StoreType.Object, ref context, ref objectContext);
byte[] newKeyArray = newKeySlice.ToArray();

var expireSpan = new SpanByteAndMemory();
var ttlStatus = TTL(ref oldKey, StoreType.Object, ref expireSpan, ref context, ref objectContext, true);

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
Expand Down
52 changes: 52 additions & 0 deletions test/Garnet.test/RespTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1161,6 +1161,26 @@ public void SingleRename()
ClassicAssert.AreEqual(null, origValue);
}

[Test]
public void SingleRenameWithExpiry()
{
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");
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.Less(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds);
}

[Test]
public void SingleRenameKeyEdgeCase([Values] bool withoutObjectStore)
{
Expand Down Expand Up @@ -1217,6 +1237,38 @@ public void SingleRenameObjectStore()
ClassicAssert.AreEqual(origList, result);
}

[Test]
public void SingleRenameObjectStoreWithExpiry()
{
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);
ClassicAssert.IsTrue(rb);
result = db.ListRange(key1);
ClassicAssert.AreEqual(Array.Empty<RedisValue>(), 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.Less(ttl.Value.TotalMilliseconds, TimeSpan.FromMinutes(1).TotalMilliseconds);
}

[Test]
public void CanSelectCommand()
{
Expand Down

0 comments on commit 485bcf5

Please sign in to comment.