[GitHub] [storm] Ethanlm commented on a change in pull request #3362: [STORM-3723] Fix isAnyPosixProcessPidDirAlive for PID reassignment

[GitBox]

Ethanlm commented on a change in pull request #3362:
URL: https://github.com/apache/storm/pull/3362#discussion_r544382944



##########
File path: storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
##########
@@ -1295,22 +1295,45 @@ public static boolean 
isAnyPosixProcessPidDirAlive(Collection<Long> pids, String
                     LOG.debug("Process {} is alive and owned by expectedUser 
{}/{}", pid, expectedUser, expectedUid);
                     return true;
                 }
-                LOG.info("Prior process is dead, since directory {} owner {} 
is not same as expected expectedUser {}/{}, "
-                        + "likely pid {} was reused for a new process for uid 
{}",
-                        pidDir, actualUser, expectedUser, expectedUid, pid, 
actualUid);
+                LOG.info("Prior process is dead, since directory {} owner {} 
is not same as expectedUser {}/{}, "
+                        + "likely pid {} was reused for a new process for uid 
{}, {}",
+                        pidDir, actualUser, expectedUser, expectedUid, pid, 
actualUid, getProcessDesc(pidDir));
             } else {
                 // actualUser is a string
                 LOG.debug("Process directory {} owner is {}", pidDir, 
actualUser);
                 if (expectedUser.equals(actualUser)) {
                     LOG.debug("Process {} is alive and owned by expectedUser 
{}", pid, expectedUser);
                     return true;
                 }
-                LOG.info("Prior process is dead, since directory {} owner {} 
is not same as expected expectedUser {}, "
-                        + "likely pid {} was reused for a new process for 
expectedUser {}",
-                        pidDir, actualUser, expectedUser, pid, actualUser);
+                LOG.info("Prior process is dead, since directory {} owner {} 
is not same as expectedUser {}, "
+                        + "likely pid {} was reused for a new process for 
actualUser {}, {}}",
+                        pidDir, actualUser, expectedUser, pid, actualUser, 
getProcessDesc(pidDir));
             }
         }
         LOG.info("None of the processes {} are alive AND owned by expectedUser 
{}", pids, expectedUser);
         return false;
     }
+
+    /**
+     * Support method to obtain additional log info for the process. Use the 
contents of comm and cmdline
+     * in the process directory. Note that this method works properly only on 
posix systems with /proc directory.
+     *
+     * @param pidDir PID directory (/proc/&lt;pid&gt;)
+     * @return process description string
+     */
+    private static String getProcessDesc(File pidDir) {
+        String comm = "";
+        try {
+            comm = String.join(", ", 
Files.readAllLines(pidDir.toPath().resolve("comm")));
+        } catch (IOException ex) {
+            // ignore

Review comment:
       Maybe log a warning here and below?




----------------------------------------------------------------
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.

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