-
Notifications
You must be signed in to change notification settings - Fork 7
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
Store upload/add shards
as a Set.
#52
Comments
it's not totally obvious what we should do with We could store |
olizilla
added a commit
that referenced
this issue
Nov 29, 2022
Change `upload/add` to return items with a `shards` array instead of expanding them out to many root#shard rows. Stores shards as a String Set in dynamo. Multiple invocations of `upload/add` with the same root store the union of all the shards from each invocation. Fixes #52 License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>
olizilla
added a commit
that referenced
this issue
Nov 30, 2022
Change `upload/add` to return items with a `shards` array instead of expanding them out to many root#shard rows. Stores shards as a String Set in dynamo. Multiple invocations of `upload/add` with the same root store the union of all the shards from each invocation. Fixes up type definitions and some tidying up from #48 Removes `issuer` and `invocation` from upload/add response, we're not using them, and there is an open question of how to store them... Fixes #52 License: MIT Signed-off-by: Oli Evans <oli@protocol.ai>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
DynamoDB supports Sets!
https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/HowItWorks.NamingRulesDataTypes.html#HowItWorks.DataTypes.SetTypes
We can insert 1 upload doc per space+root, instead of unwrapping the shards array:
with the key
so we make root unique per space...
If an agent calls
upload/add
multiple times with the same root, we can useUpdateItem
and specify that theshards
set should be added to the existing item, preserving our existing "it's the union of all calls toupload/add
semantics" while also avoiding opening us up to a single invocation expanding out to a large number of ops / records.We can use
UpdateItem
as an upsert operation by specifying all the fields as:if we choose to support user metadata from an allowed list of additional
nb
properties then we can similarly treat that as a metadata map, and merge the keys where we see multiple invocations for the same root.This change would pair nicely with a tweak to the ouput for
upload/list
where we would return the records as we store them, giving the caller a single record per root, with a shards array, to mirror the shape that they send them to us, rather than sending repeated rows per root#shard combo.The text was updated successfully, but these errors were encountered: