Michael Ho has posted comments on this change.

Change subject: IMPALA-5567: race in fragment instance teardown
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

MJ may wanna do another pass.

http://gerrit.cloudera.org:8080/#/c/7275/1/be/src/runtime/runtime-state.h
File be/src/runtime/runtime-state.h:

PS1, Line 294:   /// Returns a non-OK status if query execution should stop 
(e.g., the query was
             :   /// cancelled or a mem limit was exceeded). Exec nodes should 
check this periodically so
             :   /// execution doesn't continue if the query terminates 
abnormally. This should not be
             :   /// called after ReleaseResources().
             :   Status CheckQueryState();
> How about we rename them to be less vague. Maybe something like:
I wonder if most of the remaining callers can be converted to 
CheckQueryStatus() given that we do mostly use TryAllocate() instead of 
Allocate(). Most of the remaining callers are DataSinks. May be if we fix 
hdfs-table-writer and friends to use TryAllocate(), things will be easier ?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If567e734042b5f2b82323368dd536dbf3bdf4744
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Matthew Jacobs <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to