Bikramjeet Vig has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12086 )

Change subject: Added timeout to run-all-tests
......................................................................


Patch Set 1:

(7 comments)

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?
Done


http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@22
PS1, Line 22: SECONDS=0
> This confused me!
Done


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
Done


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:
Done


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.
Done


http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests-timeout-check.sh@52
PS1, Line 52:  "Test run timed out"
> Explain where to find information? ($IMPALA_TIMEOUT_LOGS_DIR)
Done


http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:

http://gerrit.cloudera.org:8080/#/c/12086/1/bin/run-all-tests.sh@34
PS1, Line 34: : ${TIMEOUT_FOR_RUN_ALL_TESTS_MINS:=0}
> Any reason not to default this to something high, like 20 hours?
no particular reason, just did not want to modify the current behavior. will 
change the default to 20hrs as you suggested.



--
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: 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: Thu, 20 Dec 2018 00:43:02 +0000
Gerrit-HasComments: Yes

Reply via email to