Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11337 )
Change subject: IMPALA-7335: Fix a race in HdfsScanNode ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/11337/4/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11337/4/be/src/exec/hdfs-scan-node.cc@527 PS4, Line 527: is already in the don > @Bikram, I think you missed this comment. Can you please take a look at th yes you are right, if we get a cancelled status and done is not set then we should set the status here. On that note, before your change the behavior seemed like that if done_ is set and the scanner thread encounters an error, then the status_ is set to that error. Eventually the error may or may not be propagated up the scan node depending on who gets hold of the lock_ first (GetNext or the error-ed ScannerThread). With your patch it seems like once done is set, the status does not change, which sounds like the right behaviour, but it'd be good to confirm on that with others. http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/11337/6/be/src/exec/hdfs-scan-node.cc@538 PS6, Line 538: // The scan node's execution completed, preserve the status_ currently set if this is only called when the execution is complete, worth adding that to the method comment. -- To view, visit http://gerrit.cloudera.org:8080/11337 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id470818846a5c69ad28a6ff695069702982aa793 Gerrit-Change-Number: 11337 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Pooja Nilangekar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 17 Sep 2018 20:32:45 +0000 Gerrit-HasComments: Yes
