Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10319 )
Change subject: IMPALA-6070: Adding ASAN, --tail to test-with-docker. ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/10319/1/docker/entrypoint.sh File docker/entrypoint.sh: http://gerrit.cloudera.org:8080/#/c/10319/1/docker/entrypoint.sh@262 PS1, Line 262: noclean > shouldn't we clean up before building with asan? In practice, when you do buildall, it calls cmake, and the Makefiles are different, so everything that needs to gets rebuilt. I tried this without "-noclean" and it failed starting up the minicluster. I think clean destroys parts of the minicluster, though I couldn't tell you exactly which parts right now. (At the very least the config files.) http://gerrit.cloudera.org:8080/#/c/10319/1/docker/test-with-docker.py File docker/test-with-docker.py: http://gerrit.cloudera.org:8080/#/c/10319/1/docker/test-with-docker.py@409 PS1, Line 409: self.lock = threading.Lock() > Can you add a comment explaining what this protects? Done http://gerrit.cloudera.org:8080/#/c/10319/1/docker/test-with-docker.py@565 PS1, Line 565: def _rm_container(container): > You could also add a synchronized decorator and use it to synchronize the m Seems overkill / not quite correct. There may be multiple containers getting cleaned up; we just don't want to clean up the same one simultaneously in two threads. http://gerrit.cloudera.org:8080/#/c/10319/1/docker/test-with-docker.py@573 PS1, Line 573: with container.lock: > Wouldn't it be safer to grab the lock before the if above? That way you'd o Good point; done. -- To view, visit http://gerrit.cloudera.org:8080/10319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51451cdf1352fc0f9516d729b9a77700488d993f Gerrit-Change-Number: 10319 Gerrit-PatchSet: 1 Gerrit-Owner: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Comment-Date: Tue, 08 May 2018 16:15:57 +0000 Gerrit-HasComments: Yes
