-
Notifications
You must be signed in to change notification settings - Fork 502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Compatibility] Adding RENAMENX command #661
base: main
Are you sure you want to change the base?
Changes from all commits
4c0b808
57e7619
cae6131
c70dc21
ad8feeb
dac162c
5263ed1
33e9b8c
669fb34
76aaf4c
c9d3aff
22581e6
ce3aa43
f34cf8b
71cb473
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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<ulong>("INCRBY\r\n"u8) => RespCommand.INCRBY, | ||
(2 << 4) | 6 when lastWord == MemoryMarshal.Read<ulong>("DECRBY\r\n"u8) => RespCommand.DECRBY, | ||
(2 << 4) | 6 when lastWord == MemoryMarshal.Read<ulong>("RENAME\r\n"u8) => RespCommand.RENAME, | ||
(2 << 4) | 8 when lastWord == MemoryMarshal.Read<ulong>("NAMENX\r\n"u8) => RespCommand.RENAMENX, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You need to read the 'RE' prefix as well |
||
(2 << 4) | 6 when lastWord == MemoryMarshal.Read<ulong>("GETBIT\r\n"u8) => RespCommand.GETBIT, | ||
(2 << 4) | 6 when lastWord == MemoryMarshal.Read<ulong>("APPEND\r\n"u8) => RespCommand.APPEND, | ||
(2 << 4) | 7 when lastWord == MemoryMarshal.Read<ulong>("UBLISH\r\n"u8) && ptr[8] == 'P' => RespCommand.PUBLISH, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -618,32 +618,171 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St | |
var valObj = value.garnetObject; | ||
byte[] newKeyArray = newKeySlice.ToArray(); | ||
|
||
// 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; | ||
} | ||
} | ||
finally | ||
{ | ||
if (createTransaction) | ||
txnManager.Commit(true); | ||
} | ||
} | ||
|
||
return returnStatus; | ||
} | ||
|
||
/// <summary> | ||
/// Renames key to newkey if newkey does not yet exist. It returns an error when key does not exist. | ||
/// </summary> | ||
/// <param name="oldKeySlice">The old key to be renamed.</param> | ||
/// <param name="newKeySlice">The new key name.</param> | ||
/// <param name="storeType">The type of store to perform the operation on.</param> | ||
/// <returns> | ||
/// GarnetStatus OK means rename operation is successful | ||
/// GarnetStatus NOTFOUND means old key does not exists | ||
/// GarnetStatus MOVED means new key already exists | ||
/// </returns> | ||
public unsafe GarnetStatus RENAMENX(ArgSlice oldKeySlice, ArgSlice newKeySlice, StoreType storeType) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is majorly similar to RENAME, I would suggest adding a bool parameter to the RENAME method to specify if rename should happen only if key doesn't exist, instead of copying all this code (as any future change to this would need to happen in multiple locations). |
||
{ | ||
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; | ||
SpanByte oldKey = oldKeySlice.SpanByte; | ||
|
||
if (storeType == StoreType.Main || storeType == StoreType.All) | ||
{ | ||
try | ||
{ | ||
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); | ||
|
||
// Find expiration time of the old key | ||
var expireSpan = new SpanByteAndMemory(); | ||
var ttlStatus = TTL(ref oldKey, StoreType.Object, ref expireSpan, ref context, ref objectContext, true); | ||
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 _); | ||
expireSpan.Memory.Dispose(); | ||
|
||
// If the key has an expiration, set the new key with the expiration | ||
if (expireTimeMs > 0) | ||
{ | ||
SET(newKeyArray, valObj, ref objectContext); | ||
EXPIRE(newKeySlice, TimeSpan.FromMilliseconds(expireTimeMs), out _, StoreType.Object, ExpireOption.None, ref context, ref objectContext, true); | ||
// 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 + sizeof(long)))->cmd = RespCommand.SETEXNX; | ||
((RespInputHeader*)(setValuePtr + sizeof(long)))->flags = 0; | ||
var newKey = newKeySlice.SpanByte; | ||
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 expire as 0 or -2, in those cases we don't SET the new key | ||
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 setValueSpan = setValue.SpanByte; | ||
var setValuePtr = setValueSpan.ToPointerWithMetadata(); | ||
((RespInputHeader*)setValuePtr)->cmd = RespCommand.SETEXNX; | ||
((RespInputHeader*)setValuePtr)->flags = 0; | ||
var newKey = newKeySlice.SpanByte; | ||
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 only when SET NX succeeded | ||
if (returnStatus == GarnetStatus.OK) | ||
{ | ||
SET(newKeyArray, valObj, ref objectContext); | ||
DELETE(ref oldKey, StoreType.Main, ref context, ref objectContext); | ||
} | ||
} | ||
} | ||
} | ||
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 | ||
{ | ||
byte[] oldKeyArray = oldKeySlice.ToArray(); | ||
var status = GET(oldKeyArray, out var value, ref objectContext); | ||
|
||
if (status == GarnetStatus.OK) | ||
{ | ||
var valObj = value.garnetObject; | ||
byte[] newKeyArray = newKeySlice.ToArray(); | ||
|
||
// Not using EXISTS method to avoid new allocation of Array for key | ||
var getNewStatus = GET(newKeyArray, out _, ref objectContext); | ||
|
||
if (getNewStatus == GarnetStatus.NOTFOUND) | ||
{ | ||
// 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 | ||
|
@@ -652,7 +791,6 @@ public unsafe GarnetStatus RENAME(ArgSlice oldKeySlice, ArgSlice newKeySlice, St | |
txnManager.Commit(true); | ||
} | ||
} | ||
|
||
return returnStatus; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GarnetStatus is not meant to be interpreted this way, you should pass an output parameter to this method and write the appropriate output to it.