You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by mi...@apache.org on 2017/08/03 17:29:38 UTC
incubator-impala git commit: IMPALA-5756: start memory maintenance
thread after metric creation
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 <he...@cloudera.com>
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 <ta...@cloudera.com>
Authored: Wed Aug 2 21:34:35 2017 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
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