Port pull request #117 from prometheus-community/parquet-common #28
Conversation
This file contains hidden or 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
GiedriusS
reviewed
Nov 17, 2025
Member
GiedriusS
left a comment
GiedriusS
left a comment
There was a problem hiding this comment.
Cosmetic suggestions, didn't have enough time right now to look through other changes
Member
daijy
force-pushed
the
parallelconversion
branch
from
110be77 to
547fda0
Compare
daijy
force-pushed
the
parallelconversion
branch
from
88ed15a to
073110b
Compare
Signed-off-by: Daniel Dai <[email protected]>
Signed-off-by: Daniel Dai <[email protected]>
Signed-off-by: Daniel Dai <[email protected]>
Signed-off-by: Daniel Dai <[email protected]>
Signed-off-by: Daniel Dai <[email protected]>
Signed-off-by: Daniel Dai <[email protected]>
daijy
force-pushed
the
parallelconversion
branch
from
073110b to
c431436
Compare
Member
|
Closes #23 ? |
Collaborator
yes! |
Signed-off-by: Daniel Dai <[email protected]>
Collaborator
|
One last comment, otherwise lgtm! |
Signed-off-by: Daniel Dai <[email protected]>
MichaHoffmann
requested changes
Nov 27, 2025
| rr.rowBuilder.Add(lc.ColumnIndex, parquet.ValueOf(l.Value)) | ||
| // we need to address for projecting chunk columns away later so we need to correct for the offset here | ||
| colIdxSlice = append(colIdxSlice, lc.ColumnIndex-schema.ChunkColumnsPerDay) | ||
| colIdxSlice = append(colIdxSlice, lc.ColumnIndex-schema.ChunkColumnsPerDay-1) |
Collaborator
There was a problem hiding this comment.
Why did the index change?
Author
There was a problem hiding this comment.
As I mentioned in the previous comment, this is a bug originally but corrected in optimizeShard. Now we removed optimizeShard and the bug manifests.
Collaborator
There was a problem hiding this comment.
Yeah you are right! I tested it on main and it works fine!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Port prometheus-community/parquet-common#117: Parallelize shard conversion to parquet-gateway. Added command line option convert.write.concurrency (default=4) to specify number of parallel parquet writers.
Test result:
I have converted one day of our production data, parquet write time went down from 8 hours 2 min to 1 hour 38 min. Total conversion time (including download/index reading/sorting/chunk reading/parquet write) went down from 10 hours 20 min to 4 hours 1 min. Memory usage is similar.