You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by "colvinco (via GitHub)" <gi...@apache.org> on 2023/02/03 10:54:22 UTC

[GitHub] [solr] colvinco opened a new pull request, #1328: Extend start script to support ss and netstat as alternatives to lsof

colvinco opened a new pull request, #1328:
URL: https://github.com/apache/solr/pull/1328

   https://issues.apache.org/jira/browse/SOLR-XXXXX
   
   <!--
   _(If you are a project committer then you may remove some/all of the following template.)_
   
   Before creating a pull request, please file an issue in the ASF Jira system for Solr:
   
   * https://issues.apache.org/jira/projects/SOLR
   
   For something minor (i.e. that wouldn't be worth putting in release notes), you can skip JIRA. 
   To create a Jira issue, you will need to create an account there first.
   
   The title of the PR should reference the Jira issue number in the form:
   
   * SOLR-####: <short description of problem or changes>
   
   SOLR must be fully capitalized. A short description helps people scanning pull requests for items they can work on.
   
   Properly referencing the issue in the title ensures that Jira is correctly updated with code review comments and commits. -->
   
   
   # Description
   
   The current start command requires `lsof` to be available. If you don't have `lsof` it does a hardcoded 10 second wait and then assumes the process has started. This updates the script to support netstat and ss as alternatives.
   I'm not sure if it's worth creating a JIRA for this.
   
   I'm also not sure if it's worth putting it this in, as it adds extra paths through the script so there's more potential for things to go wrong, as different versions of the tools may take different arguments, and it's generally not hard to install `lsof`.
   However we have been using this logic in our product for about 4 years, so I thought I would offer it up and see if it's of interest.
   
   # Solution
   
   Use `which` to check if `lsof`, `netstat`, `ss` is available and pick the first one, then execute the appropriate command.
   
   # Tests
   
   Manually tested
   
   # Checklist
   
   Please review the following and check all that apply:
   
   - [x] I have reviewed the guidelines for [How to Contribute](https://wiki.apache.org/solr/HowToContribute) and my code conforms to the standards described there to the best of my ability.
   - [ ] I have created a Jira issue and added the issue ID to my pull request title.
   - [x] I have given Solr maintainers [access](https://help.github.com/en/articles/allowing-changes-to-a-pull-request-branch-created-from-a-fork) to contribute to my PR branch. (optional but recommended)
   - [x] I have developed this patch against the `main` branch.
   - [ ] I have run `./gradlew check`.
   - [ ] I have added tests for my changes.
   - [ ] I have added documentation for the [Reference Guide](https://github.com/apache/solr/tree/main/solr/solr-ref-guide)
   


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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096112550


##########
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:
   Ah that's a pain. Yeah, on linux it gives the PID/program name column
   ```
   # netstat -nlp 2>/dev/null
   Active Internet connections (only servers)
   Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
   tcp        0      0 127.0.0.1:8983          0.0.0.0:*               LISTEN      22862/java
   tcp        0      0 0.0.0.0:9983            0.0.0.0:*               LISTEN      3380/java
   tcp        0      0 127.0.0.1:7983          0.0.0.0:*               LISTEN      22862/java
   ```
   
   https://apple.stackexchange.com/a/327967 suggests `-v` is the nearest alternative but I don't have a mac to test on.



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


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

Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096052115


##########
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:
   Great! thanks for looking into it. This seems reasonable, and maybe if anyone else wants to weigh in wrt how standard these flags are, that could be helpful.
   
   Assuming continuing to use grep, we really don't _need_ "extended" grep here (`grep -E` -- nor [here](https://github.com/apache/solr/blob/e35347e7cc12d91af91cb38f9cef43ccc7a7b40e/solr/bin/solr#L180)). Removing the `-E` flag would just mean a little bit of extra escaping of special characters in the pattern -- not sure it's worth it, totally fine as-is, just thought I'd mention.



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096021700


##########
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:
   Right yep. 
   
   I see `ss` supports filtering to listening sockets and filtering by ports - `ss -l '( sport = :8983 )'`, but I don't know whether those options have been standard for a long time or not, but I also can't see anything suggesting that they're new.
   The `netstat` `-l` is already filtering to listen, but it doesn't seem like there's an explicit port filter for it.
   
   I'll change the `ss` command to use those options



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


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

Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096019115


##########
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:
   ah yes, you're right! Maybe it just ensures that the script will proceed, regardless of how the script/environment does error handling (`set -e` etc.)?



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096093955


##########
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:
   Thought I would try it anyway. 
   Also hadn't realized that I had lost the `-sTCP:LISTEN` filter from lsof, so that's back in :)



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


Re: [PR] SOLR-16645: Extend start script to support ss and netstat as alternatives to lsof [solr]

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1328:
URL: https://github.com/apache/solr/pull/1328#issuecomment-2413690959

   @epugh no worries, will see what https://issues.apache.org/jira/browse/SOLR-17450 brings. To be fair, with containers it's not hard to ensure that lsof is available anyway so the need for alternatives diminished.


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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096105477


##########
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:
   > if lost doesn't exist
   
   Sorry do you mean if `lsof` doesn't exist?
   With these changes, `command -v` will already have been used to check if lsof / ss / netstat is available, and this won't be called if they aren't, it will already have branched to https://github.com/apache/solr/blob/dcb7d2406b574e24fa8636cf717a8af2039c79b4/solr/bin/solr#L2369



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


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

Posted by "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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095968781


##########
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:
   My thinking here was that it's better to verify that the process that's listening is a java process, rather than some other process that is running the port you expect Solr on.
   
   However, that's partly because my original use of these scripts was in a more general context. In this context, I guess we already have confidence that the port wasn't already in use?



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095862039


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

Review Comment:
   ```suggestion
     for tool in 'lsof' 'ss' 'netstat'; do
   ```



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095678062


##########
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 -t -PniTCP:${port} -sTCP:LISTEN || :

Review Comment:
   ```suggestion
         lsof -iTCP:${port} 2>/dev/null | grep "LISTEN" | grep -Eow "java\s*?[0-9]+" | grep -Eow [0-9]+
   ```
   Not sure if it's better to check if the process is `java` or not and just assume that it is.



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


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

Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096098937


##########
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' 'ss' 'netstat'; do
+      if [[ -x "$(command -v $tool)" ]]; 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 tool=$1
+  local port=$2
+
+  case $tool in
+    lsof)
+      lsof -iTCP:${port} 2>/dev/null -sTCP:LISTEN 2>/dev/null | grep -ow "java\s*\?[0-9]\+" | grep -ow "[0-9]\+"

Review Comment:
   `lsof -iTCP:${port} 2>/dev/null -sTCP:LISTEN 2>/dev/null ` -- looks like accidental unusual double `2>/dev/null`? Also, we really don't want to lose the `-Pn` args to `lsof` I think:
   ```
          -P   inhibits the conversion of port numbers to port names for network files. 
               Inhibiting the conversion may make lsof run a little faster.  It is also useful
               when port name lookup is not working properly.
          -n   inhibits the conversion of network numbers to host names for network files.
               Inhibiting conversion may make lsof run faster.  It is also useful when host
               name lookup is not working properly.
   ```



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


[GitHub] [solr] colvinco commented on pull request #1328: SOLR-16645: Extend start script to support ss and netstat as alternatives to lsof

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on PR #1328:
URL: https://github.com/apache/solr/pull/1328#issuecomment-1531035363

   Hello, sorry I've been away for a while. The outstanding issue is to work out what to do with netstat as the standard arguments on mac differ to linux. https://github.com/apache/solr/pull/1328#discussion_r1096105999
   
   So I guess the options are to either a) not include netstat, b) find common arguments that work on both OSes, or c) check the OS (or the arguments supported by the available netstat) and switch between them. 
   
   Option b sounds like it would be best if it's possible, but I don't have a mac to test with.
   As it's only working out whether a port is in use, returning the pid/process name isn't that important, so maybe just `netstat -nl 2>/dev/null | grep "LISTEN" | grep ":${port} "` would work well enough cross platform? 


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


Re: [PR] SOLR-16645: Extend start script to support ss and netstat as alternatives to lsof [solr]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #1328:
URL: https://github.com/apache/solr/pull/1328#issuecomment-1947529783

   This PR had no visible activity in the past 60 days, labeling it as stale. Any new activity will remove the stale label. To attract more reviewers, please tag someone or notify the dev@solr.apache.org mailing list. Thank you for your contribution!


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


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

Posted by "madrob (via GitHub)" <gi...@apache.org>.
madrob commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096095124


##########
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:
   Yes, if lost doesn't exist we don't want the script to error out.



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095676844


##########
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]+

Review Comment:
   ```suggestion
         lsof -t -PniTCP:${port} -sTCP:LISTEN || :
   ```



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


[GitHub] [solr] epugh commented on pull request #1328: SOLR-16645: Extend start script to support ss and netstat as alternatives to lsof

Posted by "epugh (via GitHub)" <gi...@apache.org>.
epugh commented on PR #1328:
URL: https://github.com/apache/solr/pull/1328#issuecomment-1529021311

   What's the status on this?  I stumbled across it as part of looking for tickets related to the Solr CLI...  SOLR-16757.   Is this ready to be merged?


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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096103320


##########
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' 'ss' 'netstat'; do
+      if [[ -x "$(command -v $tool)" ]]; 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 tool=$1
+  local port=$2
+
+  case $tool in
+    lsof)
+      lsof -iTCP:${port} 2>/dev/null -sTCP:LISTEN 2>/dev/null | grep -ow "java\s*\?[0-9]\+" | grep -ow "[0-9]\+"

Review Comment:
   Oh yep, sorry copying too many things around now.



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096011334


##########
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:
   I must admit I can't see that it would have had any effect here. But I may be coming it at from the wrong angle. I see it was originally added by @madrob https://github.com/apache/solr/commit/3ed17c2737db038c2908a1475099647fb4ef5b38  
   
   If I create a script:
   ```
   echo hello
   exit 1
   ```
   And call it with ` || :` the stdout still gets captured. The exit code will be 0 rather than 1, but I don't think that's adding anything here?



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


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

Posted by "elyograg (via GitHub)" <gi...@apache.org>.
elyograg commented on PR #1328:
URL: https://github.com/apache/solr/pull/1328#issuecomment-1415878965

   ss is probably more likely to have the same options on multiple platforms than 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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096112550


##########
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:
   Ah that's a pain. Yeah, on linux it gives the PID/program name column
   ```
   # netstat -nlp 2>/dev/null
   Active Internet connections (only servers)
   Proto Recv-Q Send-Q Local Address           Foreign Address         State       PID/Program name
   tcp        0      0 127.0.0.1:8983          0.0.0.0:*               LISTEN      22862/java
   tcp        0      0 0.0.0.0:9983            0.0.0.0:*               LISTEN      3380/java
   tcp        0      0 127.0.0.1:7983          0.0.0.0:*               LISTEN      22862/java
   ```
   
   https://apple.stackexchange.com/a/327967 suggests `-v` is the nearest alternative.
   



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


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

Posted by "magibney (via GitHub)" <gi...@apache.org>.
magibney commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1095995278


##########
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:
   I think it's fine to do a rough check that the process you get is likely a java process. The more practical concern is `grep ":${port}"` and `grep 'LISTEN '`-- ideally arguments could be provided that would directly achieve this filtering as opposed to post-filtering unrestricted program output. E.g., maybe some unfortunate person is running an executable program called "LISTEN" 😁. By comparison, the args to the `lsof` command are explicit: we're getting _exactly_ the pid, and it's exactly listening, exactly on the port we're interested in. The grep stuff, though admittedly likely to work just fine, is a lot fuzzier (even it it's probably be how I'd do troubleshooting on cmd-line, as opposed to knowing the exact flags to these commands).
   
   The only hesitance is: I'm not sure to what extent the args might vary across different implementations of these commands, on different platforms.



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096011334


##########
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:
   I must admint can't see that it would have had any effect here. But I may be coming it at from the wrong angle. I see it was originally added by @madrob https://github.com/apache/solr/commit/3ed17c2737db038c2908a1475099647fb4ef5b38  
   
   If I create a script:
   ```
   echo hello
   exit 1
   ```
   And call it with ` || :` the stdout still gets captured. The exit code will be 0 rather than 1, but I don't think that's adding anything here?



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096021700


##########
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:
   Right yep. 
   
   I see `ss` supports filtering to listening sockets and filtering by ports - `ss -l -at '( sport = :8983 )'`, but I don't know whether those options have been standard for a long time or not, but I also can't see anything suggesting that they're new.
   The `netstat` `-l` is already filtering to listen, but it doesn't seem like there's an explicit port filter for it.
   
   I'll change the `ss` command to use those options



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


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

Posted by "madrob (via GitHub)" <gi...@apache.org>.
madrob commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096105999


##########
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:
   `netstat -nlp` does different things on linux/Mac I think. on my machine I get the output
   ```
   $ netstat -nlp
   netstat: option requires an argument -- p
   ```
   where `-p` is for protocol, but on linux `-p` is for program



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


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

Posted by "colvinco (via GitHub)" <gi...@apache.org>.
colvinco commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096105477


##########
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:
   With these changes, `command -v` will already have been used to check if lsof / ss / netstat is available, and this won't be called if they aren't, it will already have branched to https://github.com/apache/solr/blob/dcb7d2406b574e24fa8636cf717a8af2039c79b4/solr/bin/solr#L2369



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


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

Posted by "madrob (via GitHub)" <gi...@apache.org>.
madrob commented on code in PR #1328:
URL: https://github.com/apache/solr/pull/1328#discussion_r1096095124


##########
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:
   Yes, if lsof doesn't exist we don't want the script to error out.



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