Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12973 )
Change subject: IMPALA-8401: SIGQUIT initiates the graceful shutdown process ...................................................................... Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/12973/1/be/src/service/impalad-main.cc File be/src/service/impalad-main.cc: http://gerrit.cloudera.org:8080/#/c/12973/1/be/src/service/impalad-main.cc@62 PS1, Line 62: boost::shared_ptr<ImpalaServer> impala_server; > What are the implications of moving this here on the lifetime of impala_ser If we want it to live for the lifetime of the process we can just leak it and annotate with IGNORE_LEAKING_OBJECT. http://gerrit.cloudera.org:8080/#/c/12973/1/be/src/service/impalad-main.cc@69 PS1, Line 69: Status status = impala_server->StartShutdown(ONE_YEAR_IN_SECONDS, &shutdown_status); > StartShutdown creates a new thread, which might not be re-entrant safe. If +1 to this suggestion, I think it's hard to maintain async signal safety even if we manage to get it working initially. We could also modify StartShutdown() so that it only sets shutdown_deadline_. The shutdown thread could be started at the start of the process and poll shutdown_deadline_. That would avoid having two background threads in the shutdown process. -- To view, visit http://gerrit.cloudera.org:8080/12973 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I521ffd7526ac9a8a5c4996994eb68d6a855aef86 Gerrit-Change-Number: 12973 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Wed, 10 Apr 2019 18:45:06 +0000 Gerrit-HasComments: Yes
