Dan Hecht has posted comments on this change.

Change subject: IMPALA-{4670,4672,4784}: Add RpcMgr and port Statestore 
services to KRPC
......................................................................


Patch Set 4:

(4 comments)

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

PS3, Line 41: 0
> It means that there are 0 services managed by the RpcMgr? Since the RpcMgr 
Thanks, the "service and clients" duality here distinct from "service and 
proxy" duality in KuduRPC threw me off.


PS3, Line 49: Service
> I've tried to standardize a bit better. 'clients' and 'servers' are pretty 
Thanks. Sure they are established nouns, but here we are talking about specific 
layer of the software stack so being consistent and always saying 'proxy' makes 
things clearer IMO. e.g. I wasn't sure if this comment was trying to introduce 
another concept or layer.


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

PS3, Line 36: Clients
> I've reworded rpc-mgr.h to standardize on Proxy. I'll write 'callers' here 
now that rpc-mgr.h doesn't use client as top level terminology, saying 
"clients" here is okay (or callers is okay too but you didn't actually change 
it).

Though I still think single RPC instance is still confusing (but wasn't 
reworded).


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

PS4, Line 36: single RPC instance
still confusing


-- 
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: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: David Knupp <dkn...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Juan Yu <j...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to