-
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 EXPIRETIME and PEXPIRETIME command #664
base: main
Are you sure you want to change the base?
[Compatibility] Adding EXPIRETIME and PEXPIRETIME command #664
Conversation
There is already a GarnetObjectType.Expire, why not use that one? |
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.
Thanks for your contribution! Added a couple of comments.
/// <summary> | ||
/// Special type indicating EXPIRETIME command | ||
/// </summary> | ||
Expiretime = 0xf9, |
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.
Don't think that there's a need for these new types
public unsafe GarnetStatus EXPIRETIME<TContext, TObjectContext>(ref SpanByte key, StoreType storeType, ref SpanByteAndMemory output, ref TContext context, ref TObjectContext objectContext, bool milliseconds = false) | ||
where TContext : ITsavoriteContext<SpanByte, SpanByte, SpanByte, SpanByteAndMemory, long, MainSessionFunctions, MainStoreFunctions, MainStoreAllocator> | ||
where TObjectContext : ITsavoriteContext<byte[], IGarnetObject, ObjectInput, GarnetObjectStoreOutput, long, ObjectSessionFunctions, ObjectStoreFunctions, ObjectStoreAllocator> | ||
{ |
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.
Why not simply call TTL and convert it to unix-time?
Adding EXPIRETIME and EXPIRETIME command to garnet
Open Question:
Expiration
instead ofEXPIRETIME
andPEXPIRETIME
? The advantage will be that it will be a single generic implementation that can be reused for others in the future if required. The disadvantage is that there will be a slight performance penalty but no additional memory needed since values ofExpiration
,EXPIRETIME
andPEXPIRETIME
arelong
type, so we can do in-place override the value after recomputing the required value.