Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12097 )
Change subject: IMPALA-7980: Fix spinning because of buggy num_unqueued_files_. ...................................................................... Patch Set 6: (4 comments) Thanks. I think we've converged here. Annoyingly, I'll have to push this without the verifier job because I need to push the LZO change simultaneously. http://gerrit.cloudera.org:8080/#/c/12097/5/be/src/exec/hdfs-scan-node-base.h File be/src/exec/hdfs-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/12097/5/be/src/exec/hdfs-scan-node-base.h@465 PS5, Line 465: remaining_scan_range_submission > Thanks for the explanation. I suppose the initial source of confusion had t I extended the comment a bit. http://gerrit.cloudera.org:8080/#/c/12097/5/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/12097/5/be/src/exec/hdfs-scan-node.cc@79 PS5, Line 79: : Status HdfsScanNode::GetNext(RuntimeState* state, RowBatch* row_batch, bool* eos) { : SCOPED_TIMER(runtime_profile_->total_time_counter()); : Sco > I was just wondering if the if-stmt body is empty as DCHECK is not compiled I think we've converged here. The outer #ifndef NDEBUG in the current code removes the if in production, and in debug mode, it's all active. http://gerrit.cloudera.org:8080/#/c/12097/6/be/src/exec/hdfs-scan-node.cc File be/src/exec/hdfs-scan-node.cc: http://gerrit.cloudera.org:8080/#/c/12097/6/be/src/exec/hdfs-scan-node.cc@196 PS6, Line 196: !(ranges_issued_barrier_.pending() != 0) > Why not ranges_issued_barrier_.pending() == 0 ? Done http://gerrit.cloudera.org:8080/#/c/12097/6/be/src/exec/hdfs-scan-node.cc@197 PS6, Line 197: && initial_ranges_issued_ : && progress_.done()) { > nit: one line I'm down to two lines here, but one line doesn't quite fit. -- To view, visit http://gerrit.cloudera.org:8080/12097 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I133de13238d3d05c510e2ff771d48979125735b1 Gerrit-Change-Number: 12097 Gerrit-PatchSet: 6 Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Wed, 30 Jan 2019 18:09:00 +0000 Gerrit-HasComments: Yes
