-
Notifications
You must be signed in to change notification settings - Fork 26
[WIP] implement multi range query in single request #345
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
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
59aad81
to
fa7d6e4
Compare
Do any object storage providers support this? I know S3 explicitly does not: https://docs.aws.amazon.com/AmazonS3/latest/API/API_GetObject.html#API_GetObject_RequestSyntax |
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
@kylebarron that's actually a good question, I've been around most of them and they only mention single ranges. |
If there are any providers that do support this, I'd expect for this to be an implementation detail of |
I know of azure supports this, but other major services are not. |
You're saying that Azure does support multiple byte ranges? This SO answer says it doesn't, and this relevant blob storage doc doesn't mention it. |
You are right. I double-checked the docs and confirmed that none of the major cloud storage services support this. I'm not sure why I had such a mistaken impression about this. Sorry for providing incorrect information. |
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.com>
Signed-off-by: Jérémie Drouet <jeremie.drouet@gmail.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.
Hi, thanks you very much for building this, but I personally think we should reject this PR.
The reasons are as following:
- No object storage services supported by object_store have this feature.
- The change set is quite large compared to the features it added (a breaking change and two new deps)
Perhaps we could file an issue where we could discuss your use-case and then decide what the best course of action is? IMO if there is a unobtrusive way to support this as an implementation detail of HttpStore::get_ranges I don't see a major issue with adding it, but I do agree with @Xuanwo that as currently written this PR is rather intrusive. FWIW I do seem to remember that the reason most stores don't support multiranges is in part because ecosystem support for it is extremely inconsistent. |
Which issue does this PR close?
Closes #.
Rationale for this change
Right now, when requesting multiple ranges for a file, multiple queries are fired (if the gap between the ranges >
OBJECT_STORE_COALESCE_DEFAULT
). This could be avoided by using the multi ranges capability when doing fetching ranges through HTTP.What changes are included in this PR?
This PR introduces a new
get_manyranges
method to extract the multipart version of the response.It also updates the
GetOptions
to add an type parameter so thatRange
can beGetRange
orGetManyRanges
.Are there any user-facing changes?
GetOptions
now has a parameterRange
but hasGetRange
as a default type which should be transparent for the users.