Concurrent Parquet Schema Inference #6366
Conversation
| } | ||
| let schemas: Vec<_> = futures::stream::iter(objects) | ||
| .map(|object| fetch_schema(store.as_ref(), object, self.metadata_size_hint)) | ||
| .boxed() // Workaround https://github.com/rust-lang/rust/issues/64552 |
There was a problem hiding this comment.
No I cannot explain why this works... But for some unknown reason it placates the compiler
There was a problem hiding this comment.
I found using streams was a similarly frustrating experience where the compiler give you an opaque error message (and opaque is being polite)
tustvold
changed the title
| let schemas: Vec<_> = futures::stream::iter(objects) | ||
| .map(|object| fetch_schema(store.as_ref(), object, self.metadata_size_hint)) | ||
| .boxed() // Workaround https://github.com/rust-lang/rust/issues/64552 | ||
| .buffered(32) |
There was a problem hiding this comment.
I think the 32 should at least be a named constant so it is more discoverable
What do you think about potentially using the same value as in #6183:
const CONCURRENCY_LIMIT: usize = 100;
(I sort of imagine some day someone will want to make that a configuration knob rather than a constant so using the same constant in the code will make it easier to find where they are used)
There was a problem hiding this comment.
I'll make this change as a follow up
Which issue does this PR close?
Closes #.
Rationale for this change
Inferring parquet metadata is largely IO bound, especially when interacting with remote stores or when there is no parquet size hint. As such it makes sense to attempt to run in parallel.
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?