From cdc24c10f0ba12a495ea583760cfef8705315056 Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Fri, 5 Sep 2025 16:08:43 +0300 Subject: [PATCH] PT-2289 - Allow pt-stalk do disable ps-lock-transactions data collection via parameter - Implemented _should_skip function as was suggested --- bin/pt-stalk | 55 +++++++++++++++++++++++++++----------------- lib/bash/collect.sh | 29 ++++++++++++++++------- t/pt-stalk/pt-2289.t | 19 +++++++++++++++ 3 files changed, 74 insertions(+), 29 deletions(-) diff --git a/bin/pt-stalk b/bin/pt-stalk index 790158a8..780e5a1f 100755 --- a/bin/pt-stalk +++ b/bin/pt-stalk @@ -971,7 +971,7 @@ collect_mysql_data_one() { fi fi - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "mysqladmin" ]]; then + if ! _should_skip "mysqladmin"; then $CMD_MYSQLADMIN $EXT_ARGV ext -i$OPT_SLEEP_COLLECT -c$cnt >>"$d/$p-mysqladmin" & mysqladmin_pid=$! fi @@ -1016,7 +1016,7 @@ collect_system_data() { collect_mysql_data_loop() { - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "processlist" ]]; then + if ! _should_skip "processlist"; then (echo $ts; $CMD_MYSQL $EXT_ARGV -e "SHOW FULL PROCESSLIST\G") \ >> "$d/$p-processlist" & fi @@ -1025,16 +1025,16 @@ collect_mysql_data_loop() { >> "$d/$p-threads" & if [ "$have_lock_waits_table" ]; then - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "lock-waits" ]]; then + if ! _should_skip "lock-waits"; then (echo $ts; lock_waits "$d/lock_waits.running") >>"$d/$p-lock-waits" & fi - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "transactions" ]]; then + if ! _should_skip "transactions"; then (echo $ts; transactions) >>"$d/$p-transactions" & fi fi if [ "${mysql_version}" '>' "5.6" ] && [ $ps_instrumentation_enabled == "yes" ] \ - && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "ps-locks-transactions" ]]; then + && ! _should_skip "ps-locks-transactions"; then ps_locks_transactions "$d/$p-ps-locks-transactions" fi @@ -1261,7 +1261,7 @@ innodb_status() { local innostat="" - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "innodbstatus" ]]; then + if ! _should_skip "innodbstatus"; then $CMD_MYSQL $EXT_ARGV -e "SHOW /*!40100 ENGINE*/ INNODB STATUS\G" \ >> "$d/$p-innodbstatus$n" grep "END OF INNODB" "$d/$p-innodbstatus$n" >/dev/null || { @@ -1285,7 +1285,7 @@ rocksdb_status() { has_rocksdb=`$CMD_MYSQL $EXT_ARGV -e "SHOW ENGINES" | grep -i 'rocksdb'` exit_code=$? - if [ $exit_code -eq 0 ] && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "rocksdbstatus" ]]; then + if [ $exit_code -eq 0 ] && ! _should_skip "rocksdbstatus"; then $CMD_MYSQL $EXT_ARGV -e "SHOW ENGINE ROCKSDB STATUS\G" \ >> "$d/$p-rocksdbstatus$n" || rm -f "$d/$p-rocksdbstatus$n" fi @@ -1367,7 +1367,7 @@ collect_mysql_variables() { echo -e "\n$sql\n" >> $outfile $CMD_MYSQL $EXT_ARGV -e "$sql" >> $outfile - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "thread-variables" ]]; then + if ! _should_skip "thread-variables"; then sql="select * from performance_schema.variables_by_thread order by thread_id, variable_name;" echo -e "\n$sql\n" >> $outfile $CMD_MYSQL $EXT_ARGV -e "$sql" >> $outfile @@ -1383,6 +1383,19 @@ collect_mysql_variables() { } +_should_skip() { + local name=$1 + + for item in "${OPT_SKIP_COLLECTION[@]}"; do + echo $item >> /tmp/sveta + if [ "$item" == "$name" ]; then + return 0 + fi + done + + return 1 +} + # ########################################################################### # End collect package # ########################################################################### @@ -1740,19 +1753,6 @@ main() { log 'Both options --system-only and --mysql-only specified, collecting only disk-space, hostname, output, and trigger metrics'; fi - if [ "$OPT_SKIP_COLLECTION" ]; then - local supported_skips=(ps-locks-transactions thread-variables innodbstatus lock-waits mysqladmin processlist rocksdbstatus transactions) - IFS=',' read -ra skips <<< "$OPT_SKIP_COLLECTION" - OPT_SKIP_COLLECTION=("${skips[*]}") - for skip in "${skips[@]}"; do - echo "$supported_skips" | grep -q "$skip" - if ! [[ " ${supported_skips[@]} " =~ " ${skip} " ]]; then - log "Invalid --skip-collection value: $skip, exiting." - exit 1 - fi - done - fi - # Note: $$ is the parent's PID, but we're a child proc. # Bash 4 has $BASHPID but we can't rely on that. Consequently, # we don't know our own PID. See the usage of $! below. @@ -1810,6 +1810,19 @@ if [ "${0##*/}" = "$TOOL" ] \ fi fi + if [ "$OPT_SKIP_COLLECTION" ]; then + local supported_skips=(ps-locks-transactions thread-variables innodbstatus lock-waits mysqladmin processlist rocksdbstatus transactions) + IFS=',' read -ra skips <<< "$OPT_SKIP_COLLECTION" + OPT_SKIP_COLLECTION=("${skips[@]}") + for skip in "${skips[@]}"; do + echo "$supported_skips" | grep -q "$skip" + if ! [[ " ${supported_skips[@]} " =~ " ${skip} " ]]; then + log "Invalid --skip-collection value: $skip, exiting." + exit 1 + fi + done + fi + if [ -z "$OPT_STALK" -a "$OPT_COLLECT" ]; then # Not stalking; do immediate collect once. OPT_CYCLES=0 diff --git a/lib/bash/collect.sh b/lib/bash/collect.sh index b7507815..00fd5005 100644 --- a/lib/bash/collect.sh +++ b/lib/bash/collect.sh @@ -243,7 +243,7 @@ collect_mysql_data_one() { # get and keep a connection to the database; in troubled times # the database tends to exceed max_connections, so reconnecting # in the loop tends not to work very well. - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "mysqladmin" ]]; then + if ! _should_skip "mysqladmin"; then $CMD_MYSQLADMIN $EXT_ARGV ext -i$OPT_SLEEP_COLLECT -c$cnt >>"$d/$p-mysqladmin" & mysqladmin_pid=$! fi @@ -291,7 +291,7 @@ collect_mysql_data_loop() { # SHOW FULL PROCESSLIST duplicates information in performance_schema.threads we collecting now # Keeping it for backward compatibility and may remove in the future - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "processlist" ]]; then + if ! _should_skip "processlist"; then (echo $ts; $CMD_MYSQL $EXT_ARGV -e "SHOW FULL PROCESSLIST\G") \ >> "$d/$p-processlist" & fi @@ -300,16 +300,16 @@ collect_mysql_data_loop() { >> "$d/$p-threads" & if [ "$have_lock_waits_table" ]; then - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "lock-waits" ]]; then + if ! _should_skip "lock-waits"; then (echo $ts; lock_waits "$d/lock_waits.running") >>"$d/$p-lock-waits" & fi - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "transactions" ]]; then + if ! _should_skip "transactions"; then (echo $ts; transactions) >>"$d/$p-transactions" & fi fi if [ "${mysql_version}" '>' "5.6" ] && [ $ps_instrumentation_enabled == "yes" ] \ - && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "ps-locks-transactions" ]]; then + && ! _should_skip "ps-locks-transactions"; then ps_locks_transactions "$d/$p-ps-locks-transactions" fi @@ -547,7 +547,7 @@ innodb_status() { local innostat="" - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "innodbstatus" ]]; then + if ! _should_skip "innodbstatus"; then $CMD_MYSQL $EXT_ARGV -e "SHOW /*!40100 ENGINE*/ INNODB STATUS\G" \ >> "$d/$p-innodbstatus$n" grep "END OF INNODB" "$d/$p-innodbstatus$n" >/dev/null || { @@ -571,7 +571,7 @@ rocksdb_status() { has_rocksdb=`$CMD_MYSQL $EXT_ARGV -e "SHOW ENGINES" | grep -i 'rocksdb'` exit_code=$? - if [ $exit_code -eq 0 ] && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "rocksdbstatus" ]]; then + if [ $exit_code -eq 0 ] && ! _should_skip "rocksdbstatus"; then $CMD_MYSQL $EXT_ARGV -e "SHOW ENGINE ROCKSDB STATUS\G" \ >> "$d/$p-rocksdbstatus$n" || rm -f "$d/$p-rocksdbstatus$n" fi @@ -657,7 +657,7 @@ collect_mysql_variables() { echo -e "\n$sql\n" >> $outfile $CMD_MYSQL $EXT_ARGV -e "$sql" >> $outfile - if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "thread-variables" ]]; then + if ! _should_skip "thread-variables"; then sql="select * from performance_schema.variables_by_thread order by thread_id, variable_name;" echo -e "\n$sql\n" >> $outfile $CMD_MYSQL $EXT_ARGV -e "$sql" >> $outfile @@ -673,6 +673,19 @@ collect_mysql_variables() { } +_should_skip() { + local name=$1 + + for item in "${OPT_SKIP_COLLECTION[@]}"; do + echo $item >> /tmp/sveta + if [ "$item" == "$name" ]; then + return 0 + fi + done + + return 1 +} + # ########################################################################### # End collect package # ########################################################################### diff --git a/t/pt-stalk/pt-2289.t b/t/pt-stalk/pt-2289.t index c560fc3a..f11c6155 100644 --- a/t/pt-stalk/pt-2289.t +++ b/t/pt-stalk/pt-2289.t @@ -232,6 +232,25 @@ like( "Rejects unsupported --skip-collection value" ); +cleanup(); + +$retval = system("$trunk/bin/pt-stalk --no-stalk --pid $pid_file --log $log_file --dest $dest --iterations 1 --skip-collection 'mysqladmin and' -- --defaults-file=$cnf >$log_file 2>&1"); + +sleep 5; +PerconaTest::kill_program(pid_file => $pid_file); + +is( + $retval >> 8, + 1, + "Parent exit 1 on unsupported --skip-collection value" +); + +like( + `cat $log_file`, + qr/Invalid --skip-collection value: mysqladmin and, exiting./, + "Rejects unsupported --skip-collection value" +); + # ############################################################################# # Done. # #############################################################################