Skip to content

Conversation

anna-tran
Copy link
Contributor

What this PR does:
Updates the DynamoDB ring client to use versioning in it's KV store entries. Specifically

  • Updates the Batch operation to use TransactWriteItems instead of BatchWriteItems because the former allows versioned optimistic locking when there are concurrent updates to the same ring entry.
  • Updates the Batch operation to use a conditional expression on Put only if the version exists and matches what was previously queried.
  • Updates the Query operation to return a dynamodbItem struct which holds the data from the queried item along with a version. If there is no version from DynamoDB for the item, the version defaults to an empty string.

Which issue(s) this PR fixes:
Fixes #6986

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

Copy link
Contributor

@danielblando danielblando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work.

One last comment talked offline. We can inprove the retry if failed before of condition error.

@@ -72,6 +72,7 @@
* [ENHANCEMENT] API: add request ID injection to context to enable tracking requests across downstream services. #6895
* [ENHANCEMENT] gRPC: Add gRPC Channelz monitoring. #6950
* [ENHANCEMENT] Upgrade build image and Go version to 1.24.6. #6970 #6976
* [ENHANCEMENT] Implement versioned transactions for batch writes to Ring DynamoDB. #6986
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i would just say "for writes"

putRequests[dynamodbKey{primaryKey: key, sortKey: childKey}] = bytes
putRequests[dynamodbKey{primaryKey: key, sortKey: childKey}] = dynamodbItem{
data: bytes,
version: resp[childKey].version,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we are adding an instance to the ring I dont think it will be on the resp from query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add versioning to DynamoDB KV store
2 participants