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 <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Henry Robinson <[email protected]>
Gerrit-Reviewer: Juan Yu <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Sailesh Mukil <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-HasComments: Yes
