bipinprasad commented on a change in pull request #3326:
URL: https://github.com/apache/storm/pull/3326#discussion_r481326434
##########
File path: storm-server/src/main/java/org/apache/storm/utils/ServerUtils.java
##########
@@ -1217,9 +1199,127 @@ public static boolean areAllProcessesDead(Map<String,
Object> conf, String user,
}
}
if (cachedUserToUidMap.containsKey(user)) {
- return allDead = !ServerUtils.isAnyProcessAlive(pids,
cachedUserToUidMap.get(user));
+ return !ServerUtils.isAnyProcessAlive(pids,
cachedUserToUidMap.get(user));
} else {
- return allDead = !ServerUtils.isAnyProcessAlive(pids, user);
+ return !ServerUtils.isAnyProcessAlive(pids, user);
}
}
+
+ /**
+ * Find if the process is alive using the existence of /proc/<pid>
directory
+ * owned by the supplied user. This is an alternative to "ps -p pid -u
uid" command
+ * used in {@link #isAnyPosixProcessAlive(Collection, int)}
+ *
+ * <p>
+ * Processes are tracked using the existence of the directory
"/proc/<pid>
+ * For each of the supplied PIDs, their PID directory is checked for
existence and ownership
+ * by the specified uid.
+ * </p>
+ *
+ * @param pids Process IDs that need to be monitored for liveness
+ * @param user the userId that is expected to own that process
+ * @return true if any one of the processes is owned by user and alive,
else false
+ * @throws IOException on I/O exception
+ */
+ public static boolean isAnyPosixProcessPidDirAlive(Collection<Long> pids,
String user) throws IOException {
+ return isAnyPosixProcessPidDirAlive(pids, user, false);
+ }
+
+ /**
+ * Find if the process is alive using the existence of /proc/<pid>
directory
+ * owned by the supplied user. This is an alternative to "ps -p pid -u
uid" command
+ * used in {@link #isAnyPosixProcessAlive(Collection, int)}
+ *
+ * <p>
+ * Processes are tracked using the existence of the directory
"/proc/<pid>
+ * For each of the supplied PIDs, their PID directory is checked for
existence and ownership
+ * by the specified uid.
+ * </p>
+ *
+ * @param pids Process IDs that need to be monitored for liveness
+ * @param user the userId that is expected to own that process
+ * @param mockFileOwnerToUid if true (used for testing), then convert
File.owner to UID
+ * @return true if any one of the processes is owned by user and alive,
else false
+ * @throws IOException on I/O exception
+ */
+ @VisibleForTesting
+ public static boolean isAnyPosixProcessPidDirAlive(Collection<Long> pids,
String user, boolean mockFileOwnerToUid)
+ throws IOException {
+ File procDir = new File("/proc");
+ if (!procDir.exists()) {
+ throw new IOException("Missing process directory " +
procDir.getAbsolutePath() + ": method not supported on "
+ + "os.name=" + System.getProperty("os.name"));
+ }
+ for (long pid: pids) {
+ File pidDir = new File(procDir, String.valueOf(pid));
+ if (!pidDir.exists()) {
+ continue;
+ }
+ // check if existing process is owned by the specified user, if
not, the process is dead
+ String pidUser;
+ try {
+ pidUser = Files.getOwner(pidDir.toPath()).getName();
+ } catch (NoSuchFileException ex) {
+ continue; // process died before the user can be checked
+ }
+ if (mockFileOwnerToUid) {
+ // code activated in testing to simulate Files.getOwner
returning UID (which sometimes happens in runtime)
+ if (StringUtils.isNumeric(pidUser)) {
+ LOG.info("Skip mocking, since owner {} of pidDir {} is
already numeric", pidUser, pidDir);
+ } else {
+ Integer uid = cachedUserToUidMap.get(pidUser);
+ if (uid == null) {
+ uid = ServerUtils.getUserId(pidUser);
+ if (uid < 0) {
+ String err = String.format("Cannot get UID for %s,
while mocking the owner of pidDir %s",
+ pidUser, pidDir.getAbsolutePath());
+ throw new IOException(err);
+ }
+ cachedUserToUidMap.put(pidUser, uid);
+ LOG.info("Found UID {} for {}, while mocking the owner
of pidDir {}", uid, pidUser, pidDir);
+ } else {
+ LOG.info("Found cached UID {} for {}, while mocking
the owner of pidDir {}", uid, pidUser, pidDir);
+ }
+ pidUser = String.valueOf(uid);
+ }
+ }
+ //sometimes uid is returned instead of username - if so, try to
convert and compare with uid
+ if (StringUtils.isNumeric(pidUser)) {
+ // numeric pidUser - this is UID
+ LOG.debug("Process directory {} owner is uid={}", pidDir,
pidUser);
+ int pidUid = Integer.parseInt(pidUser);
+ Integer uid = cachedUserToUidMap.get(user);
+ if (uid == null) {
+ uid = ServerUtils.getUserId(user);
+ if (uid < 0) {
+ String err = String.format("Cannot get uid for %s to
compare with owner id=%d of process directory %s",
+ user, pidUid, pidDir.getAbsolutePath());
+ throw new IOException(err);
+ }
+ cachedUserToUidMap.put(user, uid);
+ }
+ if (uid == pidUid) {
+ LOG.debug("Process {} is alive and owned by user {}/{}",
pid, user, uid);
+ return true;
+ } else {
+ LOG.info("Prior process is dead, since directory {} owner
{} is not same as expected user {}/{}, "
+ + "likely pid {} was reused for a new process for
uid {}", pidDir, pidUser, user, uid, pid, uid);
Review comment:
renamed variables
----------------------------------------------------------------
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:
[email protected]
