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

Reply via email to