impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5890: Abort queries if scanner hits IO errors
Date Sat, 09 Sep 2017 03:03:09 GMT
Lars Volker has posted comments on this change.

Change subject: IMPALA-5890: Abort queries if scanner hits IO errors
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/8011/3//COMMIT_MSG
Commit Message:

Line 19: - Then during error handling, the BaseSequenceScanner calls SkipToSync()
> I wonder if the same bug can happen for text files.
I tried my repro for text files and looked at the code and saw that the text scanner doesn't
try to recover from errors during ProcessRange() but wraps it in RETURN_IF_ERROR instead.
With this change queries abort with the same General IO Error.

I also checked Parquet files, but they have the metadata at the end. Truncated files immediately
fail with WARNINGS: File 'hdfs://localhost:20500/test-warehouse/tpch.partsupp_parquet/ed44648f9252550a-5b3fa31600000000_1011706823_data.0.parq'
has an invalid version number: <UTF8 Garbage>

Should we try to create a truncated file that happens to have the correct Parquet version
number at the end?


http://gerrit.cloudera.org:8080/#/c/8011/3/be/src/exec/base-sequence-scanner.cc
File be/src/exec/base-sequence-scanner.cc:

PS3, Line 187: If ab
> I'm wondering whether we should abort the whole query or abort scanning thi
Done


http://gerrit.cloudera.org:8080/#/c/8011/3/be/src/exec/scanner-context.cc
File be/src/exec/scanner-context.cc:

Line 156:     Status status = scan_range_->GetNext(&io_buffer_);
> nit: 
Done


Line 193:     // TODO(IMPALA-5914): Remove this check once we're confident we're not hitting
it.
> This would be really confusing if a user hit this. I think we should make i
Improved the error message, filed IMPALA-5914, added a TODO. Please let me know if you'd like
the error message to be more clear.


Line 364: 
> Can we file a follow-on JIRA to rework this - e.g. ensure that all HDFS rea
I filed IMPALA-5915 for this and left a TODO in the code.


Line 365: Status ScannerContext::Stream::WrapIoError(const Status& status) {
> Does the wrapped error show up in the error log and query profile ok? I kno
It doesn't. The profile only show the "General IO error" and the error log only contains the
Java errors. The info log contains the wrapped error, too. Should we try to improve this?
Do you have a suggestion how? One way I can think of is adding modifying Status so that it
can store multiple messages instead of merging them.

>     Errors: Problem parsing file hdfs://localhost:20500/test-warehouse/tpch.partsupp_avro/000000_0
at 19998210
Scanner context encountered an I/O error


http://gerrit.cloudera.org:8080/#/c/8011/3/common/thrift/generate_error_codes.py
File common/thrift/generate_error_codes.py:

Line 337:   ("SCANNER_CONTEXT_WRAPPED_IO_ERROR", 110, "Scanner context encountered an I/O
error"),
> Can we call this something like SCANNER_CONTEXT_WRAPPED_IO_ERROR for now? I
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/8011
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I44dc95184c241fbcdbdbebad54339530680d3509
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonnell@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message