Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14824 )

Change subject: IMPALA-9199: Add support for single query retries on cluster 
membership changes
......................................................................


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14824/13/be/src/runtime/query-driver.h
File be/src/runtime/query-driver.h:

http://gerrit.cloudera.org:8080/#/c/14824/13/be/src/runtime/query-driver.h@169
PS13, Line 169:
ClientRequestState has a pointer back to the QueryDriver (as does the 
Coordinator), so if ClientRequestState outlives the QueryDriver, then that 
pointer would be dangling.

I think the simplest lifetime semantic is for the QueryDriver to live as long 
as any child ClientRequestStates is alive.

I think we are pretty close to that being true. Most code looks up a 
QueryDriver and then looks up the ClientRequestState with the QueryDriver 
shared_ptr staying on the stack. One option is to formalize it and have a 
struct that contains a shared_ptr to the QueryDriver and then a pointer to 
ClientRequestState. If everything uses that struct when operating on a 
ClientRequestState, then the QueryDriver will always outlive its children 
ClientRequestStates. This is just a sketch, so I might be missing something.

An implicit way to do this is to have a shared_ptr from ClientRequestState to 
the QueryDriver.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e4a0e72a9bf8ec10b91639aefd81bef17886ddd
Gerrit-Change-Number: 14824
Gerrit-PatchSet: 13
Gerrit-Owner: Sahil Takiar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Sahil Takiar <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Comment-Date: Fri, 01 May 2020 17:48:05 +0000
Gerrit-HasComments: Yes

Reply via email to