wgtmac commented on code in PR #1139:
URL: https://github.com/apache/parquet-mr/pull/1139#discussion_r1519893179
##########
parquet-hadoop/src/main/java/org/apache/parquet/hadoop/ParquetFileReader.java:
##########
@@ -1132,6 +1138,109 @@ public ColumnChunkPageReadStore
readFilteredRowGroup(int blockIndex, RowRanges r
return internalReadFilteredRowGroup(block, rowRanges,
getColumnIndexStore(blockIndex));
}
+ /**
+ * Read data in all parts via either vectored IO or serial IO.
+ * @param allParts all parts to be read.
+ * @param builder used to build chunk list to read the pages for the
different columns.
+ * @throws IOException any IOE.
+ */
+ private void readAllPartsVectoredOrNormal(List<ConsecutivePartList>
allParts, ChunkListBuilder builder)
+ throws IOException {
+
+ if (shouldUseVectoredIO(allParts)) {
+ try {
+ readVectored(allParts, builder);
+ return;
+ } catch (IOException | IllegalArgumentException |
UnsupportedOperationException e) {
+ // possible failure modes.
+ LOG.warn("readVectored() failed; falling back to normal IO", e);
+ }
+ }
+ for (ConsecutivePartList consecutiveChunks : allParts) {
+ consecutiveChunks.readAll(f, builder);
+ }
+ }
+
+ /**
+ * Should the read use vectored IO?
+ * <p>
+ * This returns true if all necessary conditions are met:
+ * <ol>
+ * <li> The option is enabled</li>
+ * <li> The Hadoop version supports vectored IO</li>
+ * <li> The part lengths are all valid for vectored IO</li>
+ * <li> The stream implementation explicitly supports the API; for other
streams the classic
+ * API is always used.</li>
+ * <li> The allocator is not direct. This is to avoid HADOOP-19101
surfacing.
+ * </ol>
+ * @param allParts all parts to read.
+ * @return true or false.
+ */
+ private boolean shouldUseVectoredIO(final List<ConsecutivePartList>
allParts) {
+ return options.useHadoopVectoredIO()
+ && !options.getAllocator().isDirect()
+ && f.readVectoredAvailable()
+ && arePartsValidForVectoredIO(allParts);
+ }
+
+ /**
+ * Validated the parts for vectored IO.
+ * Vectored IO doesn't support reading ranges of size greater than
+ * Integer.MAX_VALUE.
+ * @param allParts all parts to read.
+ * @return true or false.
+ */
+ private boolean arePartsValidForVectoredIO(List<ConsecutivePartList>
allParts) {
+ for (ConsecutivePartList consecutivePart : allParts) {
Review Comment:
+1 for IllegalArgument on my side. IMO, fallback for overlapping ranges is
an overkill. We can simply throw and let the user to turn off vectored io if
something is wrong.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
