Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/12086 )
Change subject: Added timeout to run-all-tests ...................................................................... Patch Set 1: (5 comments) Thanks! This is just below my threshold for asking you to write it in python, but I think it'd be shorter there if you wanted to go that route. If you wanted to avoid writing the loop to wait for a process to finish, you could do: timeout 60 bash -c 'while true; do ps 15796 &> /dev/null || exit 0; sleep 1; done' (Making the pid there a variable is an exercise.) Once that timeout is done, you have to figure out whether or not you need to do the gdb business. http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh File bin/run-all-tests-timeout-check.sh: http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@8 PS1, Line 8: echo "Expected timeout value as an argument" add exit 1? http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@22 PS1, Line 22: SECONDS=0 This confused me! SECONDS is a built-in, as you know, but someone who doesn't know, (or even somebody like me who knew and was about to tell you) is super confused because you're treating it like a normal variable not not resetting it. Anyway, I don't think you need "SECONDS=0" here at all, but you need a comment that "$SECONDS is a bash built-in that counts seconds since bash started. http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@24 PS1, Line 24: sleep 60 I think you can reasonably just sleep 1 here and avoid the complication of repeating yourself in lines 28-30. http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@25 PS1, Line 25: [[ -z "$(ps -p $PPID -o pid=)" ]] && exit This is just: ps $PPID &> /dev/null || exit i.e., you can use the exit code of ps to check for a process. http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@45 PS1, Line 45: gdb -ex "set pagination 0" -ex "thread apply all bt" --batch -p $pid > \ I'm surprised we need "set pagination" in addition to --batch. -- To view, visit http://gerrit.cloudera.org:8080/12086 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iefd3e16a973709f27aeffc7a15bcab8fcdbb349b Gerrit-Change-Number: 12086 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Comment-Date: Fri, 14 Dec 2018 18:04:19 +0000 Gerrit-HasComments: Yes
