PT-2289 - Allow pt-stalk do disable ps-lock-transactions data collection via parameter

- Adjusted the implementation
- Created test cases

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

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

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.