[GitHub] [nifi] bejancsaba commented on a diff in pull request #7448: NIFI-11761 Fixed MiNiFi restart issue when graceful shutdown period expires. MiNiFi restart sends bootstrap to background

["bejancsaba (via GitHub)" <gi...@apache.org>]

bejancsaba commented on code in PR #7448:
URL: https://github.com/apache/nifi/pull/7448#discussion_r1246295415


##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/command/StopRunner.java:
##########
@@ -112,6 +114,19 @@ private void gracefulShutDownMiNiFiProcess(long minifiPid) throws IOException {
         if (minifiPid != UNINITIALIZED) {
             processUtils.shutdownProcess(minifiPid, "MiNiFi has not finished shutting down after {} seconds. Killing process.",
                 gracefulShutdownParameterProvider.getGracefulShutdownSeconds());
+            int maxRetry = 5;
+            while (processUtils.isProcessRunning(minifiPid)) {
+                if (maxRetry == 0) {
+                    throw new IOException("Failed to stop MiNiFi process. MiNiFi process is still running after graceful shutdown has completed");

Review Comment:
   We could explicitly say here "...after graceful shutdown completed and killing attempted afterwards" or something along those lines I think we can't get here only if graceful time expired kill was attempted but still the process didn't stop. Right?



##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/command/StopRunner.java:
##########
@@ -112,6 +114,19 @@ private void gracefulShutDownMiNiFiProcess(long minifiPid) throws IOException {
         if (minifiPid != UNINITIALIZED) {
             processUtils.shutdownProcess(minifiPid, "MiNiFi has not finished shutting down after {} seconds. Killing process.",
                 gracefulShutdownParameterProvider.getGracefulShutdownSeconds());
+            int maxRetry = 5;
+            while (processUtils.isProcessRunning(minifiPid)) {
+                if (maxRetry == 0) {
+                    throw new IOException("Failed to stop MiNiFi process. MiNiFi process is still running after graceful shutdown has completed");
+                }
+                CMD_LOGGER.debug("MiNiFi process is still running after shutdown has completed");

Review Comment:
   I think this could be WARN as if we are here that is already not good



##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/command/StopRunner.java:
##########
@@ -112,6 +114,19 @@ private void gracefulShutDownMiNiFiProcess(long minifiPid) throws IOException {
         if (minifiPid != UNINITIALIZED) {
             processUtils.shutdownProcess(minifiPid, "MiNiFi has not finished shutting down after {} seconds. Killing process.",
                 gracefulShutdownParameterProvider.getGracefulShutdownSeconds());
+            int maxRetry = 5;

Review Comment:
   I'm not sure whether it would be useful or not to externalise at least this one (maybe the sleep time is not justified but it can be argued that this makes sense to be configurable) what do you think?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org