[GitHub] [solr] magibney commented on a diff in pull request #1328: Extend start script to support ss and netstat as alternatives to lsof

["magibney (via GitHub)" <gi...@apache.org>]

magibney commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095906127


##########
solr/bin/solr:
##########
@@ -2158,6 +2158,35 @@ function mk_writable_dir() {
   fi
 }
 
+# Check whether at least one of lsof/netstat/ss are available
+function get_port_tool() {
+  for tool in 'lsof' 'netstat' 'ss'; do
+      which $tool &> nul

Review Comment:
   Looks like it might be preferable to use `command` in place of `which`? -- (there's [precedent](https://github.com/apache/solr/blob/e35347e7cc12d91af91cb38f9cef43ccc7a7b40e/solr/bin/solr#L125-L126) in this script already)



##########
solr/bin/solr:
##########
@@ -2312,13 +2341,13 @@ function start_solr() {
 	echo ""
     fi
     # no lsof on cygwin though
-    if lsof -v 2>&1 | grep -q revision; then
+    if [ -n "$(get_port_tool)" ]; then

Review Comment:
   rather than seeking the port tool a second time (calling `get_port_tool()` from within the `check_port_in_use()` function), would it be just as clean/easy to assign the output of `get_port_tool` to a variable, check that variable for `[ -n "$PORT_TOOL" ]`, and pass `$PORT_TOOL` as an arg to `check_port_in_use()`?
   
   Fine as-is, but seems there'd be very little downside to this change?



##########
solr/bin/solr:
##########
@@ -2312,13 +2341,13 @@ function start_solr() {
 	echo ""
     fi
     # no lsof on cygwin though
-    if lsof -v 2>&1 | grep -q revision; then
+    if [ -n "$(get_port_tool)" ]; then
       echo -n "Waiting up to $SOLR_START_WAIT seconds to see Solr running on port $SOLR_PORT"
       # Launch in a subshell to show the spinner
       (loops=0
       while true
       do
-        running=$(lsof -t -PniTCP:$SOLR_PORT -sTCP:LISTEN || :)
+        running=$(check_port_in_use $SOLR_PORT)

Review Comment:
   The ` || : ` in the initial command is curious. I infer that this protects against the possibility that a failed `lsof` command might still write to stdout, which would get captured into the `running` variable. Whatever the reason, might we want to either add ` || : ` to the `ss` and `netstat` variants of the port command, or move the ` || : ` back to the outer level here (ensuring that failed invocations of `lsof`/`ss`/`netstat` propagate failure exit codes via `check_port_in_use()`?) 



##########
solr/bin/solr:
##########
@@ -2158,6 +2158,35 @@ function mk_writable_dir() {
   fi
 }
 
+# Check whether at least one of lsof/netstat/ss are available
+function get_port_tool() {
+  for tool in 'lsof' 'netstat' 'ss'; do
+      which $tool &> nul
+      if [[ $? -eq 0 ]]; then
+        echo $tool
+        return
+      fi
+  done
+}
+
+# Will get the PID of the java process running on the specified port, if one is running
+# Otherwise empty result. Use get_port_tool to check whether lsof/ss/netstat is installed first
+function check_port_in_use() {
+  local port=$1
+
+  case $(get_port_tool) in
+    lsof)
+      lsof -iTCP:${port} 2>/dev/null | grep "LISTEN" | grep -Eow "java\s*?[0-9]+" | grep -Eow [0-9]+
+      ;;
+    ss)
+      ss -ntpa 2>/dev/null | grep ":${port} " | grep 'LISTEN ' | grep -Eow "\"java\",(pid=)?[0-9]+?" | grep -Eow "[0-9]+"
+      ;;
+    netstat)
+      netstat -nlp 2>/dev/null | grep ":${port} " | grep -Eow " [0-9]+/java" | grep -Eow "[0-9]+"
+      ;;

Review Comment:
   Analogous to the change above (restoring the more targeted use of `lsof`, avoiding the need for `grep`), I wonder if there are more targeted (but still portable) versions of this command for `ss` and `netstat`?



-- 
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-unsubscribe@solr.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org