You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Sailesh Mukil (Code Review)" <ge...@cloudera.org> on 2017/03/30 23:21:43 UTC
[kudu-CR] [rpc] WIP: Introduce configurable options to Messenger
Sailesh Mukil has uploaded a new change for review.
http://gerrit.cloudera.org:8080/6520
Change subject: [rpc] WIP: Introduce configurable options to Messenger
......................................................................
[rpc] WIP: Introduce configurable options to Messenger
Currently, the RPC layer accesses many gflags directly to take
certain decisions, eg. whether to turn on encryption, authentication,
etc.
Since the RPC layer is to be used more like a library, these should
be configurable options that are passed to the Messenger (which is
the API endpoint for the application using the RPC layer), instead
of the RPC layer itself directly accessing these flags.
The flags that have been plumbed out of the Messenger are:
FLAGS_rpc_authentication
FLAGS_rpc_encryption
FLAGS_rpc_certificate_file
FLAGS_rpc_private_key_file
FLAGS_rpc_ca_certificate_file
Future work:
There are more flags that need to be plumbed out, however some of
them are more complicated since they are accessed independantly
in many parts of the RPC code.
Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
---
M src/kudu/integration-tests/create-table-stress-test.cc
M src/kudu/integration-tests/external_mini_cluster.cc
M src/kudu/integration-tests/mini_cluster.cc
M src/kudu/integration-tests/raft_consensus-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/rpc/client_negotiation.cc
M src/kudu/rpc/client_negotiation.h
M src/kudu/rpc/messenger.cc
M src/kudu/rpc/messenger.h
M src/kudu/rpc/mt-rpc-test.cc
M src/kudu/rpc/negotiation-test.cc
M src/kudu/rpc/negotiation.cc
M src/kudu/rpc/negotiation.h
M src/kudu/rpc/reactor.cc
M src/kudu/rpc/rpc-test-base.h
M src/kudu/rpc/rpc-test.cc
M src/kudu/rpc/server_negotiation.h
A src/kudu/security/security_flags.h
M src/kudu/server/server_base.cc
M src/kudu/util/flags.h
20 files changed, 277 insertions(+), 254 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/20/6520/1
--
To view, visit http://gerrit.cloudera.org:8080/6520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
[kudu-CR] [rpc] WIP: Introduce configurable options to Messenger
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change.
Change subject: [rpc] WIP: Introduce configurable options to Messenger
......................................................................
Patch Set 1:
> Build Failed
>
> http://104.196.14.100/job/kudu-gerrit/7167/ : FAILURE
I'll do the cleanup when the general structure is agreed upon.
Just want to make sure that the reviewers are okay with this approach. Particularly about introducing a new header for RpcEncryption and RpcAuthentication.
--
To view, visit http://gerrit.cloudera.org:8080/6520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [rpc] WIP: Introduce configurable options to Messenger
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change.
Change subject: [rpc] WIP: Introduce configurable options to Messenger
......................................................................
Patch Set 1:
Dan's on PTO this week but is back next week, so I'm guessing he'll take a look when he's back (he's most knowledgeable about this stuff)
--
To view, visit http://gerrit.cloudera.org:8080/6520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: No
[kudu-CR] [rpc] WIP: Introduce configurable options to Messenger
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has abandoned this change. ( http://gerrit.cloudera.org:8080/6520 )
Change subject: [rpc] WIP: Introduce configurable options to Messenger
......................................................................
Abandoned
Re-did the patch here:
https://gerrit.cloudera.org/#/c/8789/
--
To view, visit http://gerrit.cloudera.org:8080/6520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
Gerrit-Change-Number: 6520
Gerrit-PatchSet: 1
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [rpc] WIP: Introduce configurable options to Messenger
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change.
Change subject: [rpc] WIP: Introduce configurable options to Messenger
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/messenger.cc
File src/kudu/rpc/messenger.cc:
Line 77: : connection_keepalive_time(
It seems a little troublesome to have the defaults duplicated between the flags and here, but I don't really have a better idea.
http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/messenger.h
File src/kudu/rpc/messenger.h:
Line 81: struct MessengerBuilderOptions {
Not quite seeing how this is distinguished from MessengerBuilder. They both seem to be a builder pattern, just different styles? Consider combining them by turning MessengerBuilder into a struct and add all the options from this into it.
Line 93: std::string rpc_encryption;
Could these flags instead by the parsed types, i.e. RpcAuthentication and RpcEncryption.
http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/rpc/negotiation.h
File src/kudu/rpc/negotiation.h:
Line 28: namespace security {
unused?
http://gerrit.cloudera.org:8080/#/c/6520/1/src/kudu/server/server_base.cc
File src/kudu/server/server_base.cc:
Line 122: DEFINE_string(rpc_certificate_file, "",
whitespace
--
To view, visit http://gerrit.cloudera.org:8080/6520
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3685f137770d46f7c6537a37f76a0a6f71a00b11
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-HasComments: Yes