-
Notifications
You must be signed in to change notification settings - Fork 780
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
Add dynamodb multikey kv #5026
Add dynamodb multikey kv #5026
Conversation
7261cbe
to
5be318f
Compare
5e50626
to
d5d7b83
Compare
5ea682f
to
57deeb7
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.
Do you know if there is an increase in kv network usage after this?
@friedrichg |
level.Error(c.logger).Log("msg", "error CASing", "key", key, "err", err) | ||
continue | ||
} | ||
} |
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.
I am a little bit worried about the multiple put and delete calls to DDB here. Can we use https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_BatchWriteItem.html API here to reduce this to only 1 API call?
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.
that makes sense. I am not sure why i didnt see this api from the beginning. I will work on changing this. Thanks
690176d
to
f7d4550
Compare
01f29f4
to
256a6cc
Compare
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
77bae08
to
79be362
Compare
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
2e5e480
to
96d2448
Compare
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
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 @danielblando. Great work!
Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com>
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!
* Add dynamodb multikey kv Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> * Reorganize import Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> * Rework test Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> * Change ddb CAS to use batch Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> * Use slice to batch writeRequests Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> * Change slice usage Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> Signed-off-by: Daniel Deluiggi <ddeluigg@amazon.com> Signed-off-by: Alex Le <leqiyue@amazon.com>
Signed-off-by: Daniel Deluiggi ddeluigg@amazon.com
What this PR does:
This CR introduce the idea of multikey for kv on the ring. The proposal was made at #4832
It also add a new kv store using dynamodb which uses the multikey implementation. The new kv split the ring by InstanceDesc making each instance be a key in the ring. The new kv also introduce the concept of stale data and ttl.
TTL is an optional time to live of any data not update for x amount of time. This helps cleaning the ring when a pod leaves without gracefully shutdown.
Stale data is used when we have a problem communicating to the kv store, in this case dynamodb. The stale is returned and we use the last time we could sync the data to check if a instance is unhealthy or not.
Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]