Improved error handling in replication status checks to accommodate changes in MySQL versions - #440
Open
saikumar-vs wants to merge 6 commits into
Open
Improved error handling in replication status checks to accommodate changes in MySQL versions#440saikumar-vs wants to merge 6 commits into
saikumar-vs wants to merge 6 commits into
Conversation
…CA terminology handling. Updated database creation logic to ensure the database exists before queries are executed. Improved error handling in replication status checks to accommodate changes in MySQL versions.
…A terminology across various components. Updated replication commands and status checks to ensure compatibility with MySQL versions 8.0.23 and above. Improved error reporting and streamlined code for better maintainability.
There was a problem hiding this comment.
Pull request overview
This PR updates the replication test harness to handle MySQL/Percona’s MASTER/SLAVE → SOURCE/REPLICA terminology changes (and MySQL 8.4+ removals of legacy syntax), while also improving replication-status error reporting and fixing a replication-mode startup gap in runall-new.pl.
Changes:
- Introduce
GenTest::ReplicationTermsand wire it through replication setup, reporters, validators, and executor helpers so the correct replication vocabulary is selected by server version. - Improve replication status checks by switching
SHOW SLAVE/REPLICA STATUSparsing from positional indices to named columns, and by guarding against missing/disconnected replica DB handles. - Ensure the test database is created in the replication server-start branch in
runall-new.plto avoid “Unknown database” failures.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| randgen/runall.pl | Uses version-dependent replication terminology for replication setup (STOP/CHANGE/START). |
| randgen/runall-new.pl | Creates the test database on master in the replication-start path so initial connections/gendata don’t fail. |
| randgen/lib/GenTest/Validator/ReplicationWaitForSlave.pm | Uses version-dependent pos-wait function and status statement; improves error extraction. |
| randgen/lib/GenTest/Validator/ReplicationSlaveStatus.pm | Switches to hashref-based status parsing by column name; adds replica connection guardrails. |
| randgen/lib/GenTest/Reporter/ReplicationSemiSync.pm | Adapts semisync reporter to version-dependent replication commands and wait function. |
| randgen/lib/GenTest/Reporter/ReplicationConsistency.pm | Uses version-dependent replica discovery / start / pos-wait and binlog status. |
| randgen/lib/GenTest/Reporter/CloneSlaveXtrabackup.pm | Updates clone reporter replication setup/start/wait to version-dependent terminology. |
| randgen/lib/GenTest/Reporter/CloneSlave.pm | Updates clone reporter replication setup/start/wait to version-dependent terminology. |
| randgen/lib/GenTest/Reporter.pm | Uses version-dependent replica discovery (SHOW REPLICAS vs SHOW SLAVE HOSTS). |
| randgen/lib/GenTest/ReplicationTerms.pm | Adds centralized mapping of legacy vs modern replication statements/identifiers keyed off version string. |
| randgen/lib/GenTest/Executor/MySQL.pm | Adds executor wrappers for replication terminology selection; updates slave/binlog status helpers. |
| randgen/lib/DBServer/MySQL/ReplMySQLd.pm | Stores per-pair replication terms; hardens wait/stop paths against already-stopped servers. |
| randgen/conf/replication/README.md | Documents replication test usage, version compatibility, and known gotchas. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…nsistency.pm to ensure proper execution of final binlog status function.
ParveezBaig
reviewed
Aug 28, 2026
ParveezBaig
left a comment
ParveezBaig
left a comment
There was a problem hiding this comment.
Hi Sai,
Thanks for the patch. It looks good. It's better to rename the functions, variables, filenames etc to reflect the new terminology. Have highlighted some here.
Thanks,
…gy across validators, reporters, the MySQL/Drizzle executors, and the DBServer replication helper, addressing PR review feedback from Parveez Baig. Left the file and package names of ReplicationSlaveStatus, ReplicationWaitForSlave, CloneSlave, and CloneSlaveXtrabackup unchanged since they are CLI-facing --validators/--reporter identity strings; renamed their internals (variables, ReplMySQLd's master()/slave() accessors and constructor keyword args, and the shared serverInfo slave_host/slave_port keys) instead. Wired ReplicationSemiSync's semi-sync SET GLOBAL/SHOW STATUS calls through the version-aware replication term table instead of hardcoding rpl_semi_sync_master_*/rpl_semi_sync_slave_* names, fixing semi-sync testing on MySQL 8.0.23+ where those legacy variable names no longer exist. Gave SHOW BINARY LOG STATUS its own, later version boundary (8.2.0) separate from the rest of the SOURCE/REPLICA rollout (8.0.23), since it is rejected as a syntax error on 8.0.x servers even though CHANGE REPLICATION SOURCE TO, SHOW REPLICA STATUS, and SOURCE_POS_WAIT() are already accepted there -- confirmed live against Percona Server 8.0.46. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… mysqld process running as an orphan after stopServer() believed it had already shut down. startServer() execs a single command string containing I/O redirection, so Perl invokes it via "/bin/sh -c ...". On shells that fork a child for the redirected command instead of exec-replacing themselves (confirmed with dash), the real mysqld process ends up a grandchild rather than a direct child of the RQG process. waitpid(), which only tracks direct children, then always returns ECHILD for it regardless of whether it is still running, so stopServer()'s wait loop gave up immediately without ever escalating to SIGTERM/SIGKILL. running() now checks for the process directly via kill(0, $pid), which does not depend on the parent/child relationship. Confirmed live that this is not specific to replication -- it affects any mysqld started this way whose shutdown happens to be slow enough to outrun the false "already stopped" detection; it was mainly noticed on replication's source server because a broken binlog-dump connection to an already-stopped replica tends to slow its shutdown down. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
Author
Thanks Parveez for the review and feedback. I have updated the function names the changes had to be made wider since the reference were many old names, but didnt change much on Module/Filenames for now as they some links to the documentations and further defaults with Core code. Also there was one bug fixed which was not part of the patch, but surfaced during testing rpl. Its regressed so it should be fine to be part of this patch. |
…both. ReplicationThreadRestarter still built its random STOP/START command as literal SLAVE text instead of going through the version-aware term table, so it was a syntax error on servers where the legacy terminology has been removed. Added a replica_keyword term (SLAVE/REPLICA) to ReplicationTerms for reporters that need to build an arbitrary thread-clause combination themselves rather than using the existing whole-statement terms, and wired this reporter through it. Confirmed live against the exact servers named in the review comment that the new query text is accepted. Also confirmed live, on those same servers, that the position-wait function needs its own version boundary separate from the rest of the SOURCE/REPLICA rollout, the same way the binlog-status query already does -- the modern function name isn't recognized yet on some point releases where the rest of the modern vocabulary already works, while the legacy name still is. Gated it accordingly. Extended conf/replication/README.md with a general note on why individual terms get their own boundary rather than sharing the overall cutover, and a section on ReplicationThreadRestarter's deliberately adversarial interaction with thread-status validators. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://perconadev.atlassian.net/browse/PS-11528
perl runall-new.pl
--basedir=/home/saikumar/WORKDIR/PS/DEV/percona-server/bld/install
--vardir1=/tmp/rqg_var_rpl_master
--vardir2=/tmp/rqg_var_rpl_slave
--rpl_mode=row
--grammar=conf/replication/replication.yy
--gendata=conf/replication/replication_single_engine.zz
--threads=2
--queries=500
--duration=150
--validators=ReplicationSlaveStatus
--reporter=Backtrace,ErrorLog,QueryTimeout
--sqltrace