You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by GitBox <gi...@apache.org> on 2022/09/26 21:43:50 UTC
[GitHub] [parquet-mr] mukund-thakur opened a new pull request, #999: Draft pr for Vectored IO integration, compilation fails now.
mukund-thakur opened a new pull request, #999:
URL: https://github.com/apache/parquet-mr/pull/999
Make sure you have checked _all_ steps below.
### Jira
- [ ] My PR addresses the following [Parquet Jira](https://issues.apache.org/jira/browse/PARQUET/) issues and references them in the PR title. For example, "PARQUET-1234: My Parquet PR"
- https://issues.apache.org/jira/browse/PARQUET-XXX
- In case you are adding a dependency, check if the license complies with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
### Tests
- [ ] My PR adds the following unit tests __OR__ does not need testing for this extremely good reason:
### Commits
- [ ] My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation
- [ ] In case of new functionality, my PR adds documentation that describes how to use it.
- All the public functions and the classes in the PR contain Javadoc that explain what it does
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r995112398
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> if `delta1` is small enough, you will merge `range1` and `range2` and do a single scan.
Yes.
> If range1 and range2 are very large so that the resulting range is even larger, do you still do a single scan, or do you split the large range into smaller ranges and tradeoff some seek cost for increased parallelism?
We don't split the ranges. If the ranges are big enough, they won't be merged.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> if `delta1` is small enough, you will merge `range1` and `range2` and do a single scan.
Yes.
> If range1 and range2 are very large so that the resulting range is even larger, do you still do a single scan, or do you split the large range into smaller ranges and tradeoff some seek cost for increased parallelism?
We don't split the ranges. If the ranges are big enough, they won't be merged.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r1001992617
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Let me try to get the corresponding ranges for Spark.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r993792933
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Well, I just went through the code of ConsecutivePartList#readAll() again. Yes, they are breaking the big range into smaller buffers but allocating all of them in one go only, so won't the memory issue still persists?
Also, if I do the change in readAll() like I have already done the commented readAllVectored(), we really won't be reducing the number of seek operations thus won't be getting the real benefits of vectored IO. It will just be like there is a big range to be fetched, we break into smaller ranges and fetch them parallelly. ( This is similar to PARQUET-2149 which you proposed and have already uploaded the PR :) ).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992670321
##########
parquet-common/src/main/java/org/apache/parquet/io/SeekableInputStream.java:
##########
@@ -23,6 +23,10 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.function.IntFunction;
+
+import org.apache.hadoop.fs.FileRange;
Review Comment:
Yeah, I agree with this. One solution would be to :
Introduce a custom ParquetFileRange class in the Parquet io module, use it in the interface and convert ParquetFileRange to hadoop FileRange in the implementation in H1SeekableInputStream and H2SeekableInputStream stream.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992824827
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
Sure moving to a new method makes sense. will do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r996038868
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Hmm. The largest you will get after a merge is 1 MB (default for S3). But if parquet gives provides an 8 MB range vectored io will not split it into 8 ranges of 1MB. The _smallest_ read a parquet file reader is likely to do is 1 MB (the default page size), so in effect we are never going to merge ranges.
Either way, we have two goals - reduce the number of seeks and increase the parallelism - which seem to be in conflict. If we increase the number of ranges (and consequently use smaller ranges) we get more seeks, and if we decrease the number of ranges we get less parallelism.
Do you have any suggestions on what is a good compromise based on experiments with Hdfs/S3 etc?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997539131
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Thanks for sharing. I actually saw those changes in your original PR as well. Don't you think it is similar to what I have done in the commented readVectored() implementation?
I guess we need to run some benchmarks. First, I need to see what type of ranges parquet generates for real-world/tpcds queries. Do you have that by any chance?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r995132403
##########
parquet-common/src/main/java/org/apache/parquet/io/SeekableInputStream.java:
##########
@@ -23,6 +23,10 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.function.IntFunction;
+
+import org.apache.hadoop.fs.FileRange;
Review Comment:
@danielcweeks Do you agree with my suggested approach? Thanks for reviewing the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r995178483
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Also, re memory allocation - as long as we are allocating the same amount of memory as the current code is allocating, we should be fine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r993981829
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
So if I have something like this -
`range 1, delta 1, range2`
I will give you two ranges, `range1`, and `range2` separated by a gap of `delta1` bytes. If `delta1` is small enough, you will merge `range1` and `range2` and do a single scan.
If `range1` and `range2` are very large so that the resulting range is even larger, do you still do a single scan, or do you split the large range into smaller ranges and tradeoff some seek cost for increased parallelism?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] danielcweeks commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
danielcweeks commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r990665831
##########
parquet-common/src/main/java/org/apache/parquet/io/SeekableInputStream.java:
##########
@@ -23,6 +23,10 @@
import java.io.IOException;
import java.io.InputStream;
import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.function.IntFunction;
+
+import org.apache.hadoop.fs.FileRange;
Review Comment:
I feel like this might be an issue. We probably don't want to introduce a Hadoop dependency here because it breaks the separation from Hadoop in the IO path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992787569
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
see comment below about using `options.getAllocator`
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
Right. I was thinking that `readAllVectored` will take all `ConsecutiveParts` as input.
Or at least move this block into a new function. You will need to do the same thing in `readNextRowGroup` as well.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Does `readVectored` allocate a single buffer per range? Or does it split each range into bite sized pieces? If all the columns are being read, a single range can be the entire row group, potentially more than a GB.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] shangxinli commented on pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
shangxinli commented on PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#issuecomment-1287286105
Hi @mukund-thakur Thanks for reaching out! I will have a look once I have time. It is the one in my radar. Meanwhile, can you fix the building errors? BTW, this is great feature and love to have it in next release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992841047
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Then it may make sense to do what the `readAllVectored` method is doing - split each `ConsecutiveParts` into contiguous sub-ranges of 8MB (configurable). Presumably when you merge ranges, you have a limit so you will get a large consecutive read but not hit the memory allocation issues.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r991511389
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1811,6 +1845,44 @@ public void readAll(SeekableInputStream f, ChunkListBuilder builder) throws IOEx
}
}
+ public void readAllVectored(SeekableInputStream f, ChunkListBuilder builder)
+ throws IOException, ExecutionException, InterruptedException {
+ LOG.warn("Reading through the vectored API.[readAllVectored]");
+ List<Chunk> result = new ArrayList<Chunk>(chunks.size());
+
+ int fullAllocations = (int) (length / options.getMaxAllocationSize());
+ int lastAllocationSize = (int) (length % options.getMaxAllocationSize());
+
+ int numAllocations = fullAllocations + (lastAllocationSize > 0 ? 1 : 0);
+ List<FileRange> fileRanges = new ArrayList<>(numAllocations);
+
+ long currentOffset = offset;
+ for (int i = 0; i < fullAllocations; i += 1) {
+ //buffers.add(options.getAllocator().allocate(options.getMaxAllocationSize()));
+ fileRanges.add(FileRange.createFileRange(currentOffset, options.getMaxAllocationSize()));
+ currentOffset = currentOffset + options.getMaxAllocationSize();
+ }
+
+ if (lastAllocationSize > 0) {
+ //buffers.add(options.getAllocator().allocate(lastAllocationSize));
+ fileRanges.add(FileRange.createFileRange(currentOffset, lastAllocationSize));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", fileRanges);
+ f.readVectored(fileRanges, ByteBuffer::allocate);
Review Comment:
Use the allocator (`options.getAllocator`)? Keep in mind the allocated buffer might be a direct byte buffer.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
I would do this the way you were planning to do it initially ( or so it appears). Move this into a readAllVectored method.
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
And make it configurable to choose between vectored I/O and non vectored I/O (see `HadoopReadOptions` and `ParquetReadOptions`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997314660
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
I have a prototype that mimics what I'm thinking. Let me share that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997546366
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> I actually saw those changes in your original PR as well.
You couldn't have ! :) My PR for async IO is rather different and a lot more complicated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997390810
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
@mukund-thakur take a look at the two most recent commits here - https://github.com/parthchandra/incubator-parquet-mr/commits/mergeScanRanges
The parallel reader takes a range and splits it equally across the thread pool (i.e there are as many streams as there are threads in the pool).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r995178115
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> We don't split the ranges. If the ranges are big enough, they won't be merged.
How big is 'big enough' ?
Perhaps, in your approach, you can break up a large `consecutiveParts` into 'big enough' sized ranges, instead of one large range, to get the benefit of parallelism?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r1002002330
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
that would be great. thanks.
Also, I reran the same with changes in readAllVectored and I can see that it breaks the big ranges into smaller ones if the size is greater than maxAllocationSize ( default 8 MB). For exmaple see the breaking in range[24928095,34504532]. note- this example is for query26.
So I guess changing in the above layer makes more sense.
> 2022-10-20 21:36:02,758 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[5326394,7673015), range[24928095,34504532), range[36603729,37991198), range[44105694,56402697), range[86874390,88752497)]
2022-10-20 21:36:03,146 [WARN] [TezChild] |hadoop.ParquetFileReader|: Reading through the vectored API.[readAllVectored]
2022-10-20 21:36:03,147 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[5326394,7673015)]
2022-10-20 21:36:03,225 [WARN] [TezChild] |hadoop.ParquetFileReader|: Reading through the vectored API.[readAllVectored]
2022-10-20 21:36:03,225 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[24928095,33316703), range[33316703,34504532)]
2022-10-20 21:36:03,352 [WARN] [TezChild] |hadoop.ParquetFileReader|: Reading through the vectored API.[readAllVectored]
2022-10-20 21:36:03,352 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[36603729,37991198)]
2022-10-20 21:36:03,439 [WARN] [TezChild] |hadoop.ParquetFileReader|: Reading through the vectored API.[readAllVectored]
2022-10-20 21:36:03,439 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[44105694,52494302), range[52494302,56402697)]
2022-10-20 21:36:03,652 [WARN] [TezChild] |hadoop.ParquetFileReader|: Reading through the vectored API.[readAllVectored]
2022-10-20 21:36:03,652 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[86874390,88752497)]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [DRAFT] PR to show Vectored IO integration, compilation fails now. [parquet-mr]
Posted by "mukund-thakur (via GitHub)" <gi...@apache.org>.
mukund-thakur closed pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
URL: https://github.com/apache/parquet-mr/pull/999
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
Re: [PR] [DRAFT] PR to show Vectored IO integration, compilation fails now. [parquet-mr]
Posted by "mukund-thakur (via GitHub)" <gi...@apache.org>.
mukund-thakur commented on PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#issuecomment-1939660626
closing this follow up is here https://github.com/apache/parquet-mr/pull/1103
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@parquet.apache.org
For additional commands, e-mail: issues-help@parquet.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r999972179
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
` 2022-10-18 21:51:28,308 [WARN] [TezChild] |hadoop.ParquetFileReader|: Doing vectored IO for ranges [range[1581775,2178915), range[6700883,9475957), range[10023390,10426141), range[12211215,15766053), range[24603672,25148984)]
`
I got these ranges while running tpcds query22 in hive on the parquet files in s3 with a scale factor of 1000. We can try the same in spark. thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992678347
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
I have changed both the places where I thought the integration can be done. I am not really sure which will give better performance results, which is why left the other portion commented.
1. One option is to change in readAllVectored as you suggested and I did before.
2. Another change (current one) is at the top layer.
The reason I moved from 1st to 2nd of because of the name ConsecutivePartList. The name suggests it is a consecutive part essentially meaning just a single range and for which we won't be getting the real vectored io benefit like parallel IO and range coalescing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992678347
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
Review Comment:
I have changed both the places where I thought the integration can be done. I am not really sure which will give better performance results, which is why left the other portion commented.
1. One option is to change in readAllVectored
2. Another change (current one) is at the top layer.
The reason I moved from 1st to 2nd of because of the name ConsecutivePartList. The name suggests it is a consecutive part essentially meaning just a single range and for which we won't be getting the real vectored io benefit like parallel IO and range coalescing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r995950328
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
https://github.com/apache/hadoop/blob/trunk/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/PositionedReadable.java#L105
This is the max size by default. Also this can be configured for S3 https://github.com/apache/hadoop/blob/trunk/hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/Constants.java#L1196
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r996074063
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
I haven't been able to do any benchmarks for Parquet yet. Also, we can always configure or even change the default later based on the outcome of the experiments we perform.
We have only done the benchmarks for Hive in orc till now. Even for the hive, I want to run more benchmarks by configuring the minseek and maxsize with different values and figure out the best default values.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#issuecomment-1287218176
Hi @ggershinsky @shangxinli
We discussed this during Apache Conference. Could you please take a look at this. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997548363
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> Don't you think it is similar to what I have done in the commented readVectored() implementation?
Yes, it is. However, the parallel reading will split a large range as opposed to what the vectored read will do.
> I guess we need to run some benchmarks. First, I need to see what type of ranges parquet generates for real-world/tpcds queries. Do you have that by any chance?
No I don't have the ranges. TPCDS is a large set of queries. Pick a scale factor and a couple of queries and I'll see what I can get for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] parthchandra commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
parthchandra commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997548363
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> Don't you think it is similar to what I have done in the commented readVectored() implementation?
Yes, it is. However, the parallel reading will split a large range as opposed to what the vectored read will do.
> I guess we need to run some benchmarks. First, I need to see what type of ranges parquet generates for real-world/tpcds queries. Do you have that by any chance?
No I don't have the ranges. TPCDS is a large set of queries. Pick a scale factor and a couple of queries and I'll see what I can get for you.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r997562516
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
> You couldn't have ! :) My PR for async IO is rather different and a lot more complicated.
Oh yeah just revisited. You are right. :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [parquet-mr] mukund-thakur commented on a diff in pull request #999: [DRAFT] PR to show Vectored IO integration, compilation fails now.
Posted by GitBox <gi...@apache.org>.
mukund-thakur commented on code in PR #999:
URL: https://github.com/apache/parquet-mr/pull/999#discussion_r992826565
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1093,10 +1099,38 @@ private ColumnChunkPageReadStore internalReadFilteredRowGroup(BlockMetaData bloc
}
}
}
- // actually read all the chunks
+ // Vectored IO up.
+
+ List<FileRange> ranges = new ArrayList<>();
for (ConsecutivePartList consecutiveChunks : allParts) {
- consecutiveChunks.readAll(f, builder);
+ ranges.add(FileRange.createFileRange(consecutiveChunks.offset, (int) consecutiveChunks.length));
+ }
+ LOG.warn("Doing vectored IO for ranges {}", ranges);
+ f.readVectored(ranges, ByteBuffer::allocate);
Review Comment:
Yes, vectored read allocates a single buffer per range.
If it can grow more than a GB, then I guess we will run into memory problems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org