You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@pegasus.apache.org by "limowang (via GitHub)" <gi...@apache.org> on 2025/02/06 07:38:28 UTC
[PR] Fix the way the wait_manual_compact function parses the output of the… [incubator-pegasus]
limowang opened a new pull request, #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191
… remote_command -t replica-server replica.query-compact shell command.
### What problem does this PR solve?
[#2190](https://github.com/apache/incubator-pegasus/issues/2190)
### What is changed and how does it work?
The [commit 2d68748](https://github.com/apache/incubator-pegasus/commit/2d68748264500a3fb3850a0dcfc8af8e8595a19a#:~:text=info%5B%22recent_enqueue_at%22%5D) changed the output format of the shell command remote_command -t replica-server replica.query-compact ${app_id}, which caused the wait_manual_compact function to still parse the output based on the original format, leading to incorrect behavior. It has now been fixed by modifying the parsing method.
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "limowang (via GitHub)" <gi...@apache.org>.
limowang commented on PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#issuecomment-2646830037
The shell command remote_command -t replica-server replica.query-compact ${app_id} outputs in the following format in the latest version:
CALL [replica-server] [10.1.132.34:8171] succeed:
8 processed, 0 not found
4.0@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.1@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.2@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.3@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.4@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.5@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.6@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
4.7@10.1.132.34:8171@P : {"last_finish":"-","last_used_ms":"-","recent_enqueue_at":"-","recent_start_at":"-"}
Below is the output example from the previous version：
CALL [replica-server] [10.1.132.36:8171] succeed: 8 processed, 0 not found
4.0@10.1.132.36:8171@P : last finish at [-]
4.1@10.1.132.36:8171@P : last finish at [-]
4.2@10.1.132.36:8171@P : last finish at [-]
4.3@10.1.132.36:8171@P : last finish at [-]
4.4@10.1.132.36:8171@P : last finish at [-]
4.5@10.1.132.36:8171@P : last finish at [-]
4.6@10.1.132.36:8171@P : last finish at [-]
4.7@10.1.132.36:8171@P : last finish at [-]
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#discussion_r1954007588
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
processing_count=$((queue_count+running_count))
- finish_count=`grep "last finish at" ${query_log_file} | grep -v "recent enqueue at" | grep -v "recent start at" | grep -o 'last finish at [^,]*' | sed 's/\[/,/;s/\]//' | awk -F"," -v date="$earliest_finish_time_ms" 'BEGIN{count=0}{if(length($2)==23 && $2>=date){count++;}}END{print count}'`
+ finish_count=$(awk 'BEGIN {count=0} {match($0, /"last_finish":"([^"]+)"/, finish); if (finish[1] != "-" && length(finish[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
However, what will happen if `_manual_compact_last_finish_time_ms` still hold the last finish time of previous `once` manual compaction task ? `${trigger_time}` of this manual compaction task would be less than that.
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "acelyc111 (via GitHub)" <gi...@apache.org>.
acelyc111 merged PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "limowang (via GitHub)" <gi...@apache.org>.
limowang commented on code in PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#discussion_r1952200057
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
processing_count=$((queue_count+running_count))
- finish_count=`grep "last finish at" ${query_log_file} | grep -v "recent enqueue at" | grep -v "recent start at" | grep -o 'last finish at [^,]*' | sed 's/\[/,/;s/\]//' | awk -F"," -v date="$earliest_finish_time_ms" 'BEGIN{count=0}{if(length($2)==23 && $2>=date){count++;}}END{print count}'`
+ finish_count=$(awk 'BEGIN {count=0} {match($0, /"last_finish":"([^"]+)"/, finish); if (finish[1] != "-" && length(finish[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
The $earliest_finish_time_ms parameter represents the trigger time (trigger_time) for the manual_compact task. The progress of regular background compaction tasks will not be output in the remote_command -t replica-server replica.query-compact ${app_id} shell command, so the manual_compact task and regular background compaction tasks do not conflict, preventing output errors. Therefore, when parsing the results, $earliest_finish_time_ms can be ignored.
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "limowang (via GitHub)" <gi...@apache.org>.
limowang commented on code in PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#discussion_r1952200057
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
processing_count=$((queue_count+running_count))
- finish_count=`grep "last finish at" ${query_log_file} | grep -v "recent enqueue at" | grep -v "recent start at" | grep -o 'last finish at [^,]*' | sed 's/\[/,/;s/\]//' | awk -F"," -v date="$earliest_finish_time_ms" 'BEGIN{count=0}{if(length($2)==23 && $2>=date){count++;}}END{print count}'`
+ finish_count=$(awk 'BEGIN {count=0} {match($0, /"last_finish":"([^"]+)"/, finish); if (finish[1] != "-" && length(finish[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
The `$earliest_finish_time_ms` parameter represents the trigger time (`trigger_time`) for the `manual_compact` task. The progress of regular background compaction tasks will not be output in the `remote_command -t replica-server replica.query-compact ${app_id}` shell command, so the `manual_compact` task and regular background compaction tasks do not conflict, preventing output errors. Therefore, when parsing the results, `$earliest_finish_time_ms` can be ignored.
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "empiredan (via GitHub)" <gi...@apache.org>.
empiredan commented on code in PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#discussion_r1952044350
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
Review Comment:
```suggestion
queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (1 in enqueue && enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
```
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
```suggestion
running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (1 in start && start[1] != "-") {count++}} END {print count}' "$query_log_file")
```
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
processing_count=$((queue_count+running_count))
- finish_count=`grep "last finish at" ${query_log_file} | grep -v "recent enqueue at" | grep -v "recent start at" | grep -o 'last finish at [^,]*' | sed 's/\[/,/;s/\]//' | awk -F"," -v date="$earliest_finish_time_ms" 'BEGIN{count=0}{if(length($2)==23 && $2>=date){count++;}}END{print count}'`
+ finish_count=$(awk 'BEGIN {count=0} {match($0, /"last_finish":"([^"]+)"/, finish); if (finish[1] != "-" && length(finish[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
Should `$earliest_finish_time_ms` also be considered ?
```suggestion
finish_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); {match($0, /"last_finish":"([^"]+)"/, finish); if (enqueue[1] == "-" && start[1] == "-" && 1 in finish && finish[1] != "-") {count++}} END {print count}' "$query_log_file")
```
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org
Re: [PR] fix(scripts): Fix the way the wait_manual_compact function parses the shell output [incubator-pegasus]
Posted by "limowang (via GitHub)" <gi...@apache.org>.
limowang commented on code in PR #2191:
URL: https://github.com/apache/incubator-pegasus/pull/2191#discussion_r1957523274
##########
admin_tools/pegasus_manual_compact.sh:
##########
@@ -119,10 +119,10 @@ function wait_manual_compact()
query_log_file="/tmp/$UID.$PID.pegasus.query_compact.${app_id}"
echo "${query_cmd}" | ./run.sh shell --cluster ${cluster} &>${query_log_file}
- queue_count=`grep 'recent enqueue at' ${query_log_file} | grep -v 'recent start at' | wc -l`
- running_count=`grep 'recent start at' ${query_log_file} | wc -l`
+ queue_count=$(awk 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); if (enqueue[1] != "-" && start[1] == "-") {count++}} END {print count}' "$query_log_file")
+ running_count=$(awk 'BEGIN {count=0} {match($0, /"recent_start_at":"([^"]+)"/, start); if (start[1] != "-" && length(start[1]) > 0) {count++}} END {print count}' "$query_log_file")
processing_count=$((queue_count+running_count))
- finish_count=`grep "last finish at" ${query_log_file} | grep -v "recent enqueue at" | grep -v "recent start at" | grep -o 'last finish at [^,]*' | sed 's/\[/,/;s/\]//' | awk -F"," -v date="$earliest_finish_time_ms" 'BEGIN{count=0}{if(length($2)==23 && $2>=date){count++;}}END{print count}'`
+ finish_count=$(awk 'BEGIN {count=0} {match($0, /"last_finish":"([^"]+)"/, finish); if (finish[1] != "-" && length(finish[1]) > 0) {count++}} END {print count}' "$query_log_file")
Review Comment:
The parameter _manual_compact_last_finish_time_ms will persist after the execution of manual_compact and will not be reset. If the value of ${trigger_time} is smaller than this value, the task will not be executed, which could result in incorrect task progress judgment. Therefore, the change is made to:
finish_count=$(awk -v date="$earliest_finish_time_ms" 'BEGIN {count=0} {match($0, /"recent_enqueue_at":"([^"]+)"/, enqueue); match($0, /"recent_start_at":"([^"]+)"/, start); match($0, /"last_finish":"([^"]+)"/, finish); if (enqueue[1] == "-" && start[1] == "-" && 1 in finish && finish[1] >= date) {count++}} END {print count}' "$query_log_file")
--
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: dev-unsubscribe@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org