Fix MySQL 8.4+/9.x replication compatibility#21
Conversation
…ANGE REPLICATION SOURCE TO) Use version-aware SQL commands for replication setup and monitoring: - SHOW MASTER STATUS -> SHOW BINARY LOG STATUS (8.2+) - SHOW SLAVE STATUS -> SHOW REPLICA STATUS (8.0.22+) - CHANGE MASTER TO -> CHANGE REPLICATION SOURCE TO (8.0.23+) - START/STOP/RESET SLAVE -> START/STOP/RESET REPLICA (8.0.22+) - master_pos_wait() -> source_pos_wait() (8.0.22+) - GET_MASTER_PUBLIC_KEY -> GET_SOURCE_PUBLIC_KEY (8.0.23+)
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the MySQL sandbox environment's compatibility with modern MySQL versions (8.4+ and 9.x) by implementing version-aware replication command handling. It ensures that the sandbox can correctly configure and manage replication using the appropriate SQL syntax for each MySQL version, thereby broadening the range of supported environments without breaking existing functionality for older versions. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses MySQL 8.4+ and 9.x replication compatibility by introducing version-aware SQL commands. The use of a helper function replicationCommands to centralize the logic is a good approach, and the template files are correctly updated. My main feedback is regarding code duplication when populating the data maps for templates. I've left a couple of suggestions to improve maintainability by using a loop, which would make the code more concise and easier to manage.
| replCmds := replicationCommands(sandboxDef.Version) | ||
| data["ShowMasterStatus"] = replCmds["ShowMasterStatus"] | ||
| data["ShowSlaveStatus"] = replCmds["ShowSlaveStatus"] | ||
| data["ChangeMasterTo"] = replCmds["ChangeMasterTo"] | ||
| data["StartReplica"] = replCmds["StartReplica"] | ||
| data["StopReplica"] = replCmds["StopReplica"] | ||
| data["ResetReplica"] = replCmds["ResetReplica"] | ||
| data["MasterPosWaitFunc"] = replCmds["MasterPosWaitFunc"] | ||
| data["MasterHostParam"] = replCmds["MasterHostParam"] | ||
| data["MasterPortParam"] = replCmds["MasterPortParam"] | ||
| data["MasterUserParam"] = replCmds["MasterUserParam"] | ||
| data["MasterPasswordParam"] = replCmds["MasterPasswordParam"] |
There was a problem hiding this comment.
This block of code populates replication commands into the data map. It is repeated in CreateFanInReplication and with variations in sandbox/replication.go. To improve maintainability and reduce duplication, you could use a loop. This would also make it easier to add new commands in the future.
| replCmds := replicationCommands(sandboxDef.Version) | |
| data["ShowMasterStatus"] = replCmds["ShowMasterStatus"] | |
| data["ShowSlaveStatus"] = replCmds["ShowSlaveStatus"] | |
| data["ChangeMasterTo"] = replCmds["ChangeMasterTo"] | |
| data["StartReplica"] = replCmds["StartReplica"] | |
| data["StopReplica"] = replCmds["StopReplica"] | |
| data["ResetReplica"] = replCmds["ResetReplica"] | |
| data["MasterPosWaitFunc"] = replCmds["MasterPosWaitFunc"] | |
| data["MasterHostParam"] = replCmds["MasterHostParam"] | |
| data["MasterPortParam"] = replCmds["MasterPortParam"] | |
| data["MasterUserParam"] = replCmds["MasterUserParam"] | |
| data["MasterPasswordParam"] = replCmds["MasterPasswordParam"] | |
| for k, v := range replicationCommands(sandboxDef.Version) { | |
| data[k] = v | |
| } |
| replCmds := replicationCommands(sandboxDef.Version) | ||
| data["ShowMasterStatus"] = replCmds["ShowMasterStatus"] | ||
| data["ShowSlaveStatus"] = replCmds["ShowSlaveStatus"] | ||
| data["ChangeMasterTo"] = replCmds["ChangeMasterTo"] | ||
| data["StartReplica"] = replCmds["StartReplica"] | ||
| data["StopReplica"] = replCmds["StopReplica"] | ||
| data["ResetReplica"] = replCmds["ResetReplica"] | ||
| data["MasterPosWaitFunc"] = replCmds["MasterPosWaitFunc"] | ||
| data["MasterHostParam"] = replCmds["MasterHostParam"] | ||
| data["MasterPortParam"] = replCmds["MasterPortParam"] | ||
| data["MasterUserParam"] = replCmds["MasterUserParam"] | ||
| data["MasterPasswordParam"] = replCmds["MasterPasswordParam"] |
There was a problem hiding this comment.
Similar to the CreateAllMastersReplication function, this block can be simplified using a loop to reduce code duplication and improve maintainability.
| replCmds := replicationCommands(sandboxDef.Version) | |
| data["ShowMasterStatus"] = replCmds["ShowMasterStatus"] | |
| data["ShowSlaveStatus"] = replCmds["ShowSlaveStatus"] | |
| data["ChangeMasterTo"] = replCmds["ChangeMasterTo"] | |
| data["StartReplica"] = replCmds["StartReplica"] | |
| data["StopReplica"] = replCmds["StopReplica"] | |
| data["ResetReplica"] = replCmds["ResetReplica"] | |
| data["MasterPosWaitFunc"] = replCmds["MasterPosWaitFunc"] | |
| data["MasterHostParam"] = replCmds["MasterHostParam"] | |
| data["MasterPortParam"] = replCmds["MasterPortParam"] | |
| data["MasterUserParam"] = replCmds["MasterUserParam"] | |
| data["MasterPasswordParam"] = replCmds["MasterPasswordParam"] | |
| for k, v := range replicationCommands(sandboxDef.Version) { | |
| data[k] = v | |
| } |
There was a problem hiding this comment.
Pull request overview
Updates dbdeployer’s replication sandbox generation to use MySQL-version-appropriate replication SQL syntax, restoring compatibility with MySQL 8.4+ / 9.x where several legacy commands have been removed/renamed.
Changes:
- Introduces a
replicationCommands()helper to select replication SQL statements/parameter names based on server version. - Propagates version-aware command/parameter strings into replication and multi-source replication template data.
- Updates replication shell templates to use the templatized commands/functions.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
globals/globals.go |
Adds minimum-version thresholds for renamed replication statements. |
sandbox/replication.go |
Adds replicationCommands() and injects version-aware command strings into template data; updates public key option for new syntax. |
sandbox/multi-source-replication.go |
Injects version-aware command strings into multi-source template data. |
sandbox/templates/replication/test_replication.gotxt |
Uses templated SHOW/pos-wait commands and handles Replica_* status fields. |
sandbox/templates/replication/multi_source.gotxt |
Uses templated CHANGE MASTER/SOURCE and START SLAVE/REPLICA plus renamed parameters. |
sandbox/templates/replication/init_slaves.gotxt |
Uses templated CHANGE MASTER/SOURCE and START SLAVE/REPLICA plus renamed parameters. |
sandbox/templates/replication/check_slaves.gotxt |
Uses templated SHOW commands and expands grep patterns for Source_* fields. |
sandbox/templates/replication/check_multi_source.gotxt |
Uses templated SHOW commands and expands grep patterns for Source_* fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| server_id=$($SBDIR/{{.NodeLabel}}$S/use -BN -e "show variables like 'server_id'") | ||
| echo "$port - $server_id" | ||
| $SBDIR/{{.NodeLabel}}$S/use -e 'show slave status\G' | grep "\(Running:\|Master_Log_Pos\|\<Master_Log_File\|Retrieved\|Channel\|Executed\)" | ||
| $SBDIR/{{.NodeLabel}}$S/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|\<Master_Log_File|\<Source_Log_File|Retrieved|Channel|Executed)" |
There was a problem hiding this comment.
grep -E + \< is not portable (BSD/macOS grep doesn’t support \< word-boundary anchors in extended regex). This may prevent matching Master_Log_File/Source_Log_File and hide important status lines. Prefer a portable ERE such as (^|[[:space:]])(Master_Log_File|Source_Log_File): or avoid \< entirely and anchor another way.
| $SBDIR/{{.NodeLabel}}$S/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|\<Master_Log_File|\<Source_Log_File|Retrieved|Channel|Executed)" | |
| $SBDIR/{{.NodeLabel}}$S/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|Master_Log_File:|Source_Log_File:|Retrieved|Channel|Executed)" |
| useReplicaStatus, _ := common.GreaterOrEqualVersion(version, globals.MinimumShowReplicaStatusVersion) | ||
| if useReplicaStatus { | ||
| cmds["ShowSlaveStatus"] = "show replica status" | ||
| cmds["StartReplica"] = "START REPLICA" | ||
| cmds["StopReplica"] = "STOP REPLICA" | ||
| cmds["ResetReplica"] = "RESET REPLICA" | ||
| cmds["MasterPosWaitFunc"] = "source_pos_wait" | ||
| } | ||
|
|
||
| useChangeSource, _ := common.GreaterOrEqualVersion(version, globals.MinimumChangeReplicationSourceVersion) | ||
| if useChangeSource { | ||
| cmds["ChangeMasterTo"] = "CHANGE REPLICATION SOURCE TO" | ||
| cmds["MasterHostParam"] = "source_host" | ||
| cmds["MasterPortParam"] = "source_port" | ||
| cmds["MasterUserParam"] = "source_user" | ||
| cmds["MasterPasswordParam"] = "source_password" | ||
| } | ||
|
|
||
| useBinaryLogStatus, _ := common.GreaterOrEqualVersion(version, globals.MinimumShowBinaryLogStatusVersion) | ||
| if useBinaryLogStatus { | ||
| cmds["ShowMasterStatus"] = "show binary log status" | ||
| } | ||
|
|
There was a problem hiding this comment.
common.GreaterOrEqualVersion is marked DEPRECATED in common/checks.go and it returns errors for non-x.y.z version strings. Here the error is ignored (_, _ := ...), so any unexpected version format silently falls back to old replication syntax (breaking the MySQL 8.4+/9.x compatibility this PR targets). Consider switching to the capability system (common.HasCapability) by adding new feature flags, or parse once with common.VersionToList and compare with GreaterOrEqualVersionList, and handle/log parse errors explicitly.
| useReplicaStatus, _ := common.GreaterOrEqualVersion(version, globals.MinimumShowReplicaStatusVersion) | |
| if useReplicaStatus { | |
| cmds["ShowSlaveStatus"] = "show replica status" | |
| cmds["StartReplica"] = "START REPLICA" | |
| cmds["StopReplica"] = "STOP REPLICA" | |
| cmds["ResetReplica"] = "RESET REPLICA" | |
| cmds["MasterPosWaitFunc"] = "source_pos_wait" | |
| } | |
| useChangeSource, _ := common.GreaterOrEqualVersion(version, globals.MinimumChangeReplicationSourceVersion) | |
| if useChangeSource { | |
| cmds["ChangeMasterTo"] = "CHANGE REPLICATION SOURCE TO" | |
| cmds["MasterHostParam"] = "source_host" | |
| cmds["MasterPortParam"] = "source_port" | |
| cmds["MasterUserParam"] = "source_user" | |
| cmds["MasterPasswordParam"] = "source_password" | |
| } | |
| useBinaryLogStatus, _ := common.GreaterOrEqualVersion(version, globals.MinimumShowBinaryLogStatusVersion) | |
| if useBinaryLogStatus { | |
| cmds["ShowMasterStatus"] = "show binary log status" | |
| } | |
| versionList, err := common.VersionToList(version) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "warning: unable to parse MySQL version %q: %v; using legacy replication commands\n", version, err) | |
| return cmds | |
| } | |
| minShowReplicaStatusVersionList, err := common.VersionToList(globals.MinimumShowReplicaStatusVersion) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "warning: unable to parse minimum show replica status version %q: %v; leaving slave status commands unchanged\n", globals.MinimumShowReplicaStatusVersion, err) | |
| } else { | |
| useReplicaStatus := common.GreaterOrEqualVersionList(versionList, minShowReplicaStatusVersionList) | |
| if useReplicaStatus { | |
| cmds["ShowSlaveStatus"] = "show replica status" | |
| cmds["StartReplica"] = "START REPLICA" | |
| cmds["StopReplica"] = "STOP REPLICA" | |
| cmds["ResetReplica"] = "RESET REPLICA" | |
| cmds["MasterPosWaitFunc"] = "source_pos_wait" | |
| } | |
| } | |
| minChangeSourceVersionList, err := common.VersionToList(globals.MinimumChangeReplicationSourceVersion) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "warning: unable to parse minimum change replication source version %q: %v; leaving CHANGE MASTER TO command unchanged\n", globals.MinimumChangeReplicationSourceVersion, err) | |
| } else { | |
| useChangeSource := common.GreaterOrEqualVersionList(versionList, minChangeSourceVersionList) | |
| if useChangeSource { | |
| cmds["ChangeMasterTo"] = "CHANGE REPLICATION SOURCE TO" | |
| cmds["MasterHostParam"] = "source_host" | |
| cmds["MasterPortParam"] = "source_port" | |
| cmds["MasterUserParam"] = "source_user" | |
| cmds["MasterPasswordParam"] = "source_password" | |
| } | |
| } | |
| minBinaryLogStatusVersionList, err := common.VersionToList(globals.MinimumShowBinaryLogStatusVersion) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "warning: unable to parse minimum show binary log status version %q: %v; leaving master status command unchanged\n", globals.MinimumShowBinaryLogStatusVersion, err) | |
| } else { | |
| useBinaryLogStatus := common.GreaterOrEqualVersionList(versionList, minBinaryLogStatusVersionList) | |
| if useBinaryLogStatus { | |
| cmds["ShowMasterStatus"] = "show binary log status" | |
| } | |
| } |
| useNewSourceSyntax, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumChangeReplicationSourceVersion) | ||
| if useNewSourceSyntax { | ||
| sandboxDef.ChangeMasterOptions = append(sandboxDef.ChangeMasterOptions, "GET_SOURCE_PUBLIC_KEY=1") | ||
| } else { | ||
| sandboxDef.ChangeMasterOptions = append(sandboxDef.ChangeMasterOptions, "GET_MASTER_PUBLIC_KEY=1") |
There was a problem hiding this comment.
This uses common.GreaterOrEqualVersion (deprecated) and ignores the parse error. If sandboxDef.Version isn’t strictly x.y.z (e.g., includes a prefix), this will silently choose GET_MASTER_PUBLIC_KEY even when the new CHANGE REPLICATION SOURCE TO syntax is active, which can break authentication on newer MySQL. Prefer common.HasCapability (new feature flag) or compare parsed version lists and handle the error explicitly.
| # master_pos_wait can return 0 or a positive number for successful replication | ||
| # Any result that is not NULL or -1 is acceptable | ||
| if [ "$S_READY" != "-1" -a "$S_READY" != "NULL" ] |
There was a problem hiding this comment.
The inline comment still refers to master_pos_wait, but on MySQL 8.0.22+ the template now calls {{.MasterPosWaitFunc}} (which can be source_pos_wait). Update the comment to avoid misleading readers/debugging output when the newer function is in use.
| # master_pos_wait can return 0 or a positive number for successful replication | |
| # Any result that is not NULL or -1 is acceptable | |
| if [ "$S_READY" != "-1" -a "$S_READY" != "NULL" ] | |
| # The position wait function ({{.MasterPosWaitFunc}}) can return 0 or a positive number for successful replication | |
| # Any result that is not NULL or -1 is acceptable | |
| if [ "$S_READY" != "-1" -a "$SREADY" != "NULL" ] |
| server_id=$($SBDIR/{{.NodeLabel}}{{.Node}}/use -BN -e "show variables like 'server_id'") | ||
| echo "$port - $server_id" | ||
| $SBDIR/{{.NodeLabel}}{{.Node}}/use -e 'show slave status\G' | grep "\(Running:\|Master_Log_Pos\|\<Master_Log_File\|Retrieved\|Executed\|Auto_Position\)" | ||
| $SBDIR/{{.NodeLabel}}{{.Node}}/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|\<Master_Log_File|\<Source_Log_File|Retrieved|Executed|Auto_Position)" |
There was a problem hiding this comment.
grep -E treats \< differently across platforms (notably BSD/macOS grep, which doesn’t support \< word-boundary anchors in ERE). With -E this can end up looking for a literal < and fail to match Master_Log_File/Source_Log_File, making the script miss key replication fields. Use a portable pattern (e.g., match (^|[[:space:]])(Master_Log_File|Source_Log_File):), or drop \< and rely on -w/anchoring.
| $SBDIR/{{.NodeLabel}}{{.Node}}/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|\<Master_Log_File|\<Source_Log_File|Retrieved|Executed|Auto_Position)" | |
| $SBDIR/{{.NodeLabel}}{{.Node}}/use -e '{{.ShowSlaveStatus}}\G' | grep -E "(Running:|Master_Log_Pos|Source_Log_Pos|Master_Log_File:|Source_Log_File:|Retrieved|Executed|Auto_Position)" |
Summary
Fixes MySQL 8.4+ and 9.x replication sandbox compatibility by using version-appropriate SQL commands.
Changes
SHOW MASTER STATUS→SHOW BINARY LOG STATUS(8.2+)SHOW SLAVE STATUS→SHOW REPLICA STATUS(8.0.22+)CHANGE MASTER TO→CHANGE REPLICATION SOURCE TOwith renamed params (8.0.23+)START/STOP SLAVE→START/STOP REPLICA(8.0.22+)master_pos_wait()→source_pos_wait()(8.0.22+)GET_MASTER_PUBLIC_KEY→GET_SOURCE_PUBLIC_KEY(8.0.23+)Files changed
globals/globals.go— version threshold constantssandbox/replication.go—replicationCommands()helper, version-aware data mapssandbox/multi-source-replication.go— same for fan-in/all-masters topologiesTest results
check_slaves✅,test_replication10/10 ✅check_slaves✅,test_replication10/10 ✅Closes #20