Sahil Takiar has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16351 )

Change subject: IMPALA-10065: Fix DCHECK when retrying a query in FINISHED state
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16351/1/be/src/runtime/query-driver.cc
File be/src/runtime/query-driver.cc:

http://gerrit.cloudera.org:8080/#/c/16351/1/be/src/runtime/query-driver.cc@99
PS1, Line 99: If fetch() is called but nothing was fetched (e.g. due to fetch()
            :     // timeout), the query is still retryable.
>  I think we need this. WaitForResults() returns after the coordinator 
> fragment instance finishes Open(). It doens't mean some results are ready to 
> be fetched.

IIRC when Open() returns the first RowBatch is actually available to to be 
fetched. This confused me as well, and I was able to track down the change that 
did this, but I can't seem to find it now.

The code comments for ImpalaServer::WaitForResults, ImpalaServer::BlockOnWait, 
ClientRequestState::Wait, Coordinator::Wait all confirm this behavior (e.g. 
that Wait() "Blocks until result rows are ready to be retrieved via 
GetNext()"). The ExecState is also set to FINISHED by the time WaitForResults 
returns.

> The real timeout can happen in the following ClientRequestState::FetchRows(). 
> Then the client still fetches nothing and will trigger another fetch() 
> request.

This timeout was actually added as a timeout on the time taken to materialize 
rows (e.g. convert from RowBatch to client side format). See IMPALA-7312.

> As the comment of fetched_rows_ said, it means whether a fetch was attempted 
> by a client, regardless of whether a result set (or error) was returned to 
> the client.

Right, but it is only set after WaitForResults is called. Maybe its worth 
clarifying that in the description of fetched_rows_.

Does the test still pass without this change?



--
To view, visit http://gerrit.cloudera.org:8080/16351
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11d82bf80640760a47325833463def8a3791bdda
Gerrit-Change-Number: 16351
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com>
Gerrit-Reviewer: Sahil Takiar <stak...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Aug 2020 17:25:03 +0000
Gerrit-HasComments: Yes

Reply via email to