IMPALA-5616: Add --enable_minidumps startup flag If set to 'false', this flag will disable registration of the Breakpad signal handlers during startup. The default value is 'true'. This does not affect the ability to disable the handlers by specifying an empty value for --minidump_path.
This change adds a test to test_breakpad.py. Change-Id: Ie2039b9140e1c281810b27b76140e2105198bc37 Reviewed-on: http://gerrit.cloudera.org:8080/7541 Reviewed-by: Lars Volker <[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/344c26aa Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/344c26aa Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/344c26aa Branch: refs/heads/master Commit: 344c26aa29cd5f0ec7711eb873cbfa2a4151942a Parents: 1f89d2d Author: Lars Volker <[email protected]> Authored: Mon Jul 31 13:08:39 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Wed Aug 2 01:32:06 2017 +0000 ---------------------------------------------------------------------- be/src/common/global-flags.cc | 3 +++ be/src/util/minidump.cc | 2 ++ tests/custom_cluster/test_breakpad.py | 13 +++++++++---- 3 files changed, 14 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/344c26aa/be/src/common/global-flags.cc ---------------------------------------------------------------------- diff --git a/be/src/common/global-flags.cc b/be/src/common/global-flags.cc index 1a36fad..0a6ca28 100644 --- a/be/src/common/global-flags.cc +++ b/be/src/common/global-flags.cc @@ -70,6 +70,9 @@ DEFINE_string(redaction_rules_file, "", "Absolute path to sensitive data redacti "Web UI and audit records. Query results will not be affected. Refer to the " "documentation for the rule file format."); +DEFINE_bool(enable_minidumps, true, "Whether to enable minidump generation upon process " + "crash or SIGUSR1."); + DEFINE_string(minidump_path, "minidumps", "Directory to write minidump files to. This " "can be either an absolute path or a path relative to log_dir. Each daemon will " "create an additional sub-directory to prevent naming conflicts and to make it " http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/344c26aa/be/src/util/minidump.cc ---------------------------------------------------------------------- diff --git a/be/src/util/minidump.cc b/be/src/util/minidump.cc index b87ea90..78c48df 100644 --- a/be/src/util/minidump.cc +++ b/be/src/util/minidump.cc @@ -44,6 +44,7 @@ using boost::filesystem::path; using boost::filesystem::remove; DECLARE_string(log_dir); +DECLARE_bool(enable_minidumps); DECLARE_string(minidump_path); DECLARE_int32(max_minidumps); DECLARE_int32(minidump_size_limit_hint_kb); @@ -193,6 +194,7 @@ Status RegisterMinidump(const char* cmd_line_path) { DCHECK(!registered); registered = true; + if (!FLAGS_enable_minidumps) return Status::OK(); if (FLAGS_minidump_path.empty()) return Status::OK(); if (path(FLAGS_minidump_path).is_relative()) { http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/344c26aa/tests/custom_cluster/test_breakpad.py ---------------------------------------------------------------------- diff --git a/tests/custom_cluster/test_breakpad.py b/tests/custom_cluster/test_breakpad.py index 120e9b9..8300634 100644 --- a/tests/custom_cluster/test_breakpad.py +++ b/tests/custom_cluster/test_breakpad.py @@ -83,9 +83,6 @@ class TestBreakpad(CustomClusterTestSuite): self.start_cluster_with_args(minidump_path=self.tmp_dir, max_minidumps=self.MAX_MINIDUMPS) - def start_cluster_without_minidumps(self): - self.start_cluster_with_args(minidump_path='', max_minidumps=self.MAX_MINIDUMPS) - def kill_cluster(self, signal): self.cluster.refresh() processes = self.cluster.impalads + [self.cluster.catalogd, self.cluster.statestored] @@ -216,10 +213,18 @@ class TestBreakpad(CustomClusterTestSuite): @pytest.mark.execute_serially def test_disable_minidumps(self): + """Check that setting enable_minidumps to false disables minidump creation.""" + assert self.count_all_minidumps() == 0 + self.start_cluster_with_args(enable_minidumps=False) + self.kill_cluster(SIGSEGV) + self.assert_num_logfile_entries(0) + + @pytest.mark.execute_serially + def test_empty_minidump_path_disables_breakpad(self): """Check that setting the minidump_path to an empty value disables minidump creation. """ assert self.count_all_minidumps() == 0 - self.start_cluster_without_minidumps() + self.start_cluster_with_args(minidump_path='', max_minidumps=self.MAX_MINIDUMPS) self.kill_cluster(SIGSEGV) self.assert_num_logfile_entries(0)
