ctubbsii commented on code in PR #5730:
URL: https://github.com/apache/accumulo/pull/5730#discussion_r2226978721


##########
assemble/bin/accumulo-cluster:
##########
@@ -276,9 +276,9 @@ function quote() {
   exit 1
 }
 
-# call debug to print, or execute if debug is off
+# call debug to print the command only, or execute asynchronously if debug is 
off
 function debugOrRun() {
-  debug "$(quote "$@")" || "$@"
+  debug "$(quote "$@")" || ("$@") &

Review Comment:
   We could keep a count and wait in batches, but it's hard to know how big the 
count should be, and making it configurable is a pain and adds complexity. I 
think that we should remember that these scripts are a cluster management 
reference implementation. If a user has a more complicated case that requires 
such batching, they can either run this command repeatedly with non-overlapping 
subsets of the servers to start processes on, or they can use alternate scripts 
centered around something like pssh or pdsh that has those kinds of features. I 
would hold off on this until we get feedback from users and determine that we 
actually need to have it.



##########
assemble/bin/accumulo-cluster:
##########
@@ -659,6 +659,8 @@ function control_services() {
     fi
   fi
 
+  wait

Review Comment:
   I was worried about the same. Maybe it would be a good idea to just rename 
the method to something like `debugOrRunAsync` and add a small comment that 
makes it clear that it creates background forked child processes.



##########
assemble/bin/accumulo-cluster:
##########
@@ -844,7 +846,7 @@ function main() {
   conf="${ACCUMULO_CONF_DIR:-$basedir/conf}"
 
   accumulo_cmd="$bin/accumulo"
-  SSH=('ssh' '-qnf' '-o' 'ConnectTimeout=2')
+  SSH=('ssh' '-qn' '-o' 'ConnectTimeout=2')

Review Comment:
   Since we now background it before any authentication/host-key checking, 
there's no possibility of interacting with it, so we should set it to batch 
mode, so if the host key verification fails, or if it tries to prompt for a 
password after publickey authentication fails, then it won't just be stuck 
waiting in the background waiting on user input that will never come.
   
   ```suggestion
     SSH=('ssh' '-qn' '-o' 'ConnectTimeout=2' '-o' 'BatchMode=yes')
   ```



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

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

Reply via email to