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-unsubscr...@nifi.apache.org

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

Reply via email to