Bikramjeet Vig 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 3: (5 comments) looks good to me, just some nits 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: status_ nit: HdfsScanNode::status_ http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@10 PS3, Line 10: It made : the fragment lose track of the error set by the scanner thread. nit: this resulted an error status being lost in case .....happened. http://gerrit.cloudera.org:8080/#/c/11296/3//COMMIT_MSG@12 PS3, Line 12: This change ensures that GetNext returns the first non-ok status : set in the HdfsScanNode. Does this always ensure that GetNext returns the first non-ok status? is there a possibility that GetNextInternal() might not 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: It returns Status::OK() if it successfully issues all scan ranges corresponding to : /// the scan node. nit: superflous comment. Can say: A non-OK status is returned only if...... http://gerrit.cloudera.org:8080/#/c/11296/3/be/src/exec/hdfs-scan-node-base.h@545 PS3, Line 545: A non-ok status is returned if ValidateScanRange() fails or the : /// reader_context_ is cancelled before all scan ranges are added. nit: avoid reference to private members, how about: "A non-ok status is returned only if it encounters an invalid scan range or if a scanner thread cancels the scan when it runs into an error." -- 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: 3 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 03:01:04 +0000 Gerrit-HasComments: Yes
