Skip to content

Conversation

@lcian
Copy link
Member

@lcian lcian commented Jan 20, 2026

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

@lcian
Copy link
Member Author

lcian commented Jan 20, 2026

@sentry review

@lcian lcian changed the title wip feat(server): Implement batch endpoint Jan 20, 2026
@lcian lcian marked this pull request as ready for review January 20, 2026 14:47
@lcian lcian requested a review from a team as a code owner January 20, 2026 14:47
@linear
Copy link

linear bot commented Jan 20, 2026

@lcian lcian requested a review from matt-codecov January 20, 2026 14:52
Comment on lines 85 to 90
Err(e) => {
tracing::error!(
error = &e as &dyn std::error::Error,
"failed to convert item to multipart Part, skipping"
);
}
Copy link
Contributor

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?

Copy link
Member Author

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.

@lcian lcian force-pushed the lcian/ref/service-results branch from 0543fcf to abd294f Compare January 26, 2026 11:56
@lcian lcian force-pushed the lcian/feat/batch-impl branch from 3f49d54 to d6dbeee Compare January 26, 2026 13:09
@lcian
Copy link
Member Author

lcian commented Jan 27, 2026

@sentry review

2 similar comments
@lcian
Copy link
Member Author

lcian commented Jan 27, 2026

@sentry review

@lcian
Copy link
Member Author

lcian commented Jan 27, 2026

@sentry review

@getsentry getsentry deleted a comment from cursor bot Jan 27, 2026
@lcian
Copy link
Member Author

lcian commented Jan 27, 2026

I've made some changes, I think this looks much better now.

The most important thing is that Part::new cannot fail now as we use HeaderValue::from_static and take in a Option<HeaderValue> as opposed to a Option<&str>.

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.

@lcian lcian requested a review from matt-codecov January 27, 2026 12:13
Base automatically changed from lcian/ref/service-results to main January 29, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants