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/22 03:33:58 UTC

[3/4] impala git commit: IMPALA-6070: Adding ASAN, --tail to test-with-docker.

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/9116423a
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/9116423a
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/9116423a

Branch: refs/heads/2.x
Commit: 9116423a76ca1a11fdd440d20f6fd14700bf9df9
Parents: 2caec90
Author: Philip Zeyliger <ph...@cloudera.com>
Authored: Thu Apr 26 21:10:37 2018 -0700
Committer: Impala Public Jenkins <im...@gerrit.cloudera.org>
Committed: Mon May 21 20:10:17 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/9116423a/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/9116423a/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):