impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tarmstr...@apache.org
Subject [10/23] incubator-impala git commit: IMPALA-3581: Change location of minidump folders to log_dir
Date Wed, 01 Jun 2016 06:32:29 GMT
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 <tbobrovytsky@cloudera.com>
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 <lv@cloudera.com>
Authored: Mon May 23 16:54:23 2016 +0200
Committer: Tim Armstrong <tarmstrong@cloudera.com>
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


Mime
View raw message