Copilot commented on code in PR #6955:
URL: https://github.com/apache/hive/pull/6955#discussion_r4526329138
##########
llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ParquetColumnVectorProducer.java:
##########
@@ -106,6 +120,25 @@ public ReadPipeline
createReadPipeline(Consumer<ColumnVectorBatch> consumer, Fil
}
}
+ /**
+ * Whether a failed footer read is the thread being interrupted rather than
the file failing to
+ * read. The cause chain is walked because the footer cache and Hadoop wrap
what they catch. A
+ * {@link SocketTimeoutException} is an {@link InterruptedIOException} too,
but it is a timeout,
+ * so it falls back like any other IO failure.
+ */
+ private static boolean isInterrupt(IOException e) {
+ if (Thread.currentThread().isInterrupted()) {
+ return true;
+ }
+ for (Throwable t = e; t != null; t = t.getCause() == t ? null :
t.getCause()) {
+ if (t instanceof InterruptedException || t instanceof
ClosedByInterruptException
+ || (t instanceof InterruptedIOException && !(t instanceof
SocketTimeoutException))) {
Review Comment:
`RecordReaderWrapper.create` catches a directly propagated
`InterruptedIOException` and returns `ZeroRowsRecordReader` without checking
the limit. Thus this newly accepted interruption becomes an empty split even
when no limit was reached, instead of propagating the failure. Update the
wrapper to propagate non-limit interruptions and add a regression test through
`HiveInputFormat`.
##########
llap-server/src/java/org/apache/hadoop/hive/llap/io/decode/ParquetColumnVectorProducer.java:
##########
@@ -97,6 +100,17 @@ public ReadPipeline
createReadPipeline(Consumer<ColumnVectorBatch> consumer, Fil
edc.init(reader, reader);
return edc;
} catch (IOException e) {
+ if (isInterrupt(e)) {
+ // Not a split this pipeline cannot serve: the fragment is being
killed, or the daemon shut
+ // down. Falling back would only restart the same read on the stock
reader, on a thread that
+ // should be unwinding. Rethrown as is, so the interrupt stays the
root cause that
+ // HiveInputFormat's limit-reached check looks for, with the interrupt
flag restored in case
+ // whoever wrapped it consumed it.
+ LlapIoImpl.LOG.info("Parquet native cache: interrupted while reading
the footer of {}, "
+ + "not falling back to normal reader", split.getPath());
+ Thread.currentThread().interrupt();
+ throw e;
Review Comment:
When the limit is already reached, rethrowing a plain `IOException` on an
interrupted thread or an `IOException` wrapping `InterruptedIOException` still
causes reader creation to fail. `HiveInputFormat.getRecordReader` only returns
`NullRowsRecordReader` for a deepest cause of `InterruptedException` or
`ClosedByInterruptException`, so it misses these newly supported forms. Align
the downstream limit check with this interruption handling and add an
end-to-end limit-reached test.
--
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]
