Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13883 )
Change subject: IMPALA-8779, IMPALA-8780: RowBatchQueue re-factoring and BufferedPRS impl ...................................................................... Patch Set 9: (4 comments) I played around with this a little bit and was definitely able to see the spooling behaviour by suspending impala-shell. I ran into a couple of crashes though - I think the problems might be around null row batches being dequeued from the queue http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/exec/buffered-plan-root-sink.cc@65 PS9, Line 65: rows_available_.NotifyAll(); It's better to drop the lock before calling notify, so that the woken thread can always grab the lock immediately. See https://issues.apache.org/jira/browse/IMPALA-6135 http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/exec/buffered-plan-root-sink.cc@74 PS9, Line 74: way wake http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/exec/buffered-plan-root-sink.cc@104 PS9, Line 104: Status BufferedPlanRootSink::GetNext( I saw a null pointer dereference causing a crash in the function. Emailed Sahil the logs. http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/runtime/deque-row-batch-queue.cc File be/src/runtime/deque-row-batch-queue.cc: http://gerrit.cloudera.org:8080/#/c/13883/9/be/src/runtime/deque-row-batch-queue.cc@61 PS9, Line 61: result->Reset(); I was able to trigger a segfault from a null pointer here by running "select * from tpch.lineitem" from impala-shell and then cancelling the query. C [impalad+0xbd0462] impala::RowBatch::Reset()+0x12 C [impalad+0x116fd74] impala::DequeRowBatchQueue::Close()+0x44 C [impalad+0xf701f5] impala::BufferedPlanRootSink::Close(impala::RuntimeState*)+0x1b5 C [impalad+0xc0d04c] impala::FragmentInstanceState::Close()+0x2c C [impalad+0xc0fde2] impala::FragmentInstanceState::Exec()+0xc2 C [impalad+0xbfb4b7] impala::QueryState::ExecFInstance(impala::FragmentInstanceState*)+0x287 C [impalad+0xe50220] impala::Thread::SuperviseThread(std::string const&, std::string const&, boost::function<void ()>, impala::ThreadDebugInfo const*, impala::Promise<long, (impala::PromiseMode)0>*)+0x310 C [impalad+0xe50dca] boost::detail::thread_data<boost::_bi::bind_t<void, void (*)(std::string const&, std::string const&, boost::function<void ()>, impala::ThreadDebugInfo const*, impala::Promise<long, (impala::PromiseMode)0>*), boost::_bi::list5<boost::_bi::value<std::string>, boost::_bi::value<std::string>, boost::_bi::value<boost::function<void ()> >, boost::_bi::value<impala::ThreadDebugInfo*>, boost::_bi::value<impala::Promise<long, (impala::PromiseMode)0>*> > > >::run()+0x7a -- To view, visit http://gerrit.cloudera.org:8080/13883 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9b1bb4b9c6f6e92c70e8fbee6ccdf48c2f85b7be Gerrit-Change-Number: 13883 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 25 Jul 2019 00:53:26 +0000 Gerrit-HasComments: Yes