-
Notifications
You must be signed in to change notification settings - Fork 543
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
Native ETag Support #675
base: main
Are you sure you want to change the base?
Native ETag Support #675
Conversation
77487f1
to
d564238
Compare
b126e5e
to
da48417
Compare
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.
Not finished with review yet but sending a few comments now
{ | ||
public struct EtagOffsetManagementContext | ||
{ | ||
// default values for when no Etag exists on a record |
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.
misplaced comments
@@ -98,6 +98,8 @@ private bool NetworkEXEC() | |||
private bool NetworkSKIP(RespCommand cmd) | |||
{ | |||
// Retrieve the meta-data for the command to do basic sanity checking for command arguments | |||
// Normalize will turn internal "not-real commands" such as SETEXNX, and SETEXXX to the command info parent | |||
cmd = cmd.Normalize(); |
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.
this fixes the issue that wouldnt let XX, NX, and as a result SETIFMATCH and SET + WITHETAG work inside of multi exec
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.
Remove your "temp changes" in .github/workflows/ci-bdnbenchmark.yml before this is merged
/// <summary> | ||
/// When ETag is set this returns 1 else 0. Used for branchless programming | ||
/// </summary> | ||
public int HasETagMultiplier => (int)((word & kETagBitMask) >> kEtagBitOffset); |
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.
TODO remove this
/// GETWITHETAG key | ||
/// Given a key get the value and ETag | ||
/// </summary> | ||
private bool NetworkGETWITHETAG<TGarnetApi>(ref TGarnetApi storageApi) |
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.
Move all the added etag commands to their own file such as BasicETagCommands.cs
@@ -36,6 +36,7 @@ static void CopyTo(ref SpanByte src, ref SpanByteAndMemory dst, MemoryPool<byte> | |||
|
|||
void CopyRespTo(ref SpanByte src, ref SpanByteAndMemory dst, int start = 0, int end = -1) | |||
{ | |||
// src length of the value indicating no end is supplied defaults to lengthWithoutMetadata, else it chooses the bigger of 0 or (end - start) |
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.
fix this comment, i do not parse it.
} | ||
} | ||
|
||
static void RespWriteEtagValArray(long etag, ref ReadOnlySpan<byte> value, ref byte* curr, byte* end, bool writeDirect) |
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.
this, the previous, etc. should move to RespWriteUtils
Native ETag Support
Garnet provides support for ETags on raw strings. You can associate any string-based key-value pair inserted into Garnet with an automatically updated ETag.
Compatibility with non-ETag commands and the behavior of data inserted with ETags are detailed in the markdown for garnet specific commands. Please read the markdown file for API contracts, and compatibility.
HLD:
RecordInfo
for a record indicated whether or not a record contains an ETag. This bit on the record info is used to indicate that the first 8 bytes of the payload of the record include a long data type that represents the eTag associated with the value that is present after the first 8 bytes.Reads don't modify the eTag, but any RMW operations do. Blind Upsert (SET) will remove the eTag unless sent with the RETAINETAG flag.
ETags are incremented on each update, and the user can use them to handle things such as caching and Optimistic concurrency control.