Repository: kudu Updated Branches: refs/heads/master c04981d81 -> 3b2df01c0
rpc: prevent death test from leaving behind test files I noticed this while running tests locally, though I don't know why it wasn't caught in precommit, as build-and-test.sh is supposed to look for and flag files "left behind" by tests. In any case, the issue is that a "threadsafe" death test forks and reruns the test, which generates a new test directory and creates a new set of test files. Then, the fork encounters a CHECK and crashes before cleaning up these files. Rather than fix this by making the death test a "fast" test, I opted to refactor the associated methods to properly return errors, at which point the test can evaluate the returned error. Change-Id: I235400769a85d490039df07c3c084901534bf4e1 Reviewed-on: http://gerrit.cloudera.org:8080/9454 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/3b2df01c Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/3b2df01c Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/3b2df01c Branch: refs/heads/master Commit: 3b2df01c0d75b1244bb45953953144937af8fd1c Parents: c04981d Author: Adar Dembo <[email protected]> Authored: Mon Feb 26 13:11:18 2018 -0800 Committer: Alexey Serbin <[email protected]> Committed: Tue Feb 27 06:20:30 2018 +0000 ---------------------------------------------------------------------- src/kudu/rpc/exactly_once_rpc-test.cc | 18 ++-- src/kudu/rpc/mt-rpc-test.cc | 15 +-- src/kudu/rpc/reactor-test.cc | 11 +- src/kudu/rpc/rpc-bench.cc | 11 +- src/kudu/rpc/rpc-test-base.h | 78 +++++++------- src/kudu/rpc/rpc-test.cc | 157 +++++++++++++++++------------ src/kudu/rpc/rpc_stub-test.cc | 7 +- 7 files changed, 170 insertions(+), 127 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/exactly_once_rpc-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/exactly_once_rpc-test.cc b/src/kudu/rpc/exactly_once_rpc-test.cc index 79336f3..c94e89c 100644 --- a/src/kudu/rpc/exactly_once_rpc-test.cc +++ b/src/kudu/rpc/exactly_once_rpc-test.cc @@ -199,15 +199,17 @@ class ExactlyOnceRpcTest : public RpcTestBase { SeedRandom(); } - void StartServer() { + Status StartServer() { // Set up server. - StartTestServerWithGeneratedCode(&server_addr_); - client_messenger_ = CreateMessenger("Client"); + RETURN_NOT_OK(StartTestServerWithGeneratedCode(&server_addr_)); + RETURN_NOT_OK(CreateMessenger("Client", &client_messenger_)); proxy_.reset(new CalculatorServiceProxy( client_messenger_, server_addr_, server_addr_.host())); test_picker_.reset(new TestServerPicker(proxy_.get())); request_tracker_.reset(new RequestTracker(kClientId)); attempt_nos_ = 0; + + return Status::OK(); } // An exactly once adder that uses RetriableRpc to perform the requests. @@ -386,7 +388,7 @@ class ExactlyOnceRpcTest : public RpcTestBase { // Tests that we get exactly once semantics on RPCs when we send a bunch of requests with the // same sequence number as previous requests. TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsAfterRpcCompleted) { - StartServer(); + ASSERT_OK(StartServer()); ExactlyOnceResponsePB original_resp; int mem_consumption = mem_tracker_->consumption(); { @@ -449,7 +451,7 @@ TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsAfterRpcCompleted) { // In CalculatorServiceRpc we sure that the same response is returned by all retries and, // after all the rpcs are done, we make sure that final result is the expected one. TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsWithReplicatedRpc) { - StartServer(); + ASSERT_OK(StartServer()); int kNumIterations = 10; int kNumRpcs = 10; @@ -479,7 +481,7 @@ TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsWithReplicatedRpc) { // On each iteration, after all the threads complete, we expect that the add operation was // executed exactly once. TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsWithConcurrentUpdaters) { - StartServer(); + ASSERT_OK(StartServer()); int kNumIterations = 10; int kNumThreads = 10; @@ -532,7 +534,7 @@ TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsGarbageCollection) { FLAGS_remember_clients_ttl_ms = 500; FLAGS_remember_responses_ttl_ms = 100; - StartServer(); + ASSERT_OK(StartServer()); // Make a request. ExactlyOnceResponsePB original; @@ -579,7 +581,7 @@ TEST_F(ExactlyOnceRpcTest, TestExactlyOnceSemanticsGarbageCollectionStressTest) FLAGS_remember_responses_ttl_ms = 10; FLAGS_result_tracker_gc_interval_ms = 10; - StartServer(); + ASSERT_OK(StartServer()); // The write thread runs for a shorter period to make sure client GC has a // chance to run. http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/mt-rpc-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/mt-rpc-test.cc b/src/kudu/rpc/mt-rpc-test.cc index 996aa7f..1841bc4 100644 --- a/src/kudu/rpc/mt-rpc-test.cc +++ b/src/kudu/rpc/mt-rpc-test.cc @@ -67,7 +67,8 @@ class MultiThreadedRpcTest : public RpcTestBase { void SingleCall(Sockaddr server_addr, const char* method_name, Status* result, CountDownLatch* latch) { LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("ClientSC")); + shared_ptr<Messenger> client_messenger; + CHECK_OK(CreateMessenger("ClientSC", &client_messenger)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); *result = DoTestSyncCall(p, method_name); @@ -77,7 +78,8 @@ class MultiThreadedRpcTest : public RpcTestBase { // Make RPC calls until we see a failure. void HammerServer(Sockaddr server_addr, const char* method_name, Status* last_result) { - shared_ptr<Messenger> client_messenger(CreateMessenger("ClientHS")); + shared_ptr<Messenger> client_messenger; + CHECK_OK(CreateMessenger("ClientHS", &client_messenger)); HammerServerWithMessenger(server_addr, method_name, last_result, client_messenger); } @@ -116,7 +118,7 @@ static void AssertShutdown(kudu::Thread* thread, const Status* status) { TEST_F(MultiThreadedRpcTest, TestShutdownDuringService) { // Set up server. Sockaddr server_addr; - StartTestServer(&server_addr); + ASSERT_OK(StartTestServer(&server_addr)); const int kNumThreads = 4; scoped_refptr<kudu::Thread> threads[kNumThreads]; @@ -144,9 +146,10 @@ TEST_F(MultiThreadedRpcTest, TestShutdownDuringService) { TEST_F(MultiThreadedRpcTest, TestShutdownClientWhileCallsPending) { // Set up server. Sockaddr server_addr; - StartTestServer(&server_addr); + ASSERT_OK(StartTestServer(&server_addr)); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client")); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger)); scoped_refptr<kudu::Thread> thread; Status status; @@ -282,7 +285,7 @@ static void HammerServerWithTCPConns(const Sockaddr& addr) { TEST_F(MultiThreadedRpcTest, TestShutdownWithIncomingConnections) { // Set up server. Sockaddr server_addr; - StartTestServer(&server_addr); + ASSERT_OK(StartTestServer(&server_addr)); // Start a number of threads which just hammer the server with TCP connections. vector<scoped_refptr<kudu::Thread> > threads; http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/reactor-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/reactor-test.cc b/src/kudu/rpc/reactor-test.cc index 8080723..2de5f58 100644 --- a/src/kudu/rpc/reactor-test.cc +++ b/src/kudu/rpc/reactor-test.cc @@ -27,6 +27,7 @@ #include "kudu/util/countdown_latch.h" #include "kudu/util/monotime.h" #include "kudu/util/status.h" +#include "kudu/util/test_macros.h" #include "kudu/util/thread.h" using std::shared_ptr; @@ -37,8 +38,12 @@ namespace rpc { class ReactorTest : public RpcTestBase { public: ReactorTest() - : messenger_(CreateMessenger("my_messenger", 4)), - latch_(1) { + : latch_(1) { + } + + void SetUp() override { + RpcTestBase::SetUp(); + ASSERT_OK(CreateMessenger("my_messenger", &messenger_, 4)); } void ScheduledTask(const Status& status, const Status& expected_status) { @@ -61,7 +66,7 @@ class ReactorTest : public RpcTestBase { } protected: - const shared_ptr<Messenger> messenger_; + shared_ptr<Messenger> messenger_; CountDownLatch latch_; }; http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/rpc-bench.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc-bench.cc b/src/kudu/rpc/rpc-bench.cc index 55a7409..0331e8b 100644 --- a/src/kudu/rpc/rpc-bench.cc +++ b/src/kudu/rpc/rpc-bench.cc @@ -43,6 +43,7 @@ #include "kudu/util/net/sockaddr.h" #include "kudu/util/status.h" #include "kudu/util/stopwatch.h" +#include "kudu/util/test_macros.h" #include "kudu/util/test_util.h" using std::bind; @@ -87,6 +88,7 @@ class RpcBench : public RpcTestBase { {} void SetUp() override { + RpcTestBase::SetUp(); OverrideFlagForSlowTests("run_seconds", "10"); n_worker_threads_ = FLAGS_worker_threads; @@ -94,7 +96,7 @@ class RpcBench : public RpcTestBase { // Set up server. FLAGS_rpc_encrypt_loopback_connections = FLAGS_enable_encryption; - StartTestServerWithGeneratedCode(&server_addr_, FLAGS_enable_encryption); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr_, FLAGS_enable_encryption)); } void SummarizePerf(CpuTimes elapsed, int total_reqs, bool sync) { @@ -160,7 +162,8 @@ class ClientThread { } void Run() { - shared_ptr<Messenger> client_messenger = bench_->CreateMessenger("Client"); + shared_ptr<Messenger> client_messenger; + CHECK_OK(bench_->CreateMessenger("Client", &client_messenger)); CalculatorServiceProxy p(client_messenger, bench_->server_addr_, "localhost"); @@ -256,7 +259,9 @@ TEST_F(RpcBench, BenchmarkCallsAsync) { vector<shared_ptr<Messenger>> messengers; for (int i = 0; i < threads; i++) { - messengers.push_back(CreateMessenger("Client")); + shared_ptr<Messenger> m; + ASSERT_OK(CreateMessenger("Client", &m)); + messengers.emplace_back(std::move(m)); } vector<unique_ptr<ClientAsyncWorkload>> workloads; http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/rpc-test-base.h ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h index 53de6bc..b49d6a5 100644 --- a/src/kudu/rpc/rpc-test-base.h +++ b/src/kudu/rpc/rpc-test-base.h @@ -413,10 +413,6 @@ class RpcTestBase : public KuduTest { metric_entity_(METRIC_ENTITY_server.Instantiate(&metric_registry_, "test.rpc_test")) { } - void SetUp() override { - KuduTest::SetUp(); - } - void TearDown() override { if (service_pool_) { server_messenger_->UnregisterService(service_name_); @@ -429,13 +425,14 @@ class RpcTestBase : public KuduTest { } protected: - std::shared_ptr<Messenger> CreateMessenger(const std::string &name, - int n_reactors = 1, - bool enable_ssl = false, - const std::string& rpc_certificate_file = "", - const std::string& rpc_private_key_file = "", - const std::string& rpc_ca_certificate_file = "", - const std::string& rpc_private_key_password_cmd = "") { + Status CreateMessenger(const std::string& name, + std::shared_ptr<Messenger>* messenger, + int n_reactors = 1, + bool enable_ssl = false, + const std::string& rpc_certificate_file = "", + const std::string& rpc_private_key_file = "", + const std::string& rpc_ca_certificate_file = "", + const std::string& rpc_private_key_password_cmd = "") { MessengerBuilder bld(name); if (enable_ssl) { @@ -455,9 +452,7 @@ class RpcTestBase : public KuduTest { MonoDelta::FromMilliseconds(std::min(keepalive_time_ms_ / 5, 100))); } bld.set_metric_entity(metric_entity_); - std::shared_ptr<Messenger> messenger; - CHECK_OK(bld.Build(&messenger)); - return messenger; + return bld.Build(messenger); } Status DoTestSyncCall(const Proxy &p, const char *method, @@ -563,23 +558,25 @@ class RpcTestBase : public KuduTest { LOG(INFO) << "status: " << s.ToString() << ", seconds elapsed: " << sw.elapsed().wall_seconds(); } - void StartTestServer(Sockaddr *server_addr, - bool enable_ssl = false, - const std::string& rpc_certificate_file = "", - const std::string& rpc_private_key_file = "", - const std::string& rpc_ca_certificate_file = "", - const std::string& rpc_private_key_password_cmd = "") { - DoStartTestServer<GenericCalculatorService>(server_addr, enable_ssl, rpc_certificate_file, - rpc_private_key_file, rpc_ca_certificate_file, rpc_private_key_password_cmd); + Status StartTestServer(Sockaddr *server_addr, + bool enable_ssl = false, + const std::string& rpc_certificate_file = "", + const std::string& rpc_private_key_file = "", + const std::string& rpc_ca_certificate_file = "", + const std::string& rpc_private_key_password_cmd = "") { + return DoStartTestServer<GenericCalculatorService>( + server_addr, enable_ssl, rpc_certificate_file, rpc_private_key_file, + rpc_ca_certificate_file, rpc_private_key_password_cmd); } - void StartTestServerWithGeneratedCode(Sockaddr *server_addr, bool enable_ssl = false) { - DoStartTestServer<CalculatorService>(server_addr, enable_ssl); + Status StartTestServerWithGeneratedCode(Sockaddr *server_addr, bool enable_ssl = false) { + return DoStartTestServer<CalculatorService>(server_addr, enable_ssl); } - void StartTestServerWithCustomMessenger(Sockaddr *server_addr, + Status StartTestServerWithCustomMessenger(Sockaddr *server_addr, const std::shared_ptr<Messenger>& messenger, bool enable_ssl = false) { - DoStartTestServer<GenericCalculatorService>(server_addr, enable_ssl, "", "", "", "", messenger); + return DoStartTestServer<GenericCalculatorService>( + server_addr, enable_ssl, "", "", "", "", messenger); } // Start a simple socket listening on a local port, returning the address. @@ -605,23 +602,24 @@ class RpcTestBase : public KuduTest { } template<class ServiceClass> - void DoStartTestServer(Sockaddr *server_addr, - bool enable_ssl = false, - const std::string& rpc_certificate_file = "", - const std::string& rpc_private_key_file = "", - const std::string& rpc_ca_certificate_file = "", - const std::string& rpc_private_key_password_cmd = "", - const std::shared_ptr<Messenger>& messenger = nullptr) { + Status DoStartTestServer(Sockaddr *server_addr, + bool enable_ssl = false, + const std::string& rpc_certificate_file = "", + const std::string& rpc_private_key_file = "", + const std::string& rpc_ca_certificate_file = "", + const std::string& rpc_private_key_password_cmd = "", + const std::shared_ptr<Messenger>& messenger = nullptr) { if (!messenger) { - server_messenger_ = - CreateMessenger("TestServer", n_server_reactor_threads_, enable_ssl, rpc_certificate_file, - rpc_private_key_file, rpc_ca_certificate_file, rpc_private_key_password_cmd); + RETURN_NOT_OK(CreateMessenger( + "TestServer", &server_messenger_, n_server_reactor_threads_, enable_ssl, + rpc_certificate_file, rpc_private_key_file, rpc_ca_certificate_file, + rpc_private_key_password_cmd)); } else { server_messenger_ = messenger; } std::shared_ptr<AcceptorPool> pool; - ASSERT_OK(server_messenger_->AddAcceptorPool(Sockaddr(), &pool)); - ASSERT_OK(pool->Start(2)); + RETURN_NOT_OK(server_messenger_->AddAcceptorPool(Sockaddr(), &pool)); + RETURN_NOT_OK(pool->Start(2)); *server_addr = pool->bind_address(); mem_tracker_ = MemTracker::CreateTracker(-1, "result_tracker"); result_tracker_.reset(new ResultTracker(mem_tracker_)); @@ -631,7 +629,9 @@ class RpcTestBase : public KuduTest { scoped_refptr<MetricEntity> metric_entity = server_messenger_->metric_entity(); service_pool_ = new ServicePool(std::move(service), metric_entity, service_queue_length_); server_messenger_->RegisterService(service_name_, service_pool_); - ASSERT_OK(service_pool_->Init(n_worker_threads_)); + RETURN_NOT_OK(service_pool_->Init(n_worker_threads_)); + + return Status::OK(); } protected: http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/rpc-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc-test.cc b/src/kudu/rpc/rpc-test.cc index 94a22cf..f279bff 100644 --- a/src/kudu/rpc/rpc-test.cc +++ b/src/kudu/rpc/rpc-test.cc @@ -75,7 +75,6 @@ METRIC_DECLARE_histogram(rpc_incoming_queue_time); DECLARE_bool(rpc_reopen_outbound_connections); DECLARE_int32(rpc_negotiation_inject_delay_ms); -DECLARE_int32(rpc_duration_too_long_ms); using std::shared_ptr; using std::string; @@ -108,7 +107,8 @@ TEST_F(TestRpc, TestSockaddr) { } TEST_P(TestRpc, TestMessengerCreateDestroy) { - shared_ptr<Messenger> messenger(CreateMessenger("TestCreateDestroy", 1, GetParam())); + shared_ptr<Messenger> messenger; + ASSERT_OK(CreateMessenger("TestCreateDestroy", &messenger, 1, GetParam())); LOG(INFO) << "started messenger " << messenger->name(); messenger->Shutdown(); } @@ -120,7 +120,8 @@ TEST_P(TestRpc, TestMessengerCreateDestroy) { TEST_P(TestRpc, TestAcceptorPoolStartStop) { int n_iters = AllowSlowTests() ? 100 : 5; for (int i = 0; i < n_iters; i++) { - shared_ptr<Messenger> messenger(CreateMessenger("TestAcceptorPoolStartStop", 1, GetParam())); + shared_ptr<Messenger> messenger; + ASSERT_OK(CreateMessenger("TestAcceptorPoolStartStop", &messenger, 1, GetParam())); shared_ptr<AcceptorPool> pool; ASSERT_OK(messenger->AddAcceptorPool(Sockaddr(), &pool)); Sockaddr bound_addr; @@ -157,7 +158,7 @@ TEST_P(TestRpc, TestNegotiationDeadlock) { CHECK_OK(mb.Build(&messenger)); Sockaddr server_addr; - StartTestServerWithCustomMessenger(&server_addr, messenger, enable_ssl); + ASSERT_OK(StartTestServerWithCustomMessenger(&server_addr, messenger, enable_ssl)); Proxy p(messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -169,11 +170,12 @@ TEST_P(TestRpc, TestCall) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); ASSERT_STR_CONTAINS(p.ToString(), strings::Substitute("kudu.rpc.GenericCalculatorService@" @@ -199,11 +201,12 @@ TEST_P(TestRpc, TestCallWithChainCerts) { &rpc_ca_certificate_file)); // Set up server. Sockaddr server_addr; - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. SCOPED_TRACE(strings::Substitute("Connecting to $0", server_addr.ToString())); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl, + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl, rpc_certificate_file, rpc_private_key_file, rpc_ca_certificate_file)); Proxy p(client_messenger, server_addr, server_addr.host(), @@ -228,18 +231,19 @@ TEST_P(TestRpc, TestCallWithPasswordProtectedKey) { string rpc_private_key_password_cmd; string passwd; ASSERT_OK(security::CreateTestSSLCertWithEncryptedKey(GetTestDataDirectory(), - &rpc_certificate_file, - &rpc_private_key_file, - &passwd)); + &rpc_certificate_file, + &rpc_private_key_file, + &passwd)); rpc_ca_certificate_file = rpc_certificate_file; rpc_private_key_password_cmd = strings::Substitute("echo $0", passwd); // Set up server. Sockaddr server_addr; - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. SCOPED_TRACE(strings::Substitute("Connecting to $0", server_addr.ToString())); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl, + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl, rpc_certificate_file, rpc_private_key_file, rpc_ca_certificate_file, rpc_private_key_password_cmd)); Proxy p(client_messenger, server_addr, server_addr.host(), @@ -257,30 +261,32 @@ TEST_P(TestRpc, TestCallWithBadPasswordProtectedKey) { bool enable_ssl = GetParam(); // We're only interested in running this test with TLS enabled. if (!enable_ssl) return; - ::testing::FLAGS_gtest_death_test_style = "threadsafe"; string rpc_certificate_file; string rpc_private_key_file; string rpc_ca_certificate_file; string rpc_private_key_password_cmd; string passwd; - CHECK_OK(security::CreateTestSSLCertWithEncryptedKey(GetTestDataDirectory(), - &rpc_certificate_file, - &rpc_private_key_file, - &passwd)); + ASSERT_OK(security::CreateTestSSLCertWithEncryptedKey(GetTestDataDirectory(), + &rpc_certificate_file, + &rpc_private_key_file, + &passwd)); // Overwrite the password with an invalid one. passwd = "badpassword"; rpc_ca_certificate_file = rpc_certificate_file; rpc_private_key_password_cmd = strings::Substitute("echo $0", passwd); // Verify that the server fails to start up. Sockaddr server_addr; - ASSERT_DEATH(StartTestServer(&server_addr, enable_ssl, rpc_certificate_file, rpc_private_key_file, - rpc_ca_certificate_file, rpc_private_key_password_cmd), "failed to load private key file"); + Status s = StartTestServer(&server_addr, enable_ssl, rpc_certificate_file, rpc_private_key_file, + rpc_ca_certificate_file, rpc_private_key_password_cmd); + ASSERT_TRUE(s.IsRuntimeError()); + ASSERT_STR_CONTAINS(s.ToString(), "failed to load private key file"); } // Test that connecting to an invalid server properly throws an error. TEST_P(TestRpc, TestCallToBadServer) { - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, GetParam())); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam())); Sockaddr addr; addr.set_port(0); Proxy p(client_messenger, addr, addr.host(), @@ -300,11 +306,12 @@ TEST_P(TestRpc, TestInvalidMethodCall) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -320,10 +327,11 @@ TEST_P(TestRpc, TestWrongService) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client with the wrong service name. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, "localhost", "WrongServiceName"); // Call the method which fails. @@ -357,8 +365,9 @@ TEST_P(TestRpc, TestHighFDs) { // Set up server and client, and verify we can make a successful call. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); ASSERT_OK(DoTestSyncCall(p, GenericCalculatorService::kAddMethodName)); @@ -374,11 +383,12 @@ TEST_P(TestRpc, TestConnectionKeepalive) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -418,11 +428,12 @@ TEST_P(TestRpc, TestConnectionAlwaysKeepalive) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -459,11 +470,12 @@ TEST_P(TestRpc, TestClientConnectionMetrics) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -519,11 +531,12 @@ TEST_P(TestRpc, TestReopenOutboundConnections) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -561,11 +574,12 @@ TEST_P(TestRpc, TestCredentialsPolicy) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -631,10 +645,11 @@ TEST_P(TestRpc, TestCallLongerThanKeepalive) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -653,10 +668,11 @@ TEST_P(TestRpc, TestRpcSidecar) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, GetParam())); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam())); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -682,10 +698,11 @@ TEST_P(TestRpc, TestRpcSidecarLimits) { string s = "foo"; int idx; for (int i = 0; i < TransferLimits::kMaxSidecars; ++i) { - CHECK_OK(controller.AddOutboundSidecar(RpcSidecar::FromSlice(Slice(s)), &idx)); + ASSERT_OK(controller.AddOutboundSidecar(RpcSidecar::FromSlice(Slice(s)), &idx)); } - CHECK(!controller.AddOutboundSidecar(RpcSidecar::FromSlice(Slice(s)), &idx).ok()); + ASSERT_TRUE(controller.AddOutboundSidecar( + RpcSidecar::FromSlice(Slice(s)), &idx).IsRuntimeError()); } { @@ -693,10 +710,11 @@ TEST_P(TestRpc, TestRpcSidecarLimits) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, GetParam())); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam())); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -730,8 +748,9 @@ TEST_P(TestRpc, TestRpcSidecarLimits) { TEST_P(TestRpc, TestCallTimeout) { Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -759,8 +778,9 @@ TEST_P(TestRpc, TestCallTimeout) { TEST_P(TestRpc, TestCallTimeoutDoesntAffectNegotiation) { Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -804,7 +824,8 @@ TEST_F(TestRpc, TestNegotiationTimeout) { &acceptor_thread)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client")); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -826,7 +847,8 @@ TEST_F(TestRpc, TestServerShutsDown) { // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client")); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -908,10 +930,11 @@ TEST_P(TestRpc, TestRpcHandlerLatencyMetric) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServerWithGeneratedCode(&server_addr, enable_ssl); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), CalculatorService::static_service_name()); @@ -950,7 +973,8 @@ static void DestroyMessengerCallback(shared_ptr<Messenger>* messenger, } TEST_P(TestRpc, TestRpcCallbackDestroysMessenger) { - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, GetParam())); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, GetParam())); Sockaddr bad_addr; CountDownLatch latch(1); @@ -976,10 +1000,11 @@ TEST_P(TestRpc, TestRpcContextClientDeadline) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServerWithGeneratedCode(&server_addr, enable_ssl); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), CalculatorService::static_service_name()); @@ -1003,10 +1028,11 @@ TEST_P(TestRpc, TestApplicationFeatureFlag) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServerWithGeneratedCode(&server_addr, enable_ssl); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), CalculatorService::static_service_name()); @@ -1045,10 +1071,11 @@ TEST_P(TestRpc, TestApplicationFeatureFlagUnsupportedServer) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServerWithGeneratedCode(&server_addr, enable_ssl); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr, enable_ssl)); // Set up client. - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), CalculatorService::static_service_name()); @@ -1080,11 +1107,12 @@ TEST_P(TestRpc, TestCancellation) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); @@ -1143,11 +1171,12 @@ TEST_P(TestRpc, TestCancellationAsync) { // Set up server. Sockaddr server_addr; bool enable_ssl = GetParam(); - StartTestServer(&server_addr, enable_ssl); + ASSERT_OK(StartTestServer(&server_addr, enable_ssl)); // Set up client. LOG(INFO) << "Connecting to " << server_addr.ToString(); - shared_ptr<Messenger> client_messenger(CreateMessenger("Client", 1, enable_ssl)); + shared_ptr<Messenger> client_messenger; + ASSERT_OK(CreateMessenger("Client", &client_messenger, 1, enable_ssl)); Proxy p(client_messenger, server_addr, server_addr.host(), GenericCalculatorService::static_service_name()); http://git-wip-us.apache.org/repos/asf/kudu/blob/3b2df01c/src/kudu/rpc/rpc_stub-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/rpc/rpc_stub-test.cc b/src/kudu/rpc/rpc_stub-test.cc index 5fe2885..d5b85d1 100644 --- a/src/kudu/rpc/rpc_stub-test.cc +++ b/src/kudu/rpc/rpc_stub-test.cc @@ -39,7 +39,6 @@ #include "kudu/gutil/atomicops.h" #include "kudu/gutil/gscoped_ptr.h" -#include "kudu/gutil/port.h" #include "kudu/gutil/ref_counted.h" #include "kudu/gutil/stl_util.h" #include "kudu/rpc/messenger.h" @@ -81,13 +80,13 @@ namespace rpc { class RpcStubTest : public RpcTestBase { public: - virtual void SetUp() OVERRIDE { + void SetUp() override { RpcTestBase::SetUp(); // Use a shorter queue length since some tests below need to start enough // threads to saturate the queue. service_queue_length_ = 10; - StartTestServerWithGeneratedCode(&server_addr_); - client_messenger_ = CreateMessenger("Client"); + ASSERT_OK(StartTestServerWithGeneratedCode(&server_addr_)); + ASSERT_OK(CreateMessenger("Client", &client_messenger_)); } protected: void SendSimpleCall() {
