Jim Apple has posted comments on this change. Change subject: IMPALA-5902: add ThreadSanitizer build ......................................................................
Patch Set 1: (8 comments) Nice, thanks for taking this on. Can you file a ticket to add this to https://jenkins.impala.io/job/all-build-options/ when this is done? http://gerrit.cloudera.org:8080/#/c/7977/1/be/src/common/init.cc File be/src/common/init.cc: Line 140 No longer needed? Why not? Line 199: #ifndef THREAD_SANITIZER Why can't we run this with TSAN? Should the #ifndef go inside that function itself? http://gerrit.cloudera.org:8080/#/c/7977/1/be/src/runtime/query-exec-mgr.cc File be/src/runtime/query-exec-mgr.cc: Line 115: // tcmalloc and address sanitizer cannot be used together nit: "or thread" http://gerrit.cloudera.org:8080/#/c/7977/1/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: Line 139: ss << "Memory tracking is not available with address sanitizer builds."; nit: "or thread" http://gerrit.cloudera.org:8080/#/c/7977/1/be/src/util/memory-metrics.cc File be/src/util/memory-metrics.cc: PS1, Line 86: asan naming? http://gerrit.cloudera.org:8080/#/c/7977/1/be/src/util/memory-metrics.h File be/src/util/memory-metrics.h: Line 136: /// Alternative to TCMallocMetric if we're running under Address Sanitizer, which Naming? http://gerrit.cloudera.org:8080/#/c/7977/1/bin/start-impalad.sh File bin/start-impalad.sh: PS1, Line 104: 7 How did you pick 7? http://gerrit.cloudera.org:8080/#/c/7977/1/tests/common/environ.py File tests/common/environ.py: Line 135: either compiled with code coverage enabled or a sanitizer. except ubsan -- To view, visit http://gerrit.cloudera.org:8080/7977 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I22f8faeefa5e157279c5973fe28bc573b7606d50 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Jim Apple <jbapple-imp...@apache.org> Gerrit-HasComments: Yes
