Pooja Nilangekar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11296 )

Change subject: IMPALA-7418: Return first non-ok status from 
HdfsScanNode::GetNext()
......................................................................


Patch Set 4:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@10
PS3, Line 10:
> nit: HdfsScanNode::status_
Done


http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@10
PS3, Line 10:
            : HdfsScanNodeBase::status_. This resulted in the error status be
> nit: this resulted an error status being lost in case .....happened.
Done


http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@12
PS3, Line 12: lost in case a scanner thread hit an error and cancelled the scan.
            : This change ensures that
> Does this always ensure that GetNext returns the first non-ok status?
GetNextInternal() always returns the first non-ok status_ which is set. 
However, there is a race between setting the status_ and invoking shutdown 
(IMPALA-7335). I will put up a patch for it later today.


http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h
File be/src/exec/hdfs-scan-node-base.h:

http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h@544
PS3, Line 544: A non-ok status is returned only if it encounters an invalid 
scan range or if a
             :   /// scanner thread
> nit: superflous comment. Can say: A non-OK status is returned only if......
Done


http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h@545
PS3, Line 545: cancels the scan when it runs into an error.
             :   Status IssueInitialScanRanges(RuntimeState* state) 
WARN_UNUSED_RES
> nit: avoid reference to private members, how about:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4569cc7b0843a29c617a094e590c31f7c648ff45
Gerrit-Change-Number: 11296
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Aug 2018 18:11:30 +0000
Gerrit-HasComments: Yes

Reply via email to