impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) Move Impala HTTP handlers to a separate class
Date Sat, 11 Jun 2016 01:00:15 GMT
Matthew Jacobs has posted comments on this change.

Change subject: Move Impala HTTP handlers to a separate class
......................................................................


Patch Set 1:

(1 comment)

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

PS1, Line 316: http_handler_.reset(new ImpalaHttpHandler(this));
> It holds on to the pointer to 'this', i.e. the ImpalaServer that the handle
But the lifetime of 'this' is already the lifetime of this scoped object (in fact just a tad
longer, since http_hander_ gets destructed before 'this' becomes invalid). Couldn't RegisterHandlers
even be a static method which takes both 'this' and the webserver and then needs no state?
(I'm not saying we should do that since I see that would require more changes because ImpalaServer*
has to be passed around more.) While this is a small thing, it's nice to remove any unnecessary
state from ImpalaServer.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8453b3367653914163ca6acae48e7605f73cc675
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message