Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/12973 )
Change subject: IMPALA-8401: SIGRTMIN initiates the graceful shutdown process ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/12973/5/be/src/common/init.h File be/src/common/init.h: http://gerrit.cloudera.org:8080/#/c/12973/5/be/src/common/init.h@25 PS5, Line 25: SIGNAL > nit: lowercase Done http://gerrit.cloudera.org:8080/#/c/12973/5/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/12973/5/be/src/common/init.cc@232 PS5, Line 232: void HandleSysCallError(const int syscall_ret_val, const string& msg) { > Maybe call this method "AbortIfError" in reference to the macros we already Done http://gerrit.cloudera.org:8080/#/c/12973/5/be/src/common/init.cc@236 PS5, Line 236: exit(1); > Use _exit here because exit() is also not async-signal-safe. See comment ab do we need that here? since this is on the main thread and not in the signal handler thread like in L226. http://gerrit.cloudera.org:8080/#/c/12973/5/tests/custom_cluster/test_restart_services.py File tests/custom_cluster/test_restart_services.py: http://gerrit.cloudera.org:8080/#/c/12973/5/tests/custom_cluster/test_restart_services.py@97 PS5, Line 97: match = re.match(r'shutdown grace period left: ([0-9ms]*), deadline left: ([0-9ms]*),' > nit: have space trailing the previous line, or lead the current line, but p Done -- 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: 5 Gerrit-Owner: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 17 Apr 2019 21:49:56 +0000 Gerrit-HasComments: Yes
