impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4786: Clean up how ImpalaServers are created
Date Tue, 19 Sep 2017 23:43:19 GMT
Dan Hecht has posted comments on this change.

Change subject: IMPALA-4786: Clean up how ImpalaServers are created
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/8076/2/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

Line 2078:   }
i think we still need to reset these pointers since they hold a shared_ptr to 'this'. Otherwise,
we have a cycle and 'this' will never be destructed.

For that reason, I'm not really sold that moving the ownership of the ThriftServer's into
this class is an improvement (due to this ownership cycle), but as long as you add the comment
in the header file about that, we can go forward.


http://gerrit.cloudera.org:8080/#/c/8076/2/be/src/service/impala-server.h
File be/src/service/impala-server.h:

Line 999:   /// Init() were <= 0.
add to comment:
Note that these hold a shared pointer to 'this', and so need to be reset() explicitly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If388c5618258a9c4529cd1d63e956566b92bd0d8
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message