You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2016/09/16 16:04:15 UTC
incubator-impala git commit: IMPALA-4111: backend death tests should
not produce minidumps
Repository: incubator-impala
Updated Branches:
refs/heads/master 3904d3091 -> e3abf84cc
IMPALA-4111: backend death tests should not produce minidumps
Move the existing core dump disabler into a shared utility header and
also disable minidumps too. Add a macro to simplify use of the disabler.
Change-Id: I33037f4f77c8188fc2ec46a77083a0a64f6ea404
Reviewed-on: http://gerrit.cloudera.org:8080/4353
Reviewed-by: Tim Armstrong <ta...@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/e3abf84c
Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/e3abf84c
Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/e3abf84c
Branch: refs/heads/master
Commit: e3abf84cce0dcc5fc5817688b9500b652550019d
Parents: 3904d30
Author: Tim Armstrong <ta...@cloudera.com>
Authored: Fri Sep 9 11:16:58 2016 -0700
Committer: Internal Jenkins <cl...@gerrit.cloudera.org>
Committed: Fri Sep 16 07:37:36 2016 +0000
----------------------------------------------------------------------
be/src/testutil/death-test-util.h | 59 ++++++++++++++++++++++++++++++++++
be/src/util/minidump.cc | 19 +++++++++--
be/src/util/minidump.h | 4 +++
be/src/util/promise-test.cc | 25 ++------------
4 files changed, 83 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3abf84c/be/src/testutil/death-test-util.h
----------------------------------------------------------------------
diff --git a/be/src/testutil/death-test-util.h b/be/src/testutil/death-test-util.h
new file mode 100644
index 0000000..d84b374
--- /dev/null
+++ b/be/src/testutil/death-test-util.h
@@ -0,0 +1,59 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#ifndef IMPALA_TESTUTIL_DEATH_TEST_UTIL_H_
+#define IMPALA_TESTUTIL_DEATH_TEST_UTIL_H_
+
+#include <sys/resource.h>
+
+#include "util/minidump.h"
+
+// Wrapper around gtest's ASSERT_DEBUG_DEATH that prevents coredumps and minidumps
+// being generated as the result of the death test.
+#define IMPALA_ASSERT_DEBUG_DEATH(fn, msg) \
+ do { \
+ ScopedCoredumpDisabler disable_coredumps; \
+ ASSERT_DEBUG_DEATH(fn, msg); \
+ } while (false);
+
+namespace impala {
+
+/// Utility class that disables coredumps and minidumps within a given scope.
+struct ScopedCoredumpDisabler {
+ public:
+ ScopedCoredumpDisabler() {
+ getrlimit(RLIMIT_CORE, &limit_before_);
+ rlimit limit;
+ limit.rlim_cur = limit.rlim_max = 0;
+ setrlimit(RLIMIT_CORE, &limit);
+
+ minidumps_enabled_before_ = EnableMinidumpsForTest(false);
+ }
+
+ ~ScopedCoredumpDisabler() {
+ setrlimit(RLIMIT_CORE, &limit_before_);
+
+ EnableMinidumpsForTest(minidumps_enabled_before_);
+ }
+
+ private:
+ rlimit limit_before_;
+ bool minidumps_enabled_before_;
+};
+}
+
+#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3abf84c/be/src/util/minidump.cc
----------------------------------------------------------------------
diff --git a/be/src/util/minidump.cc b/be/src/util/minidump.cc
index 23c7564..4567c48 100644
--- a/be/src/util/minidump.cc
+++ b/be/src/util/minidump.cc
@@ -56,6 +56,15 @@ namespace impala {
/// files during process crashes, but also can be used to write minidumps directly.
static google_breakpad::ExceptionHandler* minidump_exception_handler = NULL;
+/// Test helper. True if minidumps should be enabled.
+static bool minidumps_enabled = true;
+
+/// Called by the exception handler before minidump is produced. Minidump is only written
+/// if this returns true.
+static bool FilterCallback(void* context) {
+ return minidumps_enabled;
+}
+
/// Callback for breakpad. It is called by breakpad whenever a minidump file has been
/// written and should not be called directly. It logs the event before breakpad crashes
/// the process. Due to the process being in a failed state we write to stdout/stderr and
@@ -225,8 +234,8 @@ Status RegisterMinidump(const char* cmd_line_path) {
// Intentionally leaked. We want this to have the lifetime of the process.
DCHECK(minidump_exception_handler == NULL);
- minidump_exception_handler =
- new google_breakpad::ExceptionHandler(desc, NULL, DumpCallback, NULL, true, -1);
+ minidump_exception_handler = new google_breakpad::ExceptionHandler(
+ desc, FilterCallback, DumpCallback, NULL, true, -1);
// Setup signal handler for SIGUSR1.
SetupSignalHandler();
@@ -234,4 +243,10 @@ Status RegisterMinidump(const char* cmd_line_path) {
return Status::OK();
}
+bool EnableMinidumpsForTest(bool enabled) {
+ bool old_value = minidumps_enabled;
+ minidumps_enabled = enabled;
+ return old_value;
+}
+
} // end ns impala
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3abf84c/be/src/util/minidump.h
----------------------------------------------------------------------
diff --git a/be/src/util/minidump.h b/be/src/util/minidump.h
index 7750eaf..b2c7160 100644
--- a/be/src/util/minidump.h
+++ b/be/src/util/minidump.h
@@ -26,6 +26,10 @@ namespace impala {
/// See https://chromium.googlesource.com/breakpad/breakpad/ for more details.
Status RegisterMinidump(const char* cmd_line_path);
+/// Test helper to temporarily enable or disable minidumps, for example during death
+/// tests that deliberately trigger DCHECKs. Returns true if minidumps were previously
+/// enabled or false otherwise.
+bool EnableMinidumpsForTest(bool enabled);
}
#endif
http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/e3abf84c/be/src/util/promise-test.cc
----------------------------------------------------------------------
diff --git a/be/src/util/promise-test.cc b/be/src/util/promise-test.cc
index 1ce77c0..2906c27 100644
--- a/be/src/util/promise-test.cc
+++ b/be/src/util/promise-test.cc
@@ -16,10 +16,10 @@
// under the License.
#include <boost/thread.hpp>
-#include <sys/resource.h>
#include "runtime/timestamp-value.h"
#include "testutil/gtest-util.h"
+#include "testutil/death-test-util.h"
#include "util/promise.h"
#include "util/time.h"
@@ -27,23 +27,6 @@
namespace impala {
-struct ScopedLimitResetter {
- public:
- ScopedLimitResetter() {
- getrlimit(RLIMIT_CORE, &limit_before_);
- rlimit limit;
- limit.rlim_cur = limit.rlim_max = 0;
- setrlimit(RLIMIT_CORE, &limit);
- }
-
- ~ScopedLimitResetter() {
- setrlimit(RLIMIT_CORE, &limit_before_);
- }
-
- private:
- rlimit limit_before_;
-};
-
void RunThread(Promise<int64_t>* promise) {
promise->Set(100);
}
@@ -75,16 +58,14 @@ TEST(PromiseTest, TimeoutTest) {
}
TEST(PromiseDeathTest, RepeatedSetTest) {
- // This test intentionally causes a crash. Don't generate core files for it.
- ScopedLimitResetter resetter;
-
// Hint to gtest that only one thread is being used here. Multiple threads are unsafe
// for 'death' tests, see
// https://code.google.com/p/googletest/wiki/AdvancedGuide#Death_Tests for more detail
::testing::FLAGS_gtest_death_test_style = "threadsafe";
Promise<int64_t> promise;
promise.Set(100);
- ASSERT_DEBUG_DEATH(promise.Set(150), "Called Set\\(\\.\\.\\) twice on the same Promise");
+ IMPALA_ASSERT_DEBUG_DEATH(
+ promise.Set(150), "Called Set\\(\\.\\.\\) twice on the same Promise");
}
}