Philip Zeyliger has posted comments on this change. (
http://gerrit.cloudera.org:8080/11490 )
Change subject: Added dumping of minidumps to finalize.sh
......................................................................
Patch Set 5:
(6 comments)
Minor comments. We're inconsistent here about whether we quote variables
consistently. I think "${FOO}" is probably good practice.
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh
File bin/jenkins/finalize.sh:
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@36
PS5, Line 36: if [[ $(find $LOGS_DIR -path *minidumps* -name *dmp) ]]; then
See comment on line 41.
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@38
PS5, Line 38: --error "Minidumps were generated."
Do we want to add the *dumped output as --stderr on these (perhaps limited by
head)? I don't remember how big they are, but the question is whether they're
useful as part of Jenkins output?
Noet that you could reasonably generate an XML file for every minidump if we
wanted that granularity.
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@39
PS5, Line 39: SYM_DIR=`mktemp -d`
Use $(...) since you use it in line 41. Best to be consistent.
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@41
PS5, Line 41: for minidump in $(find $LOGS_DIR -path *minidumps* -name *dmp);
do
-path "*minidumps*" -name "*dmp"
Bash keeps the *'s if they don't match anything, but best not to depend on that.
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@43
PS5, Line 43: $SYM_DIR > ${minidump}_dumped 2>/dev/null
Is 2> /dev/null too aggressive here? Should we dump it to a log in case there's
something interesting there?
http://gerrit.cloudera.org:8080/#/c/11490/5/bin/jenkins/finalize.sh@45
PS5, Line 45: rm -rf $SYM_DIR
Do we keep the *.dmp files? If so, do we want to keep SYM_DIR since maybe it's
useful?
--
To view, visit http://gerrit.cloudera.org:8080/11490
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1005378f996c2764012622a16a5d66bc83b6b4a1
Gerrit-Change-Number: 11490
Gerrit-PatchSet: 5
Gerrit-Owner: Thomas Marshall <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Comment-Date: Tue, 25 Sep 2018 18:16:20 +0000
Gerrit-HasComments: Yes
