impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4670 / IMPALA-4672: Add RpcMgr and port Statestore services to KRPC
Date Wed, 25 Jan 2017 01:31:58 GMT
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4670 / IMPALA-4672: Add RpcMgr and port Statestore services to KRPC
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/5720/3/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

PS3, Line 52: 4
> Could be left as future work, but just wanted to bring it up. Do you think 
Yeah, this should be plumbed through, will do that.


PS3, Line 64: service_pool->Init
> Safer to Init() after calling messenger_->RegisterService() ?
Hm, why do you think so? Seems more safe to get the threads running before the service is
'exposed' via RegisterService() (even though StartServices() is where requests are actually
allowed to start arriving).


PS3, Line 74: int32_t num_acceptor_threads
> If this is already a user controlled flag, why pass it in as a parameter?
Because the caller may not want to respect the flag's value (maybe for tests or something).


http://gerrit.cloudera.org:8080/#/c/5720/3/be/src/rpc/rpc.h
File be/src/rpc/rpc.h:

PS3, Line 90: func
> As I understand it currently, if 'func' is the asynchronous variant of the 
You simply can't call asynchronous functions with Execute() - there's no callback provided
to the method invocation on line 108.

ExecuteAsync() is in the next patch. There's no use cases for it in this one.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8dbf12b9ecd71d26d239d31c19b487175194c766
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: David Knupp <dknupp@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Juan Yu <jyu@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message