Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/11813 )
Change subject: IMPALA-5031: Jenkins jobs should fail when UBSAN fails ...................................................................... Patch Set 1: (2 comments) The finalize.sh change looks great. I mentioned some concerns about the killer process looking through a lot of logs every second. We could maybe tighten it, or maybe it doesn't matter. Do you find the tests run in a similar amount of time with and without the killer? http://gerrit.cloudera.org:8080/#/c/11813/1/bin/jenkins/all-tests.sh File bin/jenkins/all-tests.sh: http://gerrit.cloudera.org:8080/#/c/11813/1/bin/jenkins/all-tests.sh@45 PS1, Line 45: while ! grep -rI ": runtime error: " "${IMPALA_HOME}/logs" There could be a log of logs here, no? We're actually only interested, I think, in impalad/catalogd/statestored logs, which could be more tightly matched. You could also investigate https://reviews.llvm.org/D9006 which suggests there's a "log_path" option for UBSAN (but I don't know if we have it) or use -fno-sanitize-recover... (see https://releases.llvm.org/6.0.0/tools/clang/docs/UndefinedBehaviorSanitizer.html#usage) to have impalad die when these are encountered, which tends to fail the remaining tests. http://gerrit.cloudera.org:8080/#/c/11813/1/bin/jenkins/all-tests.sh@49 PS1, Line 49: kill -9 $PID_TO_KILL Could you add a logging echo before the kill? It may catch someone by surprise. -- To view, visit http://gerrit.cloudera.org:8080/11813 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I783243458ac2765a97a1dd7dd40d458cc2e1d80b Gerrit-Change-Number: 11813 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Sat, 03 Nov 2018 21:04:06 +0000 Gerrit-HasComments: Yes
