Mike Percy has posted comments on this change. Change subject: KUDU-2021 retry master RPC if negotiation times out ......................................................................
Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/6927/6//COMMIT_MSG Commit Message: Line 7: KUDU-2021 retry master RPC if negotiation times out How about: KUDU-2021 Add regression test for negotiation timeout on Master RPCs http://gerrit.cloudera.org:8080/#/c/6927/6/src/kudu/integration-tests/client_failover-itest.cc File src/kudu/integration-tests/client_failover-itest.cc: Line 484: static const int kTimeoutMs = 1 * 60 * 1000; nit: how about const MonoDelta kTimeout = MonoDelta::FromSeconds(60) Line 488: cluster_opts_.master_rpc_ports = { 37030, 37031, 37032 }; huh. do we have to pick these manually? Line 540: ASSERT_OK(cluster_->SetFlag(m, "rpc_negotiation_inject_delay_ms", "500")); It's not obvious to me why a 500ms negotiation delay is sufficient to cause a timeout in this test Line 542: SleepFor(MonoDelta::FromSeconds(3)); What's the purpose of this sleep? Is it so this executes after ListTables() is executed? Is that required for this test to pass? It seems like we should be pausing the LeaderMaster then doing the ListTables immediately. Line 548: ScopedResumeExternalDaemon resumer(m); CHECK_OK(m->Resume()); ? -- To view, visit http://gerrit.cloudera.org:8080/6927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib62126c9d8c6c65f447c5d03a0377eaff823393c Gerrit-PatchSet: 6 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <mpe...@apache.org> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
