Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/15370 )
Change subject: IMPALA-6636: Use async IO in ORC scanner ...................................................................... Patch Set 25: (3 comments) http://gerrit.cloudera.org:8080/#/c/15370/25/be/src/exec/hdfs-orc-scanner.h File be/src/exec/hdfs-orc-scanner.h: http://gerrit.cloudera.org:8080/#/c/15370/25/be/src/exec/hdfs-orc-scanner.h@393 PS25, Line 393: is within footer stream clarification idea: "within the remaining part of the footer stream"? http://gerrit.cloudera.org:8080/#/c/15370/25/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/15370/25/be/src/exec/hdfs-orc-scanner.cc@1375 PS25, Line 1375: ReadFooterStream > Each subclass of HdfsScanner will have their initial stream_ assigned in Hd It is still not 100% clear to me - so until now we were reading the last 100KB, but didn't actually use it to read metadata, and now we will reuse it, as long as the ORC lib reads the metadata from the start of the footer to the end? What bugs me is that the ORC lib should start reading from the Postscript, which is at the very end of the file, and the rest of the metadata will be only read afterwards, so we will move backward very early. See https://orc.apache.org/specification/ORCv2/ I may misunderstand something, but my impression is that we could only avoid reading these things again by copying the whole initial range to memory and read from there if the offset + length falls in that range. In Parquet this is handled by copying the whole initial range to a buffer: https://github.com/apache/impala/blob/b28da054f3595bb92873433211438306fc22fbc7/be/src/exec/parquet/hdfs-parquet-scanner.cc#L1655 then read few fixed sized things from the end of the buffer, finally read the rest of the metadata as a single big Thrift struct: https://github.com/apache/impala/blob/b28da054f3595bb92873433211438306fc22fbc7/be/src/exec/parquet/hdfs-parquet-scanner.cc#L1655 https://github.com/apache/impala/blob/b28da054f3595bb92873433211438306fc22fbc7/be/src/exec/parquet/hdfs-parquet-scanner.cc#L1748 (+ I just realized that Parquet also "drops" the rest of the buffer, while it could easily contain other data, e.g. column indexes) http://gerrit.cloudera.org:8080/#/c/15370/25/be/src/exec/hdfs-orc-scanner.cc@1377 PS25, Line 1377: if (offset > stream_->file_offset()) { > We found out there is a corner case where ORC lib might want to seek backwa ah, I misunderstood IsInFooterRange() -- To view, visit http://gerrit.cloudera.org:8080/15370 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I348ad9e55f0cae7dff0d74d941b026dcbf5e4074 Gerrit-Change-Number: 15370 Gerrit-PatchSet: 25 Gerrit-Owner: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Mon, 31 Jan 2022 21:03:33 +0000 Gerrit-HasComments: Yes
