Hello Zoltan Borok-Nagy, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/24636
to look at the new patch set (#9).
Change subject: IMPALA-8523: Migrate hdfsOpen to builder-based openFile API
......................................................................
IMPALA-8523: Migrate hdfsOpen to builder-based openFile API
Replace the deprecated hdfsOpenFile() on the scan read path with the
libhdfs builder-based openFile API (HADOOP-15229, exposed via libhdfs in
HDFS-14478). The builder lets Impala declare per-file configuration when
opening a file, which is the basis for the S3A read optimizations below.
OpenHdfsFileOp::Execute() now uses hdfsOpenFileBuilderAlloc/Opt/Build
and blocks on the returned future via hdfsOpenFileFutureGet() inside the
SynchronousThreadPool worker, so the existing open timeout (IMPALA-7738)
is preserved. Options are threaded from the scan node down to the open
call: HdfsFileDesc::GetFileInfo() computes them once per file so that
all scan ranges for the file (including footer/column ranges) inherit
them via ScanRange::GetFileInfo().
Unlike hdfsOpenFile(), the builder future (hdfsOpenFileFutureGet()) does
not translate the underlying Java exception to errno, so a missing file
would report "Unknown error" instead of ENOENT.
OpenHdfsFileOp::Execute() detects a file-not-found root cause and
restores errno to ENOENT before building the message, preserving the
"No such file or directory" surface that scanners and tests rely on.
The options are set with the standard, filesystem-agnostic openFile keys
and are all soft (hdfsOpenFileBuilderOpt), so filesystems that do not
understand a key ignore it and non-S3A reads are unaffected:
- fs.option.openfile.read.policy = "random" for every format read on
this path: Parquet, HUDI_PARQUET, ORC, RCFile, Avro, JSON, text and
SequenceFile. Kudu and JDBC are not read here and get no policy.
Impala unbuffers the file handle after every read, so the GET-to-EOF
that 'sequential' and the default 'adaptive' produce is re-opened and
aborted per read, while a bounded 'random' GET is not.
See open-file-options-util.cc for more details.
- fs.option.openfile.length when the file length is known (> 0), so
S3A can skip the HEAD request on open. Guarded against a stale zero
length that would truncate reads.
No new flags are added: the read policy is a performance hint, and soft
options make the change safe across filesystems.
Manual S3A validation on local MinIO, run against hadoop 3.4.2.
Counted two independent ways that agreed on every arm: MinIO server-side
`mc admin trace`, and S3A client-side DEBUG (reopen() lines plus the
IOStatistics dump at stream close).
Columnar, and the length hint -- cold 24-file Parquet scan:
SELECT count(*), sum(int_col), max(string_col) FROM s3_alltypes
HEAD requests GET requests
parent (baseline) 24 72
this commit 0 72
The known-length hint eliminates the per-file HEAD on open (24 -> 0)
with no change to the read GET pattern (72 = 24 files x 3 ranges),
confirming the S3A runtime honors fs.option.openfile.length. All 72
GETs report policy=random with 0 aborts and 0 bytes discarded, each
bounded to what was asked (footer first, then the column chunks).
Row-oriented, and the read policy -- a 32,999,923-byte uncompressed
text file, deliberately under fs.s3a.block.size so it is exactly one
scan range, read as 4 x --read_size (8MB) buffers on one cached file
handle:
arm GETs aborts bytes discarded
"random" (this commit) 4 0 0
unset (parent) 4 1 24,611,315
"sequential" 4 3 48,668,121
"sequential" downloads and discards 48.7MB to read a 31.5MB file:
every GET is bound to contentLength, unbuffer() aborts it because more
than fs.s3a.readahead.range (64kB by default) is left unread, and
Sequential.isAdaptive() is false so the stream never self-corrects.
The unset arm shows that self-correction happening -- policy=default on
the first reopen(), then policy=random on the remaining three. This is
the measured reason every format on this path gets "random" instead of a
per-format policy.
Avro behaves the same way, plus a header term. On tpch_avro.customer's
single 24,165,430-byte uncompressed file (also one scan range, 3 x 8MB
buffers) BaseSequenceScanner issues one extra 1024-byte header range
per physical file, so 4 GETs = 1 header + 3 body:
arm GETs aborts bytes discarded
"random" (this commit) 4 0 0
"sequential" 4 3 47,329,442
47.3MB discarded to read a 23.05MB file -- a worse ratio than text,
because the 1024-byte header read alone wastes a whole file's worth,
once per physical file. Under "random" that header GET is bounded to
range[0-65536] and does not abort, since the 64,512 bytes left unread
are not more than fs.s3a.readahead.range.
Testing:
- new be test: OpenFileOptionsUtilTest
- e2e tests passed
- ran exhaustive tests on Ozone and S3: same failures as on master
Change-Id: I46d810b19fe7d4859e3c2bcd7568b61fe73408c1
Assisted-by: Claude Opus 5 (Claude Code)
---
M be/src/exec/hdfs-scan-node-base.h
M be/src/exec/orc/hdfs-orc-scanner.cc
M be/src/runtime/io/disk-io-mgr.cc
M be/src/runtime/io/disk-io-mgr.h
M be/src/runtime/io/handle-cache.h
M be/src/runtime/io/handle-cache.inline.h
M be/src/runtime/io/hdfs-file-reader.cc
M be/src/runtime/io/hdfs-monitored-ops.cc
M be/src/runtime/io/hdfs-monitored-ops.h
A be/src/runtime/io/open-file-options.h
M be/src/runtime/io/request-ranges.h
M be/src/runtime/io/scan-range.cc
M be/src/util/CMakeLists.txt
A be/src/util/open-file-options-util-test.cc
A be/src/util/open-file-options-util.cc
A be/src/util/open-file-options-util.h
M tests/custom_cluster/test_hdfs_timeout.py
17 files changed, 424 insertions(+), 63 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/36/24636/9
--
To view, visit http://gerrit.cloudera.org:8080/24636
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I46d810b19fe7d4859e3c2bcd7568b61fe73408c1
Gerrit-Change-Number: 24636
Gerrit-PatchSet: 9
Gerrit-Owner: Daniel Vanko <[email protected]>
Gerrit-Reviewer: Daniel Vanko <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
