Fixes for PXB python test scripts by tplavcic · Pull Request #16 · Percona-QA/server-qa

tplavcic · 2026-11-03T15:40:14Z

No description provided.

Copilot

🟡 Changes recommended

The binlog replay helper can let tests pass without verification because it ignores replay command failures and skips comparisons when required replay metadata is missing.

2 open issues

What changed in this PR

This PR updates the Percona XtraBackup Python test suite to improve restore verification and compatibility with upstream MySQL.

Changes:

Shares binlog replay across verification paths and makes failed table checks fail tests.
Disables MyRocks tests on upstream MySQL and omits the legacy log-file-size option for server versions 9.3+.
Corrects documentation for RocksDB defaults and test skips.

File	Description
`test_scripts/pxb/test_helper.py`	Extracts binlog replay and updates restore integrity checks.
`test_scripts/pxb/README.md`	Clarifies RocksDB defaults and upstream MySQL restrictions.
`test_scripts/pxb/innodb_myrocks_backup_tests.py`	Adds MyRocks guards and version-dependent backup options.

_{Review effort: Balanced}

prepare-incremental-from-dir

Copilot

🟡 Changes recommended

Unresolved verification defects can produce false mismatches during concurrent writes, fail RocksDB backup tests, and report success even when restored tables are missing.

3 open issues

2 resolved since last review

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Binlog replay omits files created after rotation, risking false restore failures, and checksum parsing rejects valid eight-column output from supported older releases.

1 open issue

Replay all binlog files written after the backup coordinate · New

1 previously missed issue

In code that hasn't changed since last review

Accept legacy eight-column pt-table-checksum output

test_scripts/pxb/test_helper.py:1751

Please also accept the eight-column output used by older pt-table-checksum releases, including 3.0.9, which check_pt_checksum() explicitly permits for MySQL 8.0. That format has no DIFF_ROWS column, so valid table rows are discarded here and the new empty-output safeguard fails the test before restore. Accept both layouts and read TABLE from the final column.

3 resolved since last review

_{Review effort: Balanced}

mchawla16

Thanks Tomislav for the fixes. I ran the tests with MS9.7.2 and PS9.7.2, adding some more suggestions.

Scenario: Run pxb tests with MS9.7.2, rocksdb is disabled by default

export TEST_BASE_DIR=$HOME/innodb_myrocks_backup_tests
export XTRABACKUP_DIR=$HOME/percona-xtrabackup-9.7.2-2-Linux-x86_64.glibc2.35/bin
export MYSQLDIR=$HOME/Mysql-9.7.2-copy
export QASCRIPTS=$HOME/server-qa
export ENABLE_CORE_DUMP=1

python innodb_myrocks_backup_tests.py -v Various_ddl_tests

innodb_myrocks_backup_tests.py::test_add_drop_index RocksDB is unsupported in MS, disabling RocksDB for this test

###################################################################################
# Test: test_add_drop_index
###################################################################################
=>Creating data directory for 'primary'
..Data directory '/home/manish.chawla/innodb_myrocks_backup_tests/data_add-drop-index_20261105_045148' created
=>Starting MySQL server 'primary' on port 21000
..Server 'primary' started successfully
Test is running against: MS-9.7.2
Run sysbench on database=test time=20
DDL: Add and drop index on test.sbtest1
=>Taking full backup
..Full backup was successfully created at: /home/manish.chawla/innodb_myrocks_backup_tests/dbbackup_add-drop-index_20261105_045148/full.
  Logs available at: /home/manish.chawla/innodb_myrocks_backup_tests/backuplogs_add-drop-index_20261105_045148/full_backup_20261105_045206_log
=>Taking incremental backup: 1
..Inc backup was successfully created at: /home/manish.chawla/innodb_myrocks_backup_tests/dbbackup_add-drop-index_20261105_045148/inc1.
  Logs available at: /home/manish.chawla/innodb_myrocks_backup_tests/backuplogs_add-drop-index_20261105_045148/inc1_backup_20261105_045206_log
Waiting for DDL threads to finish: ddl_ddl_add_drop_index
=>Preparing full backup
..Prepare of full backup was successful.
  Logs available at: /home/manish.chawla/innodb_myrocks_backup_tests/backuplogs_add-drop-index_20261105_045148/prepare_full_backup_20261105_045206_log
=>Preparing incremental backup: 1
..Prepare of incremental backup was successful.
  Logs available at: /home/manish.chawla/innodb_myrocks_backup_tests/backuplogs_add-drop-index_20261105_045148/prepare_inc1_backup_20261105_045206_log
Collecting existing table count
Stopping mysql server and moving data directory
..Server 'primary' shut down cleanly
=>Restoring full backup
..Restore of full backup was successful.
  Logs available at: /home/manish.chawla/innodb_myrocks_backup_tests/backuplogs_add-drop-index_20261105_045148/res_backup_20261105_045206_log
=>Starting MySQL server 'primary' on port 21000
..Server 'primary' started successfully
Check xtrabackup for binlog position
Xtrabackup binlog position: binlog.000004, 23979
Applying binlog to restored data starting from binlog.000004, 23979
Collecting table count after restore
Data is the same before and after restore: Pass
PASSED

It mentions that Waiting for DDL threads to finish: ddl_ddl_add_drop_index. Is it possible to remove the extra ddl word here?
All table checks are not being run after restore. These checks should be added
a) Check tables for corruption. This is the bash code

    for ((i=1; i<=${num_tables}; i++)); do
        for database in ${database_list}; do
            if ! table_status=$(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "CHECK TABLE $database.sbtest$i"|cut -f4-); then
                echo "ERR: CHECK TABLE $database.sbtest$i query failed"
                # Check if database went down
                if ! ${mysqldir}/bin/mysqladmin ping --user=root --socket=${mysqldir}/socket.sock >/dev/null 2>&1; then
                    echo "ERR: The database has gone down due to corruption in table $database.sbtest$i"
                    exit 1
                fi
                check_err=1
            fi

            if [[ "$table_status" != "OK" ]]; then
                echo "ERR: CHECK TABLE $database.sbtest$i query displayed the table status as '$table_status'"
                check_err=1
            fi
        done
    done

    # Check if database went down
    if ! ${mysqldir}/bin/mysqladmin ping --user=root --socket=${mysqldir}/socket.sock >/dev/null 2>&1; then
        echo "ERR: The database has gone down due to corruption, the restore was unsuccessful"
        exit 1
    fi

    if [[ "$check_err" -eq 0 ]]; then
        echo "All innodb and myrocks tables status: OK"
    else
        echo "After restore, some tables may be corrupt, check table status is not OK"
    fi

b) Check record count of each table, it should be same before and after restore. The bash code for this is

    echo "Check the record count of tables in databases: ${database_list}"
    # Get record count for each table in databases test and test_rocksdb
    rc_err=0
    checksum_err=0
    for ((i=1; i<=${num_tables}; i++)); do
        rc_innodb_res[$i]=$(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "SELECT COUNT(*) FROM test.sbtest$i;")
        if [[ "${rc_innodb_orig[$i]}" -ne "${rc_innodb_res[$i]}" ]]; then
            echo "ERR: The record count of test.sbtest$i changed after restore. Record count in original data: ${rc_innodb_orig[$i]}. Record count in restored data: ${rc_innodb_res[$i]}."
            rc_err=1
        fi

        if [[ "${rocksdb}" = "enabled" ]] && [[ "${MYSQLD_OPTIONS}" != *"keyring"* ]]; then
            rc_myrocks_res[$i]=$(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "SELECT COUNT(*) FROM test_rocksdb.sbtest$i;")
            if [[ "${rc_myrocks_orig[$i]}" -ne "${rc_myrocks_res[$i]}" ]]; then
                echo "ERR: The record count of test_rocksdb.sbtest$i changed after restore. Record count in original data: ${rc_myrocks_orig[$i]}. Record count in restored data: ${rc_myrocks_res[$i]}."
                rc_err=1
            fi
        fi
    done
    if [[ "$rc_err" -eq 0 ]]; then
        echo "Match record count of tables in databases ${database_list} with original data: Pass"
    fi

c) Check checksum of each table, it should be same before and after restore. The bash code for this is

    echo "Check the checksum of each table in databases: ${database_list}"
    # Get checksum of each table in databases test and test_rocksdb
    for ((i=1; i<=${num_tables}; i++)); do
        chk_innodb_res[$i]=$(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "CHECKSUM TABLE test.sbtest$i;"|awk '{print $2}')
        if [[ "${chk_innodb_orig[$i]}" -ne "${chk_innodb_res[$i]}" ]]; then
            echo "ERR: The checksum of test.sbtest$i changed after restore. Checksum in original data: ${chk_innodb_orig[$i]}. Checksum in restored data: ${chk_innodb_res[$i]}."
            checksum_err=1;
        fi

        if [[ "${rocksdb}" = "enabled" ]] && [[ "${MYSQLD_OPTIONS}" != *"keyring"* ]]; then
            chk_myrocks_res[$i]=$(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "CHECKSUM TABLE test_rocksdb.sbtest$i;"|awk '{print $2}')
            if [[ "${chk_myrocks_orig[$i]}" -ne "${chk_myrocks_res[$i]}" ]]; then
                echo "ERR: The checksum of test_rocksdb.sbtest$i changed after restore. Checksum in original data: ${chk_myrocks_orig[$i]}. Checksum in restored data: ${chk_myrocks_res[$i]}."
                checksum_err=1;
            fi
        fi
    done

    if [[ "$checksum_err" -eq 0 ]]; then
        echo "Match checksum of all tables in databases ${database_list} with original data: Pass"
    fi

d) Check for gaps in primary sequence id of tables. This is optional, since the above checks should be sufficient

    echo "Check for gaps in primary sequence id of tables"
    gap_found=0
    #for database in test test_rocksdb; do
    for database in ${database_list}; do
        for ((i=1; i<=${num_tables}; i++)); do
            j=1
            while read line; do
                if [[ "$line" != "$j" ]]; then
                    echo "ERR: Gap found in $database.sbtest$i. Expected sequence number for ID is: $j. Actual sequence number for ID is: $line."
                    gap_found=1
                    return
                fi
                let j++
            done < <(${mysqldir}/bin/mysql -uroot -S${mysqldir}/socket.sock -Bse "SELECT id FROM $database.sbtest$i ORDER BY id ASC")
        done
    done

    if [[ "$gap_found" -eq 0 ]]; then
        echo "No gaps found in primary sequence id of tables: Pass"
    fi
}

We skip record count and checksum comparison, in case binlog is encrypted and binlogs can't be applied to the restore, but tables are always checked for corruption.

Scenario: Explicit rocksdb disabled/enabled (export ROCKSDB=disabled/enabled)
Each test pauses when the test is started and prints all the output after the test is completed, so it doesn't display how the test is proceeding.

tplavcic · 2026-11-05T11:25:12Z

@mchawla16 It should be fixed now.

mchawla16 · 2026-11-05T11:49:48Z

Thanks Tomislav, I found some more issues in the meanwhile. In the tests test_inc-backup-archive and test_inc_backup_innodb_params, if rocksdb is enabled, then the rocksdb data doesn't get created. I think during initialization CREATE DATABASE test_rocksdb; needs to be added.
These are the errors coming in sysbench logs:

backuplogs_inc-backup-archive-log_20261105_102111/sysbench_test_rocksdb.log:FATAL: error 1049: Unknown database 'test_rocksdb'
backuplogs_inc-backup-archive-log_20261105_102111/sysbench_test_rocksdb.log:FATAL: `thread_init' function failed: /usr/share/sysbench/oltp_common.lua:356: connection creation failed

backuplogs_inc-backup-innodb-params_20261105_100557/sysbench_test_rocksdb.log:FATAL: error 1049: Unknown database 'test_rocksdb'
backuplogs_inc-backup-innodb-params_20261105_100557/sysbench_test_rocksdb.log:FATAL: `thread_init' function failed: /usr/share/sysbench/oltp_common.lua:356: connection creation failed

tplavcic · 2026-11-05T13:58:45Z

@mchawla16 The issue with the RocksDB database creation is now fixed as well.

tplavcic added 3 commits November 3, 2026 15:23

Disable RocksDB on upstream MySQL

0a10720

Fix innodb-log-file-size option for MySQL 9.2+

7f5e7a3

Fix checks behaviour in PXB scripts to match old bash scripts

6d1e445

tplavcic requested a balanced review from Copilot November 3, 2026 15:40

Copilot started reviewing on behalf of tplavcic November 3, 2026 15:41

View session

Copilot AI reviewed Nov 3, 2026

View reviewed changes

Comment thread test_scripts/pxb/test_helper.py Resolved

Comment thread test_scripts/pxb/test_helper.py Resolved

tplavcic requested a review from mchawla16 November 4, 2026 09:55

tplavcic added 6 commits November 4, 2026 11:20

Fix issue when missing backup binlog data skips verification

236b8da

Check mysqlbinlog exit status

7da0ed5

Fix verification when rocksdb used

82afb7d

Use sysbench as default load tool in innodb_myrocks_backup_tests.py

d040751

Add a test separator in innodb_myrocks_backup_tests.py

401f24b

Use new options apply-redo-only, backup-incremental-base and

1e1c4de

prepare-incremental-from-dir

tplavcic requested a balanced review from Copilot November 4, 2026 12:06

Copilot started reviewing on behalf of tplavcic November 4, 2026 12:06

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread test_scripts/pxb/test_helper.py Resolved

Comment thread test_scripts/pxb/test_helper.py Resolved

Comment thread test_scripts/pxb/test_helper.py Resolved

tplavcic added 4 commits November 4, 2026 13:13

Add progress and result lines into innodb_myrocks_backup_tests.py

336db8b

Wait for writers to finish before recording pre-restore baseline

5a07e4e

Check if server has any rocksdb tables

d47b512

Fix comparison for tables before and after restore

d48e317

tplavcic requested a balanced review from Copilot November 4, 2026 12:45

Copilot started reviewing on behalf of tplavcic November 4, 2026 12:45

View session

Copilot AI reviewed Nov 4, 2026

View reviewed changes

Comment thread test_scripts/pxb/test_helper.py Resolved

mchawla16 reviewed Nov 5, 2026

View reviewed changes

tplavcic added 5 commits November 5, 2026 12:05

Remove ddl_ duplication in run_ddl_in_background

4ca4c99

Apply table check after every restore, whichever path is taken

3e914f3

Make take_backup() always use per-table verification

2faaccb

Add primary key gap check

d00b1c3

Print test progress as it happens

bcabe21

tplavcic requested a review from mchawla16 November 5, 2026 11:24

tplavcic added 2 commits November 5, 2026 13:41

Move wait for ddl to finish after backup prepare

d4088c6

Fix creation of RocksDB database

0b65f6c

tplavcic merged commit 99d9e21 into main Nov 5, 2026
2 checks passed

tplavcic deleted the fix-pxb-test-upstream branch November 5, 2026 14:06

Navigation Menu

Repository navigation

Fixes for PXB python test scripts - #16

Fixes for PXB python test scripts#16
tplavcic merged 20 commits into
mainfrom
fix-pxb-test-upstream

tplavcic commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

mchawla16 left a comment

Uh oh!

tplavcic commented Nov 5, 2026

Uh oh!

mchawla16 commented Nov 5, 2026

Uh oh!

tplavcic commented Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tplavcic commented Nov 3, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

mchawla16 left a comment

Choose a reason for hiding this comment

Uh oh!

tplavcic commented Nov 5, 2026

Uh oh!

mchawla16 commented Nov 5, 2026

Uh oh!

tplavcic commented Nov 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants