Parallel prepare - open file on needed #355
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #355 +/- ##
==========================================
- Coverage 89.59% 89.58% -0.01%
==========================================
Files 17 18 +1
Lines 4929 4982 +53
==========================================
+ Hits 4416 4463 +47
- Misses 513 519 +6
|
graebm
reviewed
Oct 7, 2023
Comment on lines
159
to
161
| /* Event loop to schedule IO work related on and requires to be serial, ie, reading from non-parallel streams, | ||
| * streaming parts back to the caller, etc... After the meta request is finished, this will be reset along with the | ||
| * client reference.*/ |
There was a problem hiding this comment.
trivial: revert
I don't know what this edit is trying to say
There was a problem hiding this comment.
I was trying to edit because for preparation in parallel now happens across different eventloops, instead of the only one assigned with the request.
Co-authored-by: Michael Graeb <[email protected]>
graebm
reviewed
Oct 9, 2023
Co-authored-by: Michael Graeb <[email protected]>
graebm
approved these changes
Oct 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Parallel preparation, the parallel stream will open the file when reading from it and seek to the right offset to read, and close the file after reading.
We checked it has no performance impact with mmap impl, and it's simpler. Simpler is better
With this approach, the user of the parallel input stream will have to manage the threads and concurrence, but it's only used by our s3 client, which already handles the concurrence and thread pool, we are good.
Original: #353
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.