Tim Armstrong has posted comments on this change.

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


Patch Set 5:

(3 comments)

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

Line 284:       io_buffer_bytes_left_ = 0;
Maybe add a short comment explaining why this is necessary?

// Make state consistent in case we return early with an error below.


Line 371:   Status wrapped_status(TErrorCode::SCANNER_CONTEXT_WRAPPED_IO_ERROR, 
status.msg().msg());
Maybe reference IMPALA-4697 for context on why we're doing this instead of 
MergeStatus().


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

Line 277:     Status WrapIoError(const Status& status);
> rather than wrapping, is it not possible to make our IO code return a singl
I agree, Lars filed IMPALA-5915 for this. I think there were some questions 
about how to design it - e.g. do we have a hierarchy of error codes, or some 
other way of categorising them to avoid having to enumerate all the possible 
errors in the error handling code.


-- 
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: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to