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

Change subject: KUDU-2192: Enable TCP keepalive for all outbound connections
......................................................................


Patch Set 4:

(5 comments)

I tested with this patch with TLS and Kerberos enabled:

client connection to 172.31.115.178:27000 recv error: Network error: failed to 
read from TLS socket (remote: 0.0.0.0:0): Connection reset by peer (error 104)

The connection was closed to the peer was closed shortly after the iptables 
rule was installed. Confirmed that a new connection was re-created afterwards.

http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/rpc/reactor.cc
File src/kudu/rpc/reactor.cc:

http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/rpc/reactor.cc@631
PS3, Line 631:     Status keepalive_status = conn->SetTcpKeepAlive(
> nit: consider using scope-only variable here, something like
Done


http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/rpc/rpc-test.cc
File src/kudu/rpc/rpc-test.cc:

http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/rpc/rpc-test.cc@790
PS3, Line 790:   // Set up server.
> As Todd already mentioned, it would be great to have a scenario when connec
There is no easy way to simulate this situation programmatically without using 
iptables or tc qdisc netem command.

I manually tested with iptables rules with and without TLS by applying this 
patch in Impala code base.


http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/rpc/rpc-test.cc@798
PS3, Line 798:   FLAGS_tcp_keepalive_retry_count = 1;
> no need for this -- our toplevel KuduTest base class handles this for all t
Done


http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/util/net/socket.h
File src/kudu/util/net/socket.h:

http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/util/net/socket.h@160
PS3, Line 160:
> nit: it's a bit strange to see MonoDelta here, supposedly lower-level than
Done


http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/util/net/socket.cc
File src/kudu/util/net/socket.cc:

http://gerrit.cloudera.org:8080/#/c/13702/3/src/kudu/util/net/socket.cc@591
PS3, Line 591: DCHECK_GT(id
> nit: static const char* const ?
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa1d66d83aea1cc82d07fc6217be5fc1306695bc
Gerrit-Change-Number: 13702
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Tue, 25 Jun 2019 21:27:03 +0000
Gerrit-HasComments: Yes

Reply via email to