-
-
Notifications
You must be signed in to change notification settings - Fork 4
feat(server): Implement batch endpoint #275
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
base: main
Are you sure you want to change the base?
Conversation
|
@sentry review |
objectstore-server/src/multipart.rs
Outdated
| Err(e) => { | ||
| tracing::error!( | ||
| error = &e as &dyn std::error::Error, | ||
| "failed to convert item to multipart Part, skipping" | ||
| ); | ||
| } |
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.
is there a failure-proof way to build an error part we could use here rather than just omit the operation from the response entirely?
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 will rethink this, admittedly I don't like it very much as well.
0543fcf to
abd294f
Compare
3f49d54 to
d6dbeee
Compare
|
@sentry review |
|
I've made some changes, I think this looks much better now. The most important thing is that We also make the object key serialization infallible by base64 encoding the value -- in practice object keys cannot contain illegal values anyways, but these 2 concerns should, in principle, be separated -- this simplifies a lot of things. |
Implements the actual logic for the batch endpoint.
The endpoint just streams over the request parts and executes them in sequence.
Nothing in the design prevents a future change where the server could reorder and/or batch the requests to the underlying backends, to achieve even greater efficiency.
I've tried my best to split this in different PRs, but this is still pretty big, sorry.
The main file that should be reviewed is
objectstore-server/src/endpoints/batch.rs. The changes in other files are just renaming existing things.(those changes should ideally go in other PRs but I currently don't have good workflows to manage stacked branches, so I just made them in this one)
This doesn't include tests, even though the components it relies on have their own unit tests, and I've also tested this manually using a script where I was manually crafting payloads.
After we implement batching in at least one client, we will add e2e that use it.
Close FS-239