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

Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14157/4/be/src/exec/buffered-plan-root-sink.cc
File be/src/exec/buffered-plan-root-sink.cc:

http://gerrit.cloudera.org:8080/#/c/14157/4/be/src/exec/buffered-plan-root-sink.cc@159
PS4, Line 159:
> Don't need _ suffix on local variable.
Done


http://gerrit.cloudera.org:8080/#/c/14157/4/be/src/exec/buffered-plan-root-sink.cc@172
PS4, Line 172:         // wait_timeout_timer ensures the client only ever waits 
up to
> Why not just start the timer and leave it running? I think the elapsed time
Yeah makes sense. Moved the Start before the lock is acquired, and it is never 
stopped. This way it tracks the total time required to fetch, including waiting 
for rows to become available and waiting for rows to materialize. I have some 
additional tests in mind for this, but they require IMPALA-8825 first.


http://gerrit.cloudera.org:8080/#/c/14157/4/be/src/exec/buffered-plan-root-sink.cc@174
PS4, Line 174:         uint64_t wait_duration = max((uint64_t) 1,
> nit: could be a one liner:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d
Gerrit-Change-Number: 14157
Gerrit-PatchSet: 5
Gerrit-Owner: Sahil Takiar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Sahil Takiar <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Wed, 04 Sep 2019 23:54:06 +0000
Gerrit-HasComments: Yes

Reply via email to