impala-reviews mailing list archives

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

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


Patch Set 4:

(4 comments)

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

Line 19: - Then during error handling, the BaseSequenceScanner calls SkipToSync()
> I tried my repro for text files and looked at the code and saw that the tex
Thanks, this is good to know. I'm not too concerned about the parquet case - more fuzzing
seems like a good idea but not specifically related to this change.


Line 32: I tested this by running the repro from the JIRA and impalad did not
Do you think it's reasonable to turn this into an automated test? Would be nice to include
it with the fix but could be a follow-up.


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

Line 365: Status ScannerContext::Stream::WrapIoError(const Status& status) {
> It doesn't. The profile only show the "General IO error" and the error log 
Yeah I think there's some underlying issue with MergeStatus(). Not sure how hard it is to
fix it. Since this wrapping thing is temporary, maybe we should just include the error message
directly in the status instead of merging it? Kind of ugly but it will work ok I think


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

Line 367:   if (status.ok() || status.code() == TErrorCode::SCANNER_CONTEXT_WRAPPED_IO_ERROR)
{
I looked at the other errors coming from the I/O mgr. I don't think we should wrap CANCELLED
or MEM_LIMIT_EXCEEDED, which I think this does now.


-- 
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