From 66d20ae6da0e257f1eba33f9269dcdc49de7187d Mon Sep 17 00:00:00 2001 From: Sveta Smirnova Date: Wed, 3 Sep 2025 00:41:06 +0300 Subject: [PATCH] PT-2289 - Allow pt-stalk do disable ps-lock-transactions data collection via parameter - Adjusted the implementation - Created test cases --- bin/pt-stalk | 21 +++--- lib/bash/collect.sh | 16 ++--- t/pt-stalk/pt-2289.t | 166 ++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 184 insertions(+), 19 deletions(-) diff --git a/bin/pt-stalk b/bin/pt-stalk index c3faf5a1..790158a8 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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "lock-waits" ]]; then (echo $ts; lock_waits "$d/lock_waits.running") >>"$d/$p-lock-waits" & fi - if ! [ "${OPT_SKIP_COLLECTION}" =~ "transactions" ]; then + if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 + && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ] && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 @@ -1741,11 +1741,12 @@ main() { fi if [ "$OPT_SKIP_COLLECTION" ]; then - local supported_skips="ps-locks-transactions,thread-variables,innodbstatus,lock-waits,mysqladmin,processlist,rocksdbstatus,transactions" + 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 [ $? -ne 0 ]; then + if ! [[ " ${supported_skips[@]} " =~ " ${skip} " ]]; then log "Invalid --skip-collection value: $skip, exiting." exit 1 fi diff --git a/lib/bash/collect.sh b/lib/bash/collect.sh index 6648616d..b7507815 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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "lock-waits" ]]; then (echo $ts; lock_waits "$d/lock_waits.running") >>"$d/$p-lock-waits" & fi - if ! [ "${OPT_SKIP_COLLECTION}" =~ "transactions" ]; then + if ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 + && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ] && ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 ! [[ "${OPT_SKIP_COLLECTION[@]}" =~ "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 diff --git a/t/pt-stalk/pt-2289.t b/t/pt-stalk/pt-2289.t index f9313ddd..c560fc3a 100644 --- a/t/pt-stalk/pt-2289.t +++ b/t/pt-stalk/pt-2289.t @@ -46,9 +46,38 @@ sub cleanup { cleanup(); +# We need these to collect lock-waits +sub start_thread_pt_1897_1 { + # this must run in a thread because we need to have an active session + # with open transaction + my ($dsn_opts) = @_; + my $dp = new DSNParser(opts=>$dsn_opts); + my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); + my $dbh = $sb->get_dbh_for('source'); + $sb->load_file('source', "t/pt-stalk/samples/PT-1897-1.sql"); +} +my $thr1 = threads->create('start_thread_pt_1897_1', $dsn_opts); +$thr1->detach(); +threads->yield(); +sleep 1; + +sub start_thread_pt_1897_2 { + # this must run in a thread because we need to have an active session + # with waiting transaction + my ($dsn_opts) = @_; + my $dp = new DSNParser(opts=>$dsn_opts); + my $sb = new Sandbox(basedir => '/tmp', DSNParser => $dp); + my $dbh = $sb->get_dbh_for('source'); + $sb->load_file('source', "t/pt-stalk/samples/PT-1897-2.sql"); +} +my $thr2 = threads->create('start_thread_pt_1897_2', $dsn_opts); +$thr2->detach(); +threads->yield(); + $retval = system("$trunk/bin/pt-stalk --no-stalk --pid $pid_file --log $log_file --dest $dest --iterations 1 -- --defaults-file=$cnf >$log_file 2>&1"); -PerconaTest::wait_until(sub { !-f $pid_file }); +sleep 35; +PerconaTest::kill_program(pid_file => $pid_file); is( $retval >> 8, @@ -67,6 +96,141 @@ ok( "Collects *-ps-locks-transactions" ) or diag(`ls $dest`); +ok( + glob("$dest/*-innodbstatus*"), + "Collects *-innodbstatus*" +) or diag(`ls $dest`); + +ok( + glob("$dest/*-lock-waits"), + "Collects *-lock-waits" +) or diag(`ls $dest`); + +ok( + glob("$dest/*-mysqladmin"), + "Collects *-mysqladmin" +) or diag(`ls $dest`); + +ok( + glob("$dest/*-processlist"), + "Collects *-processlist" +) or diag(`ls $dest`); + +ok( + glob("$dest/*-transactions"), + "Collects *-transactions" +) or diag(`ls $dest`); + +# thread-variables +ok( + glob("$dest/*-variables"), + "Collects *-variables" +) or diag(`ls $dest`); + +$output = `cat $dest/*-variables 2>/dev/null`; +like( + $output, + qr/select \* from performance_schema\.variables_by_thread/, + "Thread variables collected" +); # or diag($output); + +SKIP: { + skip "These tests require MyRocks", 1 if ( !$sb->has_engine('source', 'ROCKSDB') ) ; + + # rocksdbstatus + ok( + glob("$dest/*-rocksdbstatus*"), + "Collects *-rocksdbstatus" + ) or diag(`ls $dest`); + + cleanup(); + + $retval = system("$trunk/bin/pt-stalk --no-stalk --pid $pid_file --log $log_file --dest $dest --iterations 1 --skip-collection rocksdbstatus -- --defaults-file=$cnf >$log_file 2>&1"); + + sleep 5; + PerconaTest::kill_program(pid_file => $pid_file); + + ok( + ! glob("$dest/*-rocksdbstatus*"), + "Does not collect *-rocksdbstatus" + ) or diag(`ls $dest`); +} + +cleanup(); + +$retval = system("$trunk/bin/pt-stalk --no-stalk --pid $pid_file --log $log_file --dest $dest --iterations 1 --skip-collection processlist -- --defaults-file=$cnf >$log_file 2>&1"); + +sleep 5; +PerconaTest::kill_program(pid_file => $pid_file); + +ok( + ! glob("$dest/*-processlist"), + "Does not collect *-processlist" +) or diag(`ls $dest`); + +cleanup(); + +$retval = system("$trunk/bin/pt-stalk --no-stalk --pid $pid_file --log $log_file --dest $dest --iterations 1 --skip-collection ps-locks-transactions,thread-variables,innodbstatus,mysqladmin,processlist,transactions -- --defaults-file=$cnf >$log_file 2>&1"); + +sleep 5; +PerconaTest::kill_program(pid_file => $pid_file); + +ok( + ! glob("$dest/*-ps-locks-transactions"), + "Does not collect *-ps-locks-transactions" +) or diag(`ls $dest`); + +ok( + glob("$dest/*-variables"), + "Collects *-variables" +) or diag(`ls $dest`); + +$output = `cat $dest/*-variables 2>/dev/null`; +unlike( + $output, + qr/select \* from performance_schema\.variables_by_thread/, + "Thread variables not collected" +); # or diag($output); + +ok( + ! glob("$dest/*-innodbstatus"), + "Does not collect *-innodbstatus" +) or diag(`ls $dest`); + +ok( + ! glob("$dest/*-mysqladmin"), + "Does not collect *-mysqladmin" +) or diag(`ls $dest`); + +ok( + ! glob("$dest/*-processlist"), + "Does not collect *-processlist" +) or diag(`ls $dest`); + +ok( + ! glob("$dest/*-transactions"), + "Does not collect *-transactions" +) or diag(`ls $dest`); + +#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 ps-locks-transactions,thread-variables,innodbstatus,mysqladmin,processlist,transaction -- --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: transaction, exiting./, + "Rejects unsupported --skip-collection value" +); # ############################################################################# # Done.