Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12858 )

Change subject: IMPALA-8359: Fix coverage data generation for impalads
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12858/2/be/src/common/init.cc
File be/src/common/init.cc:

http://gerrit.cloudera.org:8080/#/c/12858/2/be/src/common/init.cc@199
PS2, Line 199:   // We measure code coverage that needs cleanup of resources to 
flush coverage data.
             :   // In this case we risk calling exit() that does the cleanup 
unlike _exit().
             :   exit(0);
It's good to track this down. I'm glad we know this is due to the change to an 
_exit(0) call.

There is a function called __gcov_flush() that dumps the coverage data. You can 
declare it and use it if you link with "-lgcov". I think a common pattern is to 
call it before _exit(0). I did a quick check, and it seems to work.

Kudu also has some code to flush coverage data (see TryFlushCoverage() in 
be/src/kudu/util/debug-util.cc in our repo). If we link with -lgcov and call 
kudu::TryFlushCoverage() before _exit(0), then that would work. Kudu uses clang 
for coverage rather than gcc, but the same code should work.

I would prefer one of these two solutions rather than an exit() call, but as a 
workaround, I would approve this.



--
To view, visit http://gerrit.cloudera.org:8080/12858
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9be1e1e73b6cfc3557077f763aee4dbfcc7a2d27
Gerrit-Change-Number: 12858
Gerrit-PatchSet: 2
Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Mar 2019 17:34:04 +0000
Gerrit-HasComments: Yes

Reply via email to