Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10815 )

Change subject: IMPALA-7205: Respond to ReportExecStatus() RPC with CANCELLED 
if query execution has terminated
......................................................................


Patch Set 4: Code-Review+1

(3 comments)

http://gerrit.cloudera.org:8080/#/c/10815/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

http://gerrit.cloudera.org:8080/#/c/10815/1/be/src/runtime/coordinator-backend-state.cc@366
PS1, Line 366:       rpc_status = 
DebugAction(query_ctx().client_request.query_options,
> No, I wanted to simulate what happens when a single RPC fails. That way we
Thanks for the explanation. I think the code could be more readable if we 
reduced the indent by using continue, both for !client_status.ok() above and 
for !rpc_status.ok() here. I don't feel that strongly about it though.


http://gerrit.cloudera.org:8080/#/c/10815/1/be/src/runtime/coordinator-backend-state.cc@367
PS1, Line 367: COORD_CANCEL_QUERY_FINSTANCES_RPC
> The convention I've been using for the global debug action labels is to pre
I agree, with the COORD prefix it actually is clear enough.


http://gerrit.cloudera.org:8080/#/c/10815/4/tests/query_test/test_cancellation.py
File tests/query_test/test_cancellation.py:

http://gerrit.cloudera.org:8080/#/c/10815/4/tests/query_test/test_cancellation.py@136
PS4, Line 136: debug_action =
remove one "debug_action = "



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7bb2c26edace89853f14a329f891d1f9a065a991
Gerrit-Change-Number: 10815
Gerrit-PatchSet: 4
Gerrit-Owner: Dan Hecht <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Comment-Date: Wed, 27 Jun 2018 23:06:25 +0000
Gerrit-HasComments: Yes

Reply via email to