IMPALA-3581: Change location of minidump folders to log_dir
Currently the default minidump location is /tmp/impala-minidumps, which can be
wiped on
reboot on various distributions. This change moves the default location to
FLAGS_log_dir/minidumps/$daemon. The additional trailing $daemon folder is kept
to prevent
name collisions in case of local test clusters and strangely configured
installations.
For local test clusters the minidumps will be written to
$IMPALA_HOME/logs/cluster/minidumps/{catalogd,impalad,statestored}.
Change-Id: Idecf5a314bfb8b0870e8aa4819c4fb39a107702f
Reviewed-on: http://gerrit.cloudera.org:8080/3171
Reviewed-by: Taras Bobrovytsky <[email protected]>
Tested-by: Internal 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/d16e8321
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/d16e8321
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/d16e8321
Branch: refs/heads/master
Commit: d16e83214a956710eb56243afccb2df89b9e5285
Parents: 6f1fe4e
Author: Lars Volker <[email protected]>
Authored: Mon May 23 16:54:23 2016 +0200
Committer: Tim Armstrong <[email protected]>
Committed: Tue May 31 23:32:11 2016 -0700
----------------------------------------------------------------------
be/src/common/global-flags.cc | 12 ++++----
be/src/util/minidump.cc | 6 ++++
bin/collect_minidumps.py | 35 +++++++++++++++--------
bin/generate_minidump_collection_testdata.py | 19 +++++++-----
tests/custom_cluster/test_breakpad.py | 28 +++++++++++++++---
5 files changed, 72 insertions(+), 28 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d16e8321/be/src/common/global-flags.cc
----------------------------------------------------------------------
diff --git a/be/src/common/global-flags.cc b/be/src/common/global-flags.cc
index 82263e0..88f1daf 100644
--- a/be/src/common/global-flags.cc
+++ b/be/src/common/global-flags.cc
@@ -67,11 +67,13 @@ 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_string(minidump_path, "/tmp/impala-minidumps", "Directory to write
minidump files "
- "to. Minidump files contain crash-related information in a compressed
format and "
- "will only be written when a daemon exits unexpectedly, for example on an
unhandled "
- "exception or signal. Each daemon will create its own subdirectory under
this "
- "directory. Set to empty to disable writing minidump files.");
+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 "
+ "easier to identify a crashing daemon. Minidump files contain
crash-related "
+ "information in a compressed format and will only be written when a daemon
exits "
+ "unexpectedly, for example on an unhandled exception or signal. Set to
empty to "
+ "disable writing minidump files.");
DEFINE_int32(max_minidumps, 9, "Maximum number of minidump files to keep per
daemon. "
"Older files are removed first. Set to 0 to keep all minidump files.");
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d16e8321/be/src/util/minidump.cc
----------------------------------------------------------------------
diff --git a/be/src/util/minidump.cc b/be/src/util/minidump.cc
index 861ed3c..3a7577f 100644
--- a/be/src/util/minidump.cc
+++ b/be/src/util/minidump.cc
@@ -39,6 +39,7 @@ using boost::filesystem::path;
using boost::filesystem::remove;
using boost::system::error_code;
+DECLARE_string(log_dir);
DECLARE_string(minidump_path);
DECLARE_int32(max_minidumps);
DECLARE_int32(minidump_size_limit_hint_kb);
@@ -164,6 +165,11 @@ Status RegisterMinidump(const char* cmd_line_path) {
if (FLAGS_minidump_path.empty()) return Status::OK();
+ if (path(FLAGS_minidump_path).is_relative()) {
+ path log_dir(FLAGS_log_dir);
+ FLAGS_minidump_path = (log_dir / FLAGS_minidump_path).string();
+ }
+
// Add the daemon name to the path where minidumps will be written. This
makes
// identification easier and prevents name collisions between the files.
path daemon = path(cmd_line_path).filename();
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d16e8321/bin/collect_minidumps.py
----------------------------------------------------------------------
diff --git a/bin/collect_minidumps.py b/bin/collect_minidumps.py
index 3f9ba7b..c78e5cb 100755
--- a/bin/collect_minidumps.py
+++ b/bin/collect_minidumps.py
@@ -140,30 +140,41 @@ class FileArchiver(object):
msg = 'Success. Archived {0} out of {1} files in "{2}".'
return status, msg.format(max_num_files, len(self.file_list),
self.source_dir)
-def get_minidump_dir(conf_dir, role_name):
- '''Extracts the minidump directory path for a given role from the
configuration file.'''
+def get_config_parameter_value(conf_dir, role_name, config_parameter_name):
+ '''Extract a single config parameter from the configuration file of a
particular
+ daemon.
+ '''
ROLE_FLAGFILE_MAP = {
'impalad': 'impalad_flags',
'statestored': 'state_store_flags',
'catalogd': 'catalogserver_flags'}
- result = None
+ config_parameter_value = None
try:
file_path = os.path.join(conf_dir, ROLE_FLAGFILE_MAP[role_name])
with open(file_path, 'r') as f:
for line in f:
- m = re.match('-minidump_path=(.*)', line)
+ m = re.match('-{0}=(.*)'.format(config_parameter_name), line)
if m:
- result = m.group(1)
+ config_parameter_value = m.group(1)
except IOError as e:
print >> sys.stderr, 'Error: Unable to open "{0}".'.format(file_path)
sys.exit(1)
- if result:
- result = os.path.join(result, role_name)
- if not os.path.isdir(result):
- sys.exit(0)
- else:
- msg = 'Error: "{0}" does not contain a "-minidump_path" flag.'
- print >> sys.stderr, msg.format(file_path)
+ return config_parameter_value
+
+def get_minidump_dir(conf_dir, role_name):
+ '''Extracts the minidump directory path for a given role from the
configuration file.
+ The directory defaults to 'minidumps', relative paths are prepended with
log_dir, which
+ defaults to '/tmp'.
+ '''
+ minidump_path = get_config_parameter_value(
+ conf_dir, role_name, 'minidump_path') or 'minidumps'
+ if not os.path.isabs(minidump_path):
+ log_dir = get_config_parameter_value(conf_dir, role_name, 'log_dir') or
'/tmp'
+ minidump_path = os.path.join(log_dir, minidump_path)
+ result = os.path.join(minidump_path, role_name)
+ if not os.path.isdir(result):
+ msg = 'Error: minidump directory does not exist.'
+ print >> sys.stderr, msg
sys.exit(1)
return result
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d16e8321/bin/generate_minidump_collection_testdata.py
----------------------------------------------------------------------
diff --git a/bin/generate_minidump_collection_testdata.py
b/bin/generate_minidump_collection_testdata.py
index 862fdb4..b3f142b 100755
--- a/bin/generate_minidump_collection_testdata.py
+++ b/bin/generate_minidump_collection_testdata.py
@@ -33,7 +33,8 @@ from optparse import OptionParser
parser = OptionParser()
parser.add_option('--conf_dir', default='/tmp/impala-conf')
-parser.add_option('--minidump_dir', default='/tmp/minidumps')
+parser.add_option('--log_dir', default='/tmp/impala-logs')
+parser.add_option('--minidump_dir', default='minidumps')
parser.add_option('--start_time', default=None, type='int')
parser.add_option('--end_time', default=None, type='int')
parser.add_option('--duration', default=3600, type='int',
@@ -61,7 +62,8 @@ CONFIG_FILE = '''-beeswax_port=21000
-max_audit_event_log_file_size=5000
-abort_on_failed_audit_event=false
-lineage_event_log_dir=/var/log/impalad/lineage
--minidump_path={0}
+-log_dir={0}
+-minidump_path={1}
-max_lineage_log_file_size=5000
-hostname=vb0204.halxg.cloudera.com
-state_store_host=vb0202.halxg.cloudera.com
@@ -88,7 +90,7 @@ def generate_conf_files():
raise e
for role_name in ROLE_NAMES:
with open(os.path.join(options.conf_dir, ROLE_NAMES[role_name]), 'w') as f:
- f.write(CONFIG_FILE.format(options.minidump_dir))
+ f.write(CONFIG_FILE.format(options.log_dir, options.minidump_dir))
def random_bytes(num):
return ''.join(chr(random.randint(0, 255)) for _ in range(num))
@@ -112,10 +114,13 @@ def generate_minidumps():
else:
start_timestamp = options.start_time
end_timestamp = options.end_time
- if os.path.exists(options.minidump_dir):
- shutil.rmtree(options.minidump_dir)
+ minidump_dir = options.minidump_dir
+ if not os.path.isabs(minidump_dir):
+ minidump_dir = os.path.join(options.log_dir, minidump_dir)
+ if os.path.exists(minidump_dir):
+ shutil.rmtree(minidump_dir)
for role_name in ROLE_NAMES:
- os.makedirs(os.path.join(options.minidump_dir, role_name))
+ os.makedirs(os.path.join(minidump_dir, role_name))
# We want the files to have a high compression ratio and be several
megabytes in size.
# The parameters below should accomplish this.
repeated_token = random_bytes(256)
@@ -127,7 +132,7 @@ def generate_minidumps():
for i in xrange(options.num_minidumps):
write_minidump(common_data,
start_timestamp + interval * i,
- os.path.join(options.minidump_dir, role_name))
+ os.path.join(minidump_dir, role_name))
def main():
generate_conf_files()
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/d16e8321/tests/custom_cluster/test_breakpad.py
----------------------------------------------------------------------
diff --git a/tests/custom_cluster/test_breakpad.py
b/tests/custom_cluster/test_breakpad.py
index 4345ee9..4abd34b 100644
--- a/tests/custom_cluster/test_breakpad.py
+++ b/tests/custom_cluster/test_breakpad.py
@@ -86,12 +86,13 @@ class TestBreakpad(CustomClusterTestSuite):
assert not self.cluster.statestored
assert not self.cluster.catalogd
- def count_minidumps(self, daemon):
- path = os.path.join(self.tmp_dir, daemon)
+ def count_minidumps(self, daemon, base_dir=None):
+ base_dir = base_dir or self.tmp_dir
+ path = os.path.join(base_dir, daemon)
return len(glob.glob("%s/*.dmp" % path))
- def count_all_minidumps(self):
- return sum((self.count_minidumps(daemon) for daemon in DAEMONS))
+ def count_all_minidumps(self, base_dir=None):
+ return sum((self.count_minidumps(daemon, base_dir) for daemon in DAEMONS))
def assert_num_logfile_entries(self, expected_count):
self.assert_impalad_log_contains('INFO', 'Wrote minidump to ',
@@ -113,6 +114,25 @@ class TestBreakpad(CustomClusterTestSuite):
assert self.count_minidumps('catalogd') == 1
@pytest.mark.execute_serially
+ def test_minidump_relative_path(self):
+ """Check that setting 'minidump_path' to a relative value results in
minidump files
+ written to 'log_dir'.
+ """
+ minidump_base_dir = os.path.join(os.environ.get('LOG_DIR', '/tmp'),
'minidumps')
+ shutil.rmtree(minidump_base_dir)
+ # Omitting minidump_path as a parameter to the cluster will choose the
default
+ # configuration, which is a FLAGS_log_dir/minidumps.
+ self.start_cluster_with_args()
+ assert self.count_all_minidumps(minidump_base_dir) == 0
+ cluster_size = len(self.cluster.impalads)
+ self.kill_cluster(SIGSEGV)
+ self.assert_num_logfile_entries(1)
+ assert self.count_minidumps('impalad', minidump_base_dir) == cluster_size
+ assert self.count_minidumps('statestored', minidump_base_dir) == 1
+ assert self.count_minidumps('catalogd', minidump_base_dir) == 1
+ shutil.rmtree(minidump_base_dir)
+
+ @pytest.mark.execute_serially
def test_minidump_cleanup(self):
"""Check that a limited number of minidumps is preserved during startup."""
assert self.count_all_minidumps() == 0
