Lars Volker 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 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 have? That also makes it clear at the call-site what the behavior will be. "Handle..." still leaves it open what happens. 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 above 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 prefer not to mix -- 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 <[email protected]> Gerrit-Reviewer: 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, 17 Apr 2019 18:28:22 +0000 Gerrit-HasComments: Yes
