This is an automated email from the ASF dual-hosted git repository. awong pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 0cded868fd3d8ff088cd290dccdd9d90fdc0817c Author: Andrew Wong <[email protected]> AuthorDate: Fri Jan 17 18:46:58 2020 -0800 mini-cluster: disallow restarting daemons from other threads If we restart an external daemon from a separate thread, the daemon can be killed silently and without warning when the thread is reaped. For instance, the following would fail without logging any information about the tserver dying: TEST_F(ExternalMiniClusterITestBase, TestRestartFromThread) { ExternalMiniClusterOptions opts; opts.num_tablet_servers = 1; NO_FATALS(StartClusterWithOpts(std::move(opts))); thread t([&] { auto* ts = cluster_->tablet_server(0); ts->Shutdown(); return ts->Restart(); }); t.join(); SleepFor(MonoDelta::FromSeconds(1)); ASSERT_TRUE(cluster_->tablet_server(0)->IsProcessAlive()); } I didn't add a death test, since death tests themselves don't work well in multithreaded contexts. Change-Id: I184a01be3e1ac7f60a8b3aedab176dc9138033e0 Reviewed-on: http://gerrit.cloudera.org:8080/15069 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> --- src/kudu/mini-cluster/external_mini_cluster.cc | 9 +++++++-- src/kudu/mini-cluster/external_mini_cluster.h | 7 +++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc index 0120dc9..54eae8f 100644 --- a/src/kudu/mini-cluster/external_mini_cluster.cc +++ b/src/kudu/mini-cluster/external_mini_cluster.cc @@ -25,11 +25,11 @@ #include <iterator> #include <memory> #include <string> +#include <thread> #include <unordered_set> #include <utility> #include <gflags/gflags.h> -#include <gflags/gflags_declare.h> #include <gtest/gtest.h> #include "kudu/client/client.h" @@ -939,7 +939,8 @@ string ExternalMiniCluster::UuidForTS(int ts_idx) const { //------------------------------------------------------------ ExternalDaemon::ExternalDaemon(ExternalDaemonOptions opts) - : opts_(std::move(opts)) { + : opts_(std::move(opts)), + parent_tid_(std::this_thread::get_id()) { CHECK(rpc_bind_address().Initialized()); } @@ -968,6 +969,10 @@ Status ExternalDaemon::EnableKerberos(MiniKdc* kdc, const string& bind_host) { Status ExternalDaemon::StartProcess(const vector<string>& user_flags) { CHECK(!process_); + const auto this_tid = std::this_thread::get_id(); + CHECK_EQ(parent_tid_, this_tid) + << "Process being started from thread " << this_tid << " which is different" + << " from the instantiating thread " << parent_tid_; vector<string> argv; diff --git a/src/kudu/mini-cluster/external_mini_cluster.h b/src/kudu/mini-cluster/external_mini_cluster.h index 428ec3a..5b7b81d 100644 --- a/src/kudu/mini-cluster/external_mini_cluster.h +++ b/src/kudu/mini-cluster/external_mini_cluster.h @@ -25,6 +25,8 @@ #include <memory> #include <ostream> #include <string> +#include <thread> +#include <utility> #include <vector> #include <boost/optional/optional.hpp> @@ -610,6 +612,11 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> { HostPort bound_rpc_; HostPort bound_http_; + // ID of the thread that is spawning the child processes. This should not + // change across restarts of the daemon, as forking from different threads + // may yield behavior like daemons being killed when the new thread exits. + const std::thread::id parent_tid_; + DISALLOW_COPY_AND_ASSIGN(ExternalDaemon); };
