You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ph...@apache.org on 2018/05/19 03:43:20 UTC
[1/2] impala git commit: Moving default sanitizer options into
init.cc from shell scripts.
Repository: impala
Updated Branches:
refs/heads/master ab75dd12e -> 85ed7ae88
Moving default sanitizer options into init.cc from shell scripts.
When running tests with ASAN, you need to set ASAN_OPTIONS explicitly,
to avoid various failures. In particular, backend tests fail complaining
about memory leaks and tests that use the parquet-reader binary complain
similarly. It turns out that we can shove the default options into our
code base directly, avoiding the need for users to set it explicitly.
I've done the same thing for TSAN and UBSAN.
I've manually checked that these are being read. In the UBSAN case, I
checked both with gdb and with inotifywatch on the suppressions file.
Change-Id: I3cbbd210c67750a48003f336bea1f3e1cb2d9e6b
Reviewed-on: http://gerrit.cloudera.org:8080/10404
Reviewed-by: Jim Apple <jb...@apache.org>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/48625335
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/48625335
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/48625335
Branch: refs/heads/master
Commit: 48625335d220566a1d69e65fb34bfca9a7dc3cff
Parents: ab75dd1
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Mon May 14 20:36:57 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Sat May 19 00:26:53 2018 +0000
----------------------------------------------------------------------
be/CMakeLists.txt | 2 +-
be/src/common/init.cc | 26 ++++++++++++++++++++++++++
bin/run-backend-tests.sh | 4 +---
bin/start-catalogd.sh | 4 ----
bin/start-impalad.sh | 4 ----
bin/start-statestored.sh | 4 ----
6 files changed, 28 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/be/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/be/CMakeLists.txt b/be/CMakeLists.txt
index cc5a597..2ef7b85 100644
--- a/be/CMakeLists.txt
+++ b/be/CMakeLists.txt
@@ -112,7 +112,7 @@ SET(CXX_FLAGS_ADDRESS_SANITIZER
# Set the flags to the undefined behavior sanitizer, also known as "ubsan"
# Turn on sanitizer and debug symbols to get stack traces:
-SET(CXX_FLAGS_UBSAN "${CXX_CLANG_FLAGS} -ggdb3 -fno-omit-frame-pointer -fsanitize=undefined")
+SET(CXX_FLAGS_UBSAN "${CXX_CLANG_FLAGS} -ggdb3 -fno-omit-frame-pointer -fsanitize=undefined -DUNDEFINED_SANITIZER")
# Add flags to enable symbol resolution in the stack traces:
SET(CXX_FLAGS_UBSAN "${CXX_FLAGS_UBSAN} -rtlib=compiler-rt -lgcc_s")
# Ignore a number of noisy errors with too many false positives:
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/be/src/common/init.cc
----------------------------------------------------------------------
diff --git a/be/src/common/init.cc b/be/src/common/init.cc
index 25bfbda..a692a53 100644
--- a/be/src/common/init.cc
+++ b/be/src/common/init.cc
@@ -280,3 +280,29 @@ Status impala::StartMemoryMaintenanceThread() {
return Thread::Create("common", "memory-maintenance-thread",
&MemoryMaintenanceThread, &memory_maintenance_thread);
}
+
+#if defined(ADDRESS_SANITIZER)
+// Default ASAN_OPTIONS. Override by setting environment variable $ASAN_OPTIONS.
+extern "C" const char *__asan_default_options() {
+ // IMPALA-2746: backend tests don't pass with leak sanitizer enabled.
+ return "handle_segv=0 detect_leaks=0 allocator_may_return_null=1";
+}
+#endif
+
+#if defined(THREAD_SANITIZER)
+// Default TSAN_OPTIONS. Override by setting environment variable $TSAN_OPTIONS.
+extern "C" const char *__tsan_default_options() {
+ // Note that backend test should re-configure to halt_on_error=1
+ return "halt_on_error=0 history_size=7";
+}
+#endif
+
+// Default UBSAN_OPTIONS. Override by setting environment variable $UBSAN_OPTIONS.
+#if defined(UNDEFINED_SANITIZER)
+extern "C" const char *__ubsan_default_options() {
+ static const string default_options = Substitute(
+ "print_stacktrace=1 suppressions=$0/bin/ubsan-suppressions.txt",
+ getenv("IMPALA_HOME") == nullptr ? "." : getenv("IMPALA_HOME"));
+ return default_options.c_str();
+}
+#endif
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/bin/run-backend-tests.sh
----------------------------------------------------------------------
diff --git a/bin/run-backend-tests.sh b/bin/run-backend-tests.sh
index d4d3142..3bc84c2 100755
--- a/bin/run-backend-tests.sh
+++ b/bin/run-backend-tests.sh
@@ -37,9 +37,7 @@ cd ${IMPALA_BE_DIR}
cd ..
export CTEST_OUTPUT_ON_FAILURE=1
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
-export UBSAN_OPTIONS="print_stacktrace=1"
-UBSAN_OPTIONS="${UBSAN_OPTIONS} suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
+# Override default TSAN_OPTIONS so that halt_on_error is set.
export TSAN_OPTIONS="halt_on_error=1 history_size=7"
export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"
"${MAKE_CMD:-make}" test ARGS="${BE_TEST_ARGS}"
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/bin/start-catalogd.sh
----------------------------------------------------------------------
diff --git a/bin/start-catalogd.sh b/bin/start-catalogd.sh
index 4ec6846..a8b7e28 100755
--- a/bin/start-catalogd.sh
+++ b/bin/start-catalogd.sh
@@ -70,9 +70,5 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
fi
. ${IMPALA_HOME}/bin/set-classpath.sh
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
-export UBSAN_OPTIONS="print_stacktrace=1"
-UBSAN_OPTIONS="${UBSAN_OPTIONS} suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
-export TSAN_OPTIONS="halt_on_error=0 history_size=7"
export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"
exec ${BINARY_BASE_DIR}/${BUILD_TYPE}/catalog/catalogd ${CATALOGD_ARGS}
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/bin/start-impalad.sh
----------------------------------------------------------------------
diff --git a/bin/start-impalad.sh b/bin/start-impalad.sh
index 76a5f2c..f052fa2 100755
--- a/bin/start-impalad.sh
+++ b/bin/start-impalad.sh
@@ -98,9 +98,5 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
fi
. ${IMPALA_HOME}/bin/set-classpath.sh
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
-export UBSAN_OPTIONS="print_stacktrace=1"
-UBSAN_OPTIONS="${UBSAN_OPTIONS} suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
-export TSAN_OPTIONS="halt_on_error=0 history_size=7"
export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"
exec ${TOOL_PREFIX} ${IMPALA_CMD} ${IMPALAD_ARGS}
http://git-wip-us.apache.org/repos/asf/impala/blob/48625335/bin/start-statestored.sh
----------------------------------------------------------------------
diff --git a/bin/start-statestored.sh b/bin/start-statestored.sh
index 02cf09f..f023810 100755
--- a/bin/start-statestored.sh
+++ b/bin/start-statestored.sh
@@ -59,9 +59,5 @@ if ${CLUSTER_DIR}/admin is_kerberized; then
fi
fi
-export ASAN_OPTIONS="handle_segv=0 detect_leaks=0 allocator_may_return_null=1"
-export UBSAN_OPTIONS="print_stacktrace=1"
-UBSAN_OPTIONS="${UBSAN_OPTIONS} suppressions=${IMPALA_HOME}/bin/ubsan-suppressions.txt"
-export TSAN_OPTIONS="halt_on_error=0 history_size=7"
export PATH="${IMPALA_TOOLCHAIN}/llvm-${IMPALA_LLVM_VERSION}/bin:${PATH}"
exec ${BINARY_BASE_DIR}/${BUILD_TYPE}/statestore/statestored ${STATESTORED_ARGS}
[2/2] impala git commit: IMPALA-6070: Adding ASAN,
--tail to test-with-docker.
Posted by ph...@apache.org.
IMPALA-6070: Adding ASAN, --tail to test-with-docker.
* Adds -ASAN suites to test-with-docker.
* Adds --tail flag, which starts a tail subprocess. This
isn't pretty (there's potential for overlap), but it's a dead simple
way to keep an eye on what's going on.
* Fixes a bug wherein I could call "docker rm <container>" twice
simultaneously, which would make Docker fail the second call,
and then fail the related "docker rmi". It's better to serialize,
and I did that with a simple lock.
Change-Id: I51451cdf1352fc0f9516d729b9a77700488d993f
Reviewed-on: http://gerrit.cloudera.org:8080/10319
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Project: http://git-wip-us.apache.org/repos/asf/impala/repo
Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/85ed7ae8
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/85ed7ae8
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/85ed7ae8
Branch: refs/heads/master
Commit: 85ed7ae88bcec17ffd45b1dd66d07818cb1d55b0
Parents: 4862533
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Thu Apr 26 21:10:37 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Sat May 19 00:37:50 2018 +0000
----------------------------------------------------------------------
docker/entrypoint.sh | 11 +++++++++--
docker/test-with-docker.py | 44 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 48 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/85ed7ae8/docker/entrypoint.sh
----------------------------------------------------------------------
diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh
index 1243e28..37809cd 100755
--- a/docker/entrypoint.sh
+++ b/docker/entrypoint.sh
@@ -183,7 +183,9 @@ function build_impdev() {
# Note that IMPALA-6494 prevents us from using shared library builds,
# which are smaller and thereby speed things up. We use "-notests"
# to avoid building backend tests, which are sizable, and
- # can be built when executing those tests.
+ # can be built when executing those tests. We use "-noclean" to
+ # avoid deleting the log for this invocation which is in logs/,
+ # and, this is a first build anyway.
./buildall.sh -noclean -format -testdata -notests
# Dump current memory usage to logs, before shutting things down.
@@ -257,8 +259,13 @@ function test_suite() {
boot_container
impala_environment
+ if [[ ${REBUILD_ASAN:-false} = true ]]; then
+ # Note: we're not redoing data loading.
+ SKIP_TOOLCHAIN_BOOTSTRAP=true ./buildall.sh -noclean -notests -asan
+ fi
+
# BE tests don't require the minicluster, so we can run them directly.
- if [[ $1 = BE_TEST ]]; then
+ if [[ $1 = BE_TEST* ]]; then
make -j$(nproc) --load-average=$(nproc) be-test be-benchmarks
if ! bin/run-backend-tests.sh; then
echo "Tests $1 failed!"
http://git-wip-us.apache.org/repos/asf/impala/blob/85ed7ae8/docker/test-with-docker.py
----------------------------------------------------------------------
diff --git a/docker/test-with-docker.py b/docker/test-with-docker.py
index fa230e8..1922c57 100755
--- a/docker/test-with-docker.py
+++ b/docker/test-with-docker.py
@@ -113,6 +113,7 @@ import re
import subprocess
import sys
import tempfile
+import threading
import time
if __name__ == '__main__' and __package__ is None:
@@ -178,6 +179,8 @@ def main():
parser.add_argument('--ccache-dir', metavar='DIR',
help="CCache directory to use",
default=os.path.expanduser("~/.ccache"))
+ parser.add_argument('--tail', action="store_true",
+ help="Run tail on all container log files.")
parser.add_argument('--test', action="store_true")
args = parser.parse_args()
@@ -193,7 +196,8 @@ def main():
cleanup_image=args.cleanup_image, ccache_dir=args.ccache_dir, test_mode=args.test,
parallel_test_concurrency=args.parallel_test_concurrency,
suite_concurrency=args.suite_concurrency,
- impalad_mem_limit_bytes=args.impalad_mem_limit_bytes)
+ impalad_mem_limit_bytes=args.impalad_mem_limit_bytes,
+ tail=args.tail)
fh = logging.FileHandler(os.path.join(_make_dir_if_not_exist(t.log_dir), "log.txt"))
fh.setFormatter(logging.Formatter(LOG_FORMAT))
@@ -296,6 +300,12 @@ class Suite(object):
r.timeout_minutes = 240
return r
+ def asan(self):
+ """Returns an ASAN copy of this suite."""
+ r = self.copy(self.name + "_ASAN", REBUILD_ASAN="true")
+ r.timeout_minutes = self.timeout_minutes * 2.0 + 10
+ return r
+
def sharded(self, shards):
"""Returns a list of sharded copies of the list.
@@ -311,6 +321,7 @@ class Suite(object):
return ret
# Definitions of all known suites:
+be_test = Suite("BE_TEST")
ee_test_serial = Suite("EE_TEST_SERIAL", EE_TEST="true",
RUN_TESTS_ARGS="--skip-parallel --skip-stress")
ee_test_serial.shard_at_concurrency = 4
@@ -339,6 +350,13 @@ OTHER_SUITES = [
ee_test_parallel_exhaustive,
ee_test_serial_exhaustive,
cluster_test_exhaustive,
+
+ # ASAN
+ be_test.asan(),
+ cluster_test.asan(),
+ ee_test_parallel.asan(),
+ ee_test_serial.asan(),
+
Suite("RAT_CHECK"),
# These are used for testing this script
Suite("NOOP"),
@@ -388,6 +406,9 @@ class Container(object):
self.end = None
self.removed = False
+ # Protects multiple calls to "docker rm <self.id>"
+ self.lock = threading.Lock()
+
# Updated by Timeline class
self.total_user_cpu = -1
self.total_system_cpu = -1
@@ -409,7 +430,7 @@ class TestWithDocker(object):
def __init__(self, build_image, suite_names, name, cleanup_containers,
cleanup_image, ccache_dir, test_mode,
suite_concurrency, parallel_test_concurrency,
- impalad_mem_limit_bytes):
+ impalad_mem_limit_bytes, tail):
self.build_image = build_image
self.name = name
self.containers = []
@@ -442,6 +463,7 @@ class TestWithDocker(object):
self.suite_concurrency = suite_concurrency
self.parallel_test_concurrency = parallel_test_concurrency
self.impalad_mem_limit_bytes = impalad_mem_limit_bytes
+ self.tail = tail
# Map suites back into objects; we ignore case for this mapping.
suites = []
@@ -518,8 +540,13 @@ class TestWithDocker(object):
run through annotate.py to add timestamps and saved into the container's log file.
"""
container.running = True
+ tailer = None
+
+ with open(container.logfile, "aw") as log_output:
+ if self.tail:
+ tailer = subprocess.Popen(
+ ["tail", "-f", "--pid", str(os.getpid()), "-v", container.logfile])
- with file(container.logfile, "w") as log_output:
container.start = time.time()
# Sets up a "docker start ... | annotate.py > logfile" pipeline using
# subprocess.
@@ -542,6 +569,8 @@ class TestWithDocker(object):
container.exitcode = ret
container.running = False
container.end = time.time()
+ if tailer:
+ tailer.kill()
return ret == 0
@staticmethod
@@ -555,8 +584,13 @@ class TestWithDocker(object):
@staticmethod
def _rm_container(container):
"""Removes container."""
- if not container.removed:
- _call(["docker", "rm", container.id], check=False)
+ # We can have multiple threads trying to call "docker rm" on a container.
+ # Docker will fail one of those with "already running", but we actually
+ # want to block until it's removed. Using a lock to serialize the "docker # rm"
+ # calls handles that.
+ with container.lock:
+ if not container.removed:
+ _call(["docker", "rm", container.id], check=False)
container.removed = True
def _create_build_image(self):