Michael Ho has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8439 )

Change subject: IMPALA-5054: [SECURITY] Enable KRPC w/ TLS in Impala
......................................................................


Patch Set 3:

(6 comments)

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

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@105
PS3, Line 105: void use_tls(bool value) { use_tls_ = value; }
Seems more appropriate to pass it to the constructor of RpcMgr() instead of 
having this function.


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@182
PS3, Line 182:   /// messenger_ will be configured to use TLS if this is set.
True if TLS is configured for communication between Impalad backend servers. 
messenger_ ....


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.h@183
PS3, Line 183: bool use_tls_ = false;
const bool use_tls_; and initialize it in the ctor.


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

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@88
PS3, Line 88:     // TODO: Once the Messenger can take more options pertaining 
to 'rpc_authentication'
            :     // and more, we need to explicitly set those options here. 
(KUDU-2228)
This may be worth fixing too in this patch if it's not too complicated.


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/rpc/rpc-mgr.cc@117
PS3, Line 117:   RETURN_IF_ERROR(
             :       service_pool->Init(num_service_threads));
One line ?


http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h
File be/src/util/openssl-util.h:

http://gerrit.cloudera.org:8080/#/c/8439/3/be/src/util/openssl-util.h@31
PS3, Line 31: //
nit: ///



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9a14a44fdea9ab668f3714eb69fdb188bce38f5a
Gerrit-Change-Number: 8439
Gerrit-PatchSet: 3
Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com>
Gerrit-Reviewer: Michael Ho <k...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Jan 2018 22:33:41 +0000
Gerrit-HasComments: Yes

Reply via email to