Alexey Serbin 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 3: (4 comments) 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: s = conn->SetTcpKeepAlive(FLAGS_tcp_keepalive_probe_period_s + rng_.Uniform32(4), nit: consider using scope-only variable here, something like auto s = ...; 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: TEST_P(TestRpc, TestTCPKeepalive) { As Todd already mentioned, it would be great to have a scenario when connection is dropped due to non-acked keepalive probes. 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: MonoDelta nit: it's a bit strange to see MonoDelta here, supposedly lower-level than rpc/connection.h, but see int as number of seconds in rpc/connnection.h Maybe, unify that and use integer type everywhere? 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: const string nit: static const char* const ? -- 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: 3 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: Fri, 21 Jun 2019 22:00:58 +0000 Gerrit-HasComments: Yes
