You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "wangning (Code Review)" <ge...@cloudera.org> on 2020/12/18 10:05:01 UTC

[kudu-CR] [java] allow disable TLS transportation from java client 

wangning has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16887


Change subject: [java] allow disable TLS transportation from java client
......................................................................

[java] allow disable TLS transportation from java client

In a cluster which is running in intranet, security is less concerned than
perfromance cause port is not exposed. And to disable TLS from server side
requires to reboot cluster, it's a big burden for a working cluster.
Also, on JAVA8 the TLS cipher perfromance is poor[1].

This commit introduced DISABLE_KUDU_CLIENT_TLS environment to allow
disable TLS transportation when talking to server.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
1 file changed, 15 insertions(+), 5 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 1
Gerrit-Owner: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#6) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces KUDU_CLIENT_DISABLE_TLS environment variable to allow
disabling use TLS transportation in client side.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/SystemEnvUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
4 files changed, 199 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/6
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 6
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5: Code-Review-1

This is a nice feature, but right now we don't use many environment variables, especially on the client-side. I was also thinking about adding a similar config to *require* TLS as right now it's trivial to MITM and downgrade to clear text. If we used env vars, these two vars would collide.

For these reasons, I'd rather we used code config which is a lot more robust and I feel like it's more widespread in the Java world. Env vars can still be added on the application-side if a user decides they want to control this behavior by env vars, but it's also possible to read the config from an XML or YAML config file as well if we opt to use code-configs.

I realize this patch might not be the best place to discuss this, so maybe we should bring this up on dev@ instead. In the meantime, I'd recommend against merging this so I'm voting -1.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 14:05:06 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5: Code-Review-2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 14:05:33 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#9) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces a new variable 'rpcEnableTls' in AsyncKuduClientBuilder
variable to allow disabling use TLS transportation in client side. The default
value is true.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java
7 files changed, 125 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/9
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 9
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 3:

Should this also be mirrored in the C++ client for unified behavior?


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Dec 2020 19:48:51 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has removed a vote on this change.

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5: Code-Review+2

Looks good to me.

Thank you for the contribution!


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Sun, 10 Jan 2021 21:38:29 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5:

> Patch Set 5:
> 
> > Patch Set 3:
> > 
> > Should this also be mirrored in the C++ client for unified behavior?
> 
> It seems I can do some cpp cipher/non-cipher transportation benchmark, to find if there is any performance promotion/cpu free. We also strongly relied on impala.

I attached some pics in slack, it seems that c++ client can also benefit from this. https://getkudu.slack.com/archives/C0CPXJ3CH/p1608189443378800


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Dec 2020 12:14:52 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#8) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces a new variable 'rpcEnableTls' in AsyncKuduClientBuilder
variable to allow disabling use TLS transportation in client side. The default
value is true.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java
7 files changed, 125 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/8
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 8
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16887

to look at the new patch set (#3).

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths the trade off for a working cluster.

This commit introduce DISABLE_KUDU_CLIENT_TLS environment to allow
disabling use TLS transportation in client side.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
2 files changed, 109 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#7) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces a new variable 'rpcEnableTls' in AsyncKuduClientBuilder
variable to allow disabling use TLS transportation in client side. The default
value is true.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java
7 files changed, 125 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/7
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 7
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has removed a vote on this change.

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Removed Code-Review-2 by Attila Bukor <ab...@apache.org>
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@102
PS3, Line 102: DISABLE_KUDU_CLIENT_TLS
nit: rename into KUDU_CLIENT_DISABLE_TLS

That would follow the suite with naming of other Kudu-specific environment variables.


http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@260
PS3, Line 260:       builder.addSupportedFeatures(RpcFeatureFlag.TLS_AUTHENTICATION_ONLY);
> Perhaps instead of disabling TLS all together we could use TLS_AUTHENTICATI
This sounds like a good idea, but TLS_AUTHENTICATION_ONLY should be present on the both side to work as expected.  That means that simply adding TLS_AUTHENTICATION_ONLY on the client wouldn't do any effect unless the server also adds that into the list of its features when negotiating a connection with a client that has TLS_AUTHENTICATION_ONLY in the features.

That means it would require a separate changelist to make sense of using TLS_AUTHENTICATION_ONLY effectively here.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Dec 2020 21:25:16 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 6:

This patch's author is wangning, I have obtained his consent and continue his work. Welcome to review this again.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 6
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Mon, 26 Jun 2023 06:31:34 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 6:

> Patch Set 5: Code-Review-1
> 
> This is a nice feature, but right now we don't use many environment variables, especially on the client-side. I was also thinking about adding a similar config to *require* TLS as right now it's trivial to MITM and downgrade to clear text. If we used env vars, these two vars would collide.
> 
> For these reasons, I'd rather we used code config which is a lot more robust and I feel like it's more widespread in the Java world. Env vars can still be added on the application-side if a user decides they want to control this behavior by env vars, but it's also possible to read the config from an XML or YAML config file as well if we opt to use code-configs.
> 
> I realize this patch might not be the best place to discuss this, so maybe we should bring this up on dev@ instead. In the meantime, I'd recommend against merging this so I'm voting -1.


I'll remove the variable environment and pass a clientBuilder variable to control whether rpc use TLS.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 6
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Fri, 30 Jun 2023 06:53:50 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disable TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16887

to look at the new patch set (#2).

Change subject: [java] allow disable TLS transportation from java client
......................................................................

[java] allow disable TLS transportation from java client

In a cluster which is running in intranet, security is less concerned than
perfromance cause port is not exposed. And to disable TLS from server side
requires to reboot cluster, it's a big burden for a working cluster.
Also, on JAVA8 the TLS cipher perfromance is poor[1].

This commit introduced DISABLE_KUDU_CLIENT_TLS environment to allow
disable TLS transportation when talking to server.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
2 files changed, 108 insertions(+), 5 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 2
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16887

to look at the new patch set (#5).

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces KUDU_CLIENT_DISABLE_TLS environment variable to allow
disabling use TLS transportation in client side.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/SystemEnvUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
4 files changed, 179 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/5
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 11:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16887/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16887/11//COMMIT_MSG@11
PS11, Line 11: JAVA8 the TLS cypher performance is poor
As I understand, that's just of using non-GCM TLS ciphers.  Did you try to evaluate this after commit https://github.com/apache/kudu/commit/a8fb42dc34e8f1f876db5b26fc3f5eb3196ce854 ?

Also, disabling TLS means authn tokens cannot be used since they cannot be passed over non-confidential channels.  If you want authn tokens to work, you need to address that as well.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Mon, 03 Jul 2023 05:11:05 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@102
PS3, Line 102:   static {
> Perhaps it's worth moving the methods in the reporter to a utility class fo
Done


http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@102
PS3, Line 102: 
> nit: rename into KUDU_CLIENT_DISABLE_TLS
Done


http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@260
PS3, Line 260:     }
> This sounds like a good idea, but TLS_AUTHENTICATION_ONLY should be present
Ack



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Dec 2020 09:21:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java:

http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@102
PS3, Line 102:     String disableKuduClientTlsStr = System.getenv("DISABLE_KUDU_CLIENT_TLS");
Perhaps it's worth moving the methods in the reporter to a utility class for re-use here: https://github.com/apache/kudu/blob/cece57bcdf5d8d8ce596a3c1da4c2c26472784bc/java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java#L139-L162


http://gerrit.cloudera.org:8080/#/c/16887/3/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java@260
PS3, Line 260:       builder.addSupportedFeatures(RpcFeatureFlag.TLS_AUTHENTICATION_ONLY);
Perhaps instead of disabling TLS all together we could use TLS_AUTHENTICATION_ONLY similar to loopback connections?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 3
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Dec 2020 19:57:08 +0000
Gerrit-HasComments: Yes

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#10) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This patch introduces a new method 'disableRpcTls' in AsyncKuduClientBuilder
to allow disabling use TLS transportation in client side. It's default value
is true.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java
7 files changed, 125 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/10
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 10
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has uploaded a new patch set (#11) to the change originally created by wangning. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This patch introduces a new method 'disableRpcTls' in AsyncKuduClientBuilder
to allow disabling use TLS transportation in client side. It's default value
is true.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ConnectionCache.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiator.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/KuduTestHarness.java
7 files changed, 126 insertions(+), 23 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/11
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 11:

> Patch Set 5:
> 
> > Patch Set 5: Code-Review-2
> 
> (changed my vote to -2 to make sure we don't accidentally submit)

I'll remove you -2. This patch remove the environment variable and use a AsyncKuduClient option variable. It need review again.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Sun, 02 Jul 2023 14:35:21 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5:

> Patch Set 5: Code-Review-2

(changed my vote to -2 to make sure we don't accidentally submit)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Mon, 11 Jan 2021 14:05:58 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
wangning has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 5:

> Patch Set 3:
> 
> Should this also be mirrored in the C++ client for unified behavior?

It seems I can do some cpp cipher/non-cipher transportation benchmark, to find if there is any performance promotion/cpu free. We also strongly relied on impala.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 5
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Wed, 23 Dec 2020 09:53:13 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/16887 )

Change subject: [java] allow disabling TLS transportation from java client
......................................................................


Patch Set 11: Verified+1

A test in DEBUG: LocationAwareRebalancingBasicTest.IntraLocationNoConcurrency. failed due to a tcmalloc coredump.  It does not matter with this patch.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 11
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Andrew Wong <an...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: KeDeng <kd...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Wang Xixu <14...@qq.com>
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Reviewer: wangning <19...@gmail.com>
Gerrit-Comment-Date: Sun, 02 Jul 2023 14:28:13 +0000
Gerrit-HasComments: No

[kudu-CR] [java] allow disabling TLS transportation from java client

Posted by "wangning (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Andrew Wong, Grant Henke, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16887

to look at the new patch set (#4).

Change subject: [java] allow disabling TLS transportation from java client
......................................................................

[java] allow disabling TLS transportation from java client

In a cluster which is running in intranet, sometimes security is less concerned
than performance in case port is not exposed.
As mentioned in [1], on JAVA8 the TLS cypher performance is poor.
And to disable TLS from server side requires to reboot cluster,
it worths a trade off for a working cluster.

This commit introduces KUDU_CLIENT_DISABLE_TLS environment variable to allow
disabling use TLS transportation in client side.

[1] https://issues.apache.org/jira/browse/KUDU-3133

Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
---
M java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java
A java/kudu-client/src/main/java/org/apache/kudu/util/SystemEnvUtil.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestNegotiation.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/junit/ResultReporter.java
4 files changed, 178 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/87/16887/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16887
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I762b9d6ef9257a7853684dd0aef315e5c5bba8ed
Gerrit-Change-Number: 16887
Gerrit-PatchSet: 4
Gerrit-Owner: wangning <19...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)