[GitHub] [accumulo-testing] ctubbsii commented on a change in pull request #187: Eliminate possible race condition in `agitator stop` script

[GitBox <gi...@apache.org>]

ctubbsii commented on a change in pull request #187:
URL: https://github.com/apache/accumulo-testing/pull/187#discussion_r785130004



##########
File path: bin/agitator
##########
@@ -157,10 +157,13 @@ function start_agitator() {
 
 function stop_agitator() {
   [[ -n $AGITATOR_USER ]] || AGITATOR_USER=$(whoami)
-  echo "Stopping all processes matching 'sleep' as $AGITATOR_USER"
-  pkill -f sleep 2>/dev/null
-  echo "Stopping all processes matching 'agitator' as $AGITATOR_USER"
-  pkill -f agitator 2>/dev/null
+  echo "Stopping all processes in the same process group as 'agitator' as $AGITATOR_USER"
+  ## get process ids of agitator processes then just keep the first
+  agitator_pid=$(pgrep -f agitator | head -1)
+  ## get the group process id of the agitator process then clear whitespace
+  group_pid=$(ps -p "${agitator_pid}" -o pgid= | xargs)
+  ## kill all processes in process group, should be all agitators and their sleep processes
+  kill -- -"${group_pid}"

Review comment:
       I think this will work better if you want to make use of arrays to get all agitator processes
   
   ```suggestion
     ## get process ids of all agitator processes (return 1 if none found)
     local agitator_pids=(); agitator_pids=($(pgrep -f agitator)) || return 1
     ## get the group process ids of all the agitator processes
     local group_pids=(); group_pids=($(ps -o pgid= -p "${agitator_pids[@]}"))
     ## kill all processes in the process groups (should include agitators and their sleep processes)
     kill -- "${group_pids[@]/#/-}"
   ```




-- 
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: notifications-unsubscribe@accumulo.apache.org

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