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