[ObjectStore] Add SpawnService for running requests on different tokio runtime/Handle

[tustvold]

Which issue does this PR close?

• Relates to [DISCUSSION] [object_store] New crate with object store combinators / utilitles arrow-rs-object-store#14
• closes [object_store] Run requests on a different tokio runtime arrow-rs-object-store#13

Rationale for this change

This allows spawning IO onto a separate tokio runtime

What changes are included in this PR?

Are there any user-facing changes?

[tustvold]

This is a drive-by improvement, and allows using HttpResponseBody with http_body_util::BodyExt.

It technically isn't necessary, as object_store ignores any non-data frames and therefore HttpResponseBody::bytes is sufficient, but this is cleaner

[alamb]

That is probably good context to add as an inline comment for future readers

[tustvold]

It was a little confusing having the public HTTP client abstraction mixed in with the client utilities for implementing object_store - so split it out into a separate module

[tustvold]

It's a bit ouroborosy but I wonder if we should implement HttpService for HttpClient so that you can also wrap an HttpClient with this

[alamb]

That is probably good context to add as an inline comment for future readers

[alamb]

I always find templated code like this harder to use and understand than just dyn ...

When it is important to make type specialized code for performance I understand the need

But when the usecase is to make a function call for an http request I think the mental overhead is unecessary

I realize this is a style opinion / point of view and that others may not feel the same way

[tustvold]

So I originally actually had this wrapping HttpClient, but it ended up a bit weird as you ended up doing

let client = HttpClient::new(SpawnService::new(HttpClient::new(reqwest::Client::new())))

As opposed to composing the services to yield the final client

let client = HttpClient::new(SpawnService::new(reqwest::Client::new()))

[ion-elgreco]

How would one configure a store with a custom HttpClient?

I don't see anything that resembles with_http_client() on the stores

[tustvold]

I don't see anything that resembles with_http_client() on the stores

0.12 (when it is released) adds with_http_connector

[alamb]

Is the idea that this would be able poll the response stream on a different runtime without all the high level wrapping needed if we did it in ObjectStore? If so that sounds like a good ideae

I am less clear if other layers (like chunked requests) for example make sense to do at this layer (as I think they also need to take into account things like verifing that the etag is the same for all responses, etc)

[tustvold]

Is the idea that this would be able poll the response stream on a different runtime without all the high level wrapping needed if we did it in ObjectStore?

Yes, similar story for metrics, tracing, etc...

Caching could also be implemented at this layer quite easily, either by caching anything with an ETag header or by respecting the Cache-Control header, or some other mechanism.

chunks

Chunking can be done at this layer, however, it has the exact same restrictions as implementing it at the ObjectStore level:

• Request needs to specify an ETag precondition to ensure same object
• Requests must specify range(s)
• Cannot return chunks out of order

It is unclear if these restrictions are too onerous for the feature to be useful.

[ion-elgreco]

@tustvold do you think this would still land in a 0.12.x?

I would like to do a deltalake 1.0 soon but this is kind of a must to have included since it's a better implementation then the DeltaStorageBackend we have now.

[tustvold]

@tustvold do you think this would still land in a 0.12.x?

I suspect the biggest potential blocker to that would be getting DataFusion updated to the 0.12 line, which in turn is waiting on the next breaking arrow release scheduled for the end of April.

would like to do a deltalake 1.0 soon

FWIW given that arrow and DataFusion are not "stable", i.e. they make periodic breaking releases, I would have thought releasing deltalake 1.0 would quickly end up needing a deltalake 2.0, etc... IMO neither arrow nor datafusion should be 1.0, but there is a historical reason for it.

[ion-elgreco]

@tustvold do you think this would still land in a 0.12.x?

I suspect the biggest potential blocker to that would be getting DataFusion updated to the 0.12 line, which in turn is waiting on the next breaking arrow release scheduled for the end of April.

That's fine! I will probably start with some RCs first, I was mainly curious that it gets published in the 0.12.x versioning.

would like to do a deltalake 1.0 soon

FWIW given that arrow and DataFusion are not "stable", i.e. they make periodic breaking releases, I would have thought releasing deltalake 1.0 would quickly end up needing a deltalake 2.0, etc... IMO neither arrow nor datafusion should be 1.0, but there is a historical reason for it.

I am mainly talking about the python API, which has been quite stable. Breaking changes generally effect our internal api's, and DF44 has resolved most of our issues.

There are some minor issues still in datafusion but tbh these are edge cases so that's fine for now

[alamb]

I will try and review this a bit more carefully tomorrow

I think it could do with some more documentation / examples (which I can help with)

I also think the name SpawnService while it makes sense given the underlying implementation (to spawn on another runtime) is somewhat confusing. I recommend a more explicit name like CrossRTService or CrossRTHttpService -- I realize I may prefer longer names than @tustvold , but given this one would likely be used only a few times (when setting up the object store) I think being more explicit is better

[alamb]

I filed apache/arrow-rs-object-store#13 and linked it to this ticket's close

[alamb]

Thank you for this PR. We are in the process of moving the object_store code to its own repository. Would it be possible for you to create a PR in that repository instead?

• See details on [EPIC] Port object_store content from arrow-rs repository arrow-rs-object-store#2
• Backstory on [DISCUSSION] Proposal move object_store to its own github repo? #6183

(we will handle moving all existing issues to the new repository)