Repository: incubator-impala Updated Branches: refs/heads/master 5797f8596 -> 79205bb33
IMPALA-5756: start memory maintenance thread after metric creation Daemons were sometimes crashing on startup if the memory maintenance thread started running before the memory metrics were created. This changes the startup sequence so that the thread is started after the metrics are registered. Change-Id: Ibb7337a68547b4d660f3100b04df2c20befb044c Reviewed-on: http://gerrit.cloudera.org:8080/7573 Reviewed-by: Henry Robinson <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/79205bb3 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/79205bb3 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/79205bb3 Branch: refs/heads/master Commit: 79205bb3307a48cdfe62a8a5ccd4c78b93d2ecef Parents: 5797f85 Author: Tim Armstrong <[email protected]> Authored: Wed Aug 2 21:34:35 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Thu Aug 3 17:28:32 2017 +0000 ---------------------------------------------------------------------- be/src/catalog/catalogd-main.cc | 1 + be/src/common/init.cc | 14 ++++++-------- be/src/common/init.h | 4 ++++ be/src/service/impalad-main.cc | 1 + be/src/statestore/statestored-main.cc | 1 + 5 files changed, 13 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/79205bb3/be/src/catalog/catalogd-main.cc ---------------------------------------------------------------------- diff --git a/be/src/catalog/catalogd-main.cc b/be/src/catalog/catalogd-main.cc index 102b66b..ae1cdf6 100644 --- a/be/src/catalog/catalogd-main.cc +++ b/be/src/catalog/catalogd-main.cc @@ -72,6 +72,7 @@ int CatalogdMain(int argc, char** argv) { metrics->Init(FLAGS_enable_webserver ? webserver.get() : nullptr); ABORT_IF_ERROR(RegisterMemoryMetrics(metrics.get(), true, nullptr, nullptr)); + StartMemoryMaintenanceThread(); StartThreadInstrumentation(metrics.get(), webserver.get(), true); InitRpcEventTracing(webserver.get()); http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/79205bb3/be/src/common/init.cc ---------------------------------------------------------------------- diff --git a/be/src/common/init.cc b/be/src/common/init.cc index 797df39..523796a 100644 --- a/be/src/common/init.cc +++ b/be/src/common/init.cc @@ -205,14 +205,6 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, log_maintenance_thread.reset( new Thread("common", "log-maintenance-thread", &LogMaintenanceThread)); - // Memory maintenance isn't necessary for frontend tests, and it's undesirable - // to asynchronously free memory in backend tests that are testing memory - // management behaviour. - if (!impala::TestInfo::is_test()) { - memory_maintenance_thread.reset( - new Thread("common", "memory-maintenance-thread", &MemoryMaintenanceThread)); - } - pause_monitor.reset(new Thread("common", "pause-monitor", &PauseMonitorLoop)); LOG(INFO) << impala::GetVersionString(); @@ -253,3 +245,9 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, } #endif } + +void impala::StartMemoryMaintenanceThread() { + DCHECK(AggregateMemoryMetrics::NUM_MAPS != nullptr) << "Mem metrics not registered."; + memory_maintenance_thread.reset( + new Thread("common", "memory-maintenance-thread", &MemoryMaintenanceThread)); +} http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/79205bb3/be/src/common/init.h ---------------------------------------------------------------------- diff --git a/be/src/common/init.h b/be/src/common/init.h index 941d879..b1de6d8 100644 --- a/be/src/common/init.h +++ b/be/src/common/init.h @@ -30,6 +30,10 @@ namespace impala { void InitCommonRuntime(int argc, char** argv, bool init_jvm, TestInfo::Mode m = TestInfo::NON_TEST); +/// Starts background memory maintenance thread. Must be called after +/// RegisterMemoryMetrics(). This thread is needed for daemons to free memory and +/// refresh metrics but is not needed for standalone tests. +void StartMemoryMaintenanceThread(); } #endif http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/79205bb3/be/src/service/impalad-main.cc ---------------------------------------------------------------------- diff --git a/be/src/service/impalad-main.cc b/be/src/service/impalad-main.cc index 1f0dd81..32fbcc8 100644 --- a/be/src/service/impalad-main.cc +++ b/be/src/service/impalad-main.cc @@ -96,6 +96,7 @@ int ImpaladMain(int argc, char** argv) { ShutdownLogging(); exit(1); } + StartMemoryMaintenanceThread(); // Memory metrics are created in StartServices(). DCHECK(exec_env.process_mem_tracker() != nullptr) << "ExecEnv::StartServices() must be called before starting RPC services"; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/79205bb3/be/src/statestore/statestored-main.cc ---------------------------------------------------------------------- diff --git a/be/src/statestore/statestored-main.cc b/be/src/statestore/statestored-main.cc index 53286f5..40e2a73 100644 --- a/be/src/statestore/statestored-main.cc +++ b/be/src/statestore/statestored-main.cc @@ -66,6 +66,7 @@ int StatestoredMain(int argc, char** argv) { metrics->Init(FLAGS_enable_webserver ? webserver.get() : nullptr); ABORT_IF_ERROR(RegisterMemoryMetrics(metrics.get(), false, nullptr, nullptr)); + StartMemoryMaintenanceThread(); StartThreadInstrumentation(metrics.get(), webserver.get(), false); InitRpcEventTracing(webserver.get()); // TODO: Add a 'common metrics' method to add standard metrics to
