GH-3078: Use Hadoop FileSystem.openFile() to open files #3079
Conversation
* Open files with FileSystem.openFile(), passing in file status * And read policy of "parquet, vector, random, adaptive"
steveloughran
force-pushed
the
GH-3078-openfile
branch
from
b1b490b
to
ab31f8f
Compare
|
@Fokko @gszadovszky Could you help review this? Thanks! |
| try { | ||
| return future.get(); | ||
| } catch (InterruptedException e) { | ||
| throw (InterruptedIOException) new InterruptedIOException(e.toString()).initCause(e); |
There was a problem hiding this comment.
initCause() returns a throwable; you need to casr it back to what the actual exception is so you can throw it
There was a problem hiding this comment.
Oh, missed that, thanks!
| // equal the path in the FileStatus; Hive virtual FS could create this condition. | ||
| // As the path to open is derived from stat.getPath(), this condition seems | ||
| // near-impossible to create -but is handled here for due diligence. | ||
| stream = fs.open(stat.getPath()); |
There was a problem hiding this comment.
Shouldn't we at least log the original exception?
There was a problem hiding this comment.
good point. at debug? I'm never a fan of excessively noisy libraries as they ruin the lives of people downstream, especially if messages for developers end up being printed in user logs far too often. In fact, I'm not above dynamically patching log levels to shut those libraries up (HADOOP-19272)...
stack trace then, debug.
error text itself? Probably the same unless there's a desire to log exactly once during the life of a process, which could be done trivially
There was a problem hiding this comment.
I'm fine with any solutions just did not want to loose the exception may occur.
Another idea I've had is to catch the potential exception may come out of this line (let's call it e2) and add the previously caught e as suppressed to e2, then re-throw. But it might be an overkill since this code path "near-impossible" to be taken.
Rationale for this change
If parquet passes down the
FileStatusit previously collected, S3A and ABFS clients can skip their own HEAD probe. This saves time, money and IO capacity.When the read policy is passed in, the clients know immediately what kind of read pattern to expect, rather than infer it as reads take place or from the file suffix.
What changes are included in this PR?
Are these changes tested?
Through parquet-hadoop.
Are there any user-facing changes?
no.
Closes #3078