You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by "ddanielr (via GitHub)" <gi...@apache.org> on 2025/04/24 17:24:44 UTC
[PR] Improves accumulo-service script [accumulo]
ddanielr opened a new pull request, #5502:
URL: https://github.com/apache/accumulo/pull/5502
Adds improved list output and cleanup of dead pid files.
Ported changes from #5439
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr merged PR #5502:
URL: https://github.com/apache/accumulo/pull/5502
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223663176
##########
assemble/bin/accumulo-service:
##########
@@ -215,9 +225,20 @@ function kill_service() {
function list_processes() {
local service_type=$1
find_processes "$service_type"
- echo "Currently running ${service_type} processes:"
+ echo "Currently running ${service_type} processes (fields: process pid port):"
for process in "${RUNNING_PROCESSES[@]}"; do
- echo "$process"
+ local pid_file
+ local pid
+ local port
+ pid_file="${ACCUMULO_PID_DIR}/accumulo-${process}.pid"
Review Comment:
removed braces in 40ca7b5af1cf38601942650a387958f1b49f2c0a
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "dlmarion (via GitHub)" <gi...@apache.org>.
dlmarion commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2059083729
##########
assemble/bin/accumulo-service:
##########
@@ -215,9 +225,20 @@ function kill_service() {
function list_processes() {
local service_type=$1
find_processes "$service_type"
- echo "Currently running ${service_type} processes:"
+ echo "Currently running ${service_type} processes (fields: process pid port):"
for process in "${RUNNING_PROCESSES[@]}"; do
- echo "$process"
+ local pid_file
+ local pid
+ local port
+ pid_file="${ACCUMULO_PID_DIR}/accumulo-${process}.pid"
+ pid=<("$pid_file")
+ port=$(netstat -tnlp 2>/dev/null | grep "$pid" | awk '{print $4}' | awk -F ':' '{print $2}' | paste -sd "," -)
Review Comment:
I think that if you do the following, then it will handle ipv6 addresses of the form ":::1234"
```suggestion
port=$(netstat -tnlp 2>/dev/null | grep "$pid" | awk '{print $4}' | awk -F ':' '{print $NF}' | paste -sd "," -)
```
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223672952
##########
assemble/bin/accumulo-service:
##########
@@ -215,9 +225,20 @@ function kill_service() {
function list_processes() {
local service_type=$1
find_processes "$service_type"
- echo "Currently running ${service_type} processes:"
+ echo "Currently running ${service_type} processes (fields: process pid port):"
for process in "${RUNNING_PROCESSES[@]}"; do
- echo "$process"
+ local pid_file
+ local pid
+ local port
+ pid_file="${ACCUMULO_PID_DIR}/accumulo-${process}.pid"
+ pid=$(<"$pid_file")
+ port=$(netstat -tnlp 2>/dev/null | grep "$pid" | awk '{print $4}' | awk -F ':' '{print $NF}' | paste -sd "," -)
Review Comment:
Switched to using `ss` vs `netstat` as it's more likely to be installed by default.
Also, `ss` presents a slightly better match output for pids `users:(("java",pid=640697,fd=415))` so I updated the grep -F statement to check for `pid=$pid,` in #5750
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223663923
##########
assemble/bin/accumulo-service:
##########
@@ -215,9 +225,20 @@ function kill_service() {
function list_processes() {
local service_type=$1
find_processes "$service_type"
- echo "Currently running ${service_type} processes:"
+ echo "Currently running ${service_type} processes (fields: process pid port):"
for process in "${RUNNING_PROCESSES[@]}"; do
- echo "$process"
+ local pid_file
+ local pid
+ local port
+ pid_file="${ACCUMULO_PID_DIR}/accumulo-${process}.pid"
+ pid=$(<"$pid_file")
Review Comment:
Added comment in 40ca7b5af1cf38601942650a387958f1b49f2c0a
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223660521
##########
assemble/bin/accumulo-service:
##########
@@ -148,9 +148,19 @@ function control_process() {
function find_processes() {
local service_type=$1
local file
- for file in "$ACCUMULO_PID_DIR"/*; do
- if file=$(expr "$file" : '^.*/accumulo-\('"$service_type"'.*\)[.]pid$'); then
- RUNNING_PROCESSES+=("$file")
+ local filepath
+ local expected_pid
+ local found_pid
+ for filepath in "$ACCUMULO_PID_DIR"/*; do
+ if file=$(expr "$filepath" : '^.*/accumulo-\('"$service_type"'.*\)[.]pid$'); then
Review Comment:
changed in 40ca7b5af1cf38601942650a387958f1b49f2c0a
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "keith-turner (via GitHub)" <gi...@apache.org>.
keith-turner commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2059099665
##########
assemble/bin/accumulo-service:
##########
@@ -148,9 +148,19 @@ function control_process() {
function find_processes() {
local service_type=$1
local file
- for file in "$ACCUMULO_PID_DIR"/*; do
- if file=$(expr "$file" : '^.*/accumulo-\('"$service_type"'.*\)[.]pid$'); then
- RUNNING_PROCESSES+=("$file")
+ local filepath
+ local expected_pid
+ local found_pid
+ for filepath in "$ACCUMULO_PID_DIR"/*; do
+ if file=$(expr "$filepath" : '^.*/accumulo-\('"$service_type"'.*\)[.]pid$'); then
+ expected_pid=<("$filepath")
+ found_pid=$(pgrep -F "$filepath" -f "$file")
+ if [[ $found_pid != "$expected_pid" ]]; then
+ echo "failed pid in file: $filepath" >&2
Review Comment:
```suggestion
echo "removing stale pid file $filepath" >&2
```
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223661611
##########
assemble/bin/accumulo-service:
##########
@@ -159,7 +169,7 @@ function stop_service() {
local service_type=$1
local service_name=$2
local all_flag=$3
- if [[ $all_flag == 'true' ]]; then
+ if $all_flag; then
Review Comment:
yep, reverted in 40ca7b5af1cf38601942650a387958f1b49f2c0a
--
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
Re: [PR] Improves accumulo-service script [accumulo]
Posted by "ddanielr (via GitHub)" <gi...@apache.org>.
ddanielr commented on code in PR #5502:
URL: https://github.com/apache/accumulo/pull/5502#discussion_r2223675053
##########
assemble/bin/accumulo-service:
##########
@@ -215,9 +225,20 @@ function kill_service() {
function list_processes() {
local service_type=$1
find_processes "$service_type"
- echo "Currently running ${service_type} processes:"
+ echo "Currently running ${service_type} processes (fields: process pid port):"
for process in "${RUNNING_PROCESSES[@]}"; do
- echo "$process"
+ local pid_file
+ local pid
+ local port
+ pid_file="${ACCUMULO_PID_DIR}/accumulo-${process}.pid"
+ pid=$(<"$pid_file")
+ port=$(netstat -tnlp 2>/dev/null | grep "$pid" | awk '{print $4}' | awk -F ':' '{print $NF}' | paste -sd "," -)
+ # check that only a single port was seen
+ if (($(echo "$port" | grep -c -E '^[0-9]+(,[0-9]+)*$') != 1)); then
+ echo "ERROR unexpected ports $(hostname) process:$process pid:$pid ports:$port" >&2
+ else
+ echo "$process $pid $port"
+ fi
Review Comment:
Multiple ports are a valid state due to metric exporters like prometheus binding to a second port.
Removed the comment but updated the check logic so it was cleaner.
--
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