Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions globals/globals.go
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,8 @@ var (
MinimumShowReplicaStatusVersion = NumericVersion{8, 0, 22}
MinimumChangeReplicationSourceVersion = NumericVersion{8, 0, 23}
MinimumShowBinaryLogStatusVersion = NumericVersion{8, 2, 0}
MinimumNoWriteSetExtractionVersion = NumericVersion{8, 3, 0}
MinimumResetBinaryLogsVersion = NumericVersion{8, 4, 0}
)

const (
Expand Down
24 changes: 19 additions & 5 deletions sandbox/group_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ func CreateGroupReplication(sandboxDef SandboxDef, origin string, nodes int, mas
data["MasterPasswordParam"] = replCmds["MasterPasswordParam"]
data["StartReplica"] = replCmds["StartReplica"]
data["StopReplica"] = replCmds["StopReplica"]
data["ResetMasterCmd"] = replCmds["ResetMasterCmd"]
connectionString := ""
for i := 0; i < nodes; i++ {
groupPort := baseGroupPort + i + 1
Expand Down Expand Up @@ -305,6 +306,7 @@ func CreateGroupReplication(sandboxDef SandboxDef, origin string, nodes int, mas
"ChangeMasterTo": replCmds["ChangeMasterTo"],
"MasterUserParam": replCmds["MasterUserParam"],
"MasterPasswordParam": replCmds["MasterPasswordParam"],
"ResetMasterCmd": replCmds["ResetMasterCmd"],
"MasterLabel": masterLabel,
"MasterAbbr": masterAbbr,
"SandboxDir": sandboxDef.SandboxDir,
Expand Down Expand Up @@ -332,11 +334,18 @@ func CreateGroupReplication(sandboxDef SandboxDef, origin string, nodes int, mas
}

basePortText := fmt.Sprintf("%08d", basePort)

// Version-aware options for group replication
useReplicaUpdates, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumShowReplicaStatusVersion)
useNoWriteSetExtraction, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumNoWriteSetExtractionVersion)

replicationData := common.StringMap{
"BasePort": basePortText,
"GroupSeeds": connectionString,
"LocalAddresses": fmt.Sprintf("%s:%d", masterIp, groupPort),
"PrimaryMode": singlePrimaryMode,
"BasePort": basePortText,
"GroupSeeds": connectionString,
"LocalAddresses": fmt.Sprintf("%s:%d", masterIp, groupPort),
"PrimaryMode": singlePrimaryMode,
"UseReplicaUpdates": useReplicaUpdates,
"SkipWriteSetExtraction": useNoWriteSetExtraction,
}

replOptionsText, err := common.SafeTemplateFill("group_replication",
Expand All @@ -350,7 +359,11 @@ func CreateGroupReplication(sandboxDef SandboxDef, origin string, nodes int, mas
sandboxDef.ReplOptions = reMasterIp.ReplaceAllString(sandboxDef.ReplOptions, masterIp)

sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplGtidOptions57].Contents)
sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplReplCrashSafeOptions].Contents)
// master-info-repository and relay-log-info-repository removed in 8.4+
skipCrashSafeOpts, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafeOpts {
sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplReplCrashSafeOptions].Contents)
}
Comment on lines +362 to +366
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinimumResetBinaryLogsVersion is being used here as the cutoff for skipping crash-safe replication options (because master-info-repository/relay-log-info-repository were removed in 8.4+). Since the constant name describes a different behavior (reset binary logs), this coupling makes future maintenance harder (e.g., if one cutoff changes independently). Consider introducing a dedicated version constant for the removal of crash-safe repository options, and use that here.

Copilot uses AI. Check for mistakes.

// 8.0.11
isMinimumMySQLXDefault, err := common.HasCapability(sandboxDef.Flavor, common.MySQLXDefault, sandboxDef.Version)
Expand Down Expand Up @@ -397,6 +410,7 @@ func CreateGroupReplication(sandboxDef SandboxDef, origin string, nodes int, mas
"ChangeMasterTo": replCmds["ChangeMasterTo"],
"MasterUserParam": replCmds["MasterUserParam"],
"MasterPasswordParam": replCmds["MasterPasswordParam"],
"ResetMasterCmd": replCmds["ResetMasterCmd"],
"SlaveLabel": slaveLabel,
"SlaveAbbr": slaveAbbr,
"SandboxDir": sandboxDef.SandboxDir,
Expand Down
38 changes: 14 additions & 24 deletions sandbox/multi-source-replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,10 @@ func CreateAllMastersReplication(sandboxDef SandboxDef, origin string, nodes int
}

sandboxDef.GtidOptions = SingleTemplates[globals.TmplGtidOptions57].Contents
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
skipCrashSafeOpts, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafeOpts {
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When skipCrashSafeOpts is true (8.4+), sandboxDef.ReplCrashSafeOptions is left untouched. Since SandboxDef can already carry crash-safe options (e.g., set by --gtid / --repl-crash-safe via fillSandboxDefinition), this can accidentally keep master-info-repository/relay-log-info-repository in the generated my.cnf and still break 8.4+ startup. Explicitly set sandboxDef.ReplCrashSafeOptions = "" in the skip path to avoid leaking preexisting options.

Suggested change
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
} else {
// For versions >= MinimumResetBinaryLogsVersion (e.g. 8.4+), ensure that any
// pre-existing crash-safe replication options are cleared so they are not
// written into my.cnf where they could break server startup.
sandboxDef.ReplCrashSafeOptions = ""

Copilot uses AI. Check for mistakes.
}
if sandboxDef.DirName == "" {
sandboxDef.DirName += defaults.Defaults().AllMastersPrefix + common.VersionToName(origin)
}
Expand Down Expand Up @@ -183,17 +186,9 @@ func CreateAllMastersReplication(sandboxDef SandboxDef, origin string, nodes int
data["NodeLabel"] = defaults.Defaults().NodePrefix
data["ChangeMasterExtra"] = setChangeMasterProperties("", sandboxDef.ChangeMasterOptions, logger)
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 replCmds {
data[k] = v
}
logger.Printf("Writing master and slave scripts in %s\n", sandboxDef.SandboxDir)
for _, node := range slaveList {
data["Node"] = node
Expand Down Expand Up @@ -330,7 +325,10 @@ func CreateFanInReplication(sandboxDef SandboxDef, origin string, nodes int, mas
slaveList = globals.SlaveListValue
}
sandboxDef.GtidOptions = SingleTemplates[globals.TmplGtidOptions57].Contents
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
skipCrashSafe2, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafe2 {
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as above: when skipCrashSafe2 is true (8.4+), sandboxDef.ReplCrashSafeOptions is not cleared, so any pre-populated crash-safe options may still be emitted into configs and trigger 8.4+ startup failures. Set sandboxDef.ReplCrashSafeOptions = "" in the skip branch to ensure removed options aren’t carried over.

Suggested change
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
} else {
sandboxDef.ReplCrashSafeOptions = ""

Copilot uses AI. Check for mistakes.
}
Comment on lines +328 to +331
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable name skipCrashSafe2 is a bit confusing. Since this variable is local to the CreateFanInReplication function, you could reuse the name skipCrashSafeOpts as used in CreateAllMastersReplication for consistency.

Suggested change
skipCrashSafe2, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafe2 {
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
}
skipCrashSafeOpts, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafeOpts {
sandboxDef.ReplCrashSafeOptions = SingleTemplates[globals.TmplReplCrashSafeOptions].Contents
}

if sandboxDef.DirName == "" {
sandboxDef.DirName = defaults.Defaults().FanInPrefix + common.VersionToName(origin)
}
Expand Down Expand Up @@ -420,17 +418,9 @@ func CreateFanInReplication(sandboxDef SandboxDef, origin string, nodes int, mas
data["ChangeMasterExtra"] = setChangeMasterProperties("", sandboxDef.ChangeMasterOptions, logger)
data["MasterIp"] = masterIp
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 replCmds {
data[k] = v
}
logger.Printf("Writing master and slave scripts in %s\n", sandboxDef.SandboxDir)
for _, slave := range slist {
data["Node"] = slave
Expand Down
13 changes: 11 additions & 2 deletions sandbox/pxc_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,12 @@ func CreatePxcReplication(sandboxDef SandboxDef, origin string, nodes int, maste
return err
}
if !skipLogSlaveUpdates {
sandboxDef.ReplOptions = fmt.Sprintf("%s\nlog_slave_updates=ON\n", sandboxDef.ReplOptions)
useReplicaUpdates, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumShowReplicaStatusVersion)
if useReplicaUpdates {
sandboxDef.ReplOptions = fmt.Sprintf("%s\nlog_replica_updates=ON\n", sandboxDef.ReplOptions)
} else {
sandboxDef.ReplOptions = fmt.Sprintf("%s\nlog_slave_updates=ON\n", sandboxDef.ReplOptions)
}
Comment on lines +228 to +233
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To improve maintainability and reduce code duplication, you can determine the option name first and then use it in a single fmt.Sprintf call.

Suggested change
useReplicaUpdates, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumShowReplicaStatusVersion)
if useReplicaUpdates {
sandboxDef.ReplOptions = fmt.Sprintf("%s\nlog_replica_updates=ON\n", sandboxDef.ReplOptions)
} else {
sandboxDef.ReplOptions = fmt.Sprintf("%s\nlog_slave_updates=ON\n", sandboxDef.ReplOptions)
}
useReplicaUpdates, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumShowReplicaStatusVersion)
optionName := "log_slave_updates"
if useReplicaUpdates {
optionName = "log_replica_updates"
}
sandboxDef.ReplOptions = fmt.Sprintf("%s\n%s=ON\n", sandboxDef.ReplOptions, optionName)

}
baseReplicationOptions := sandboxDef.ReplOptions
var groupCommunication string = "gcomm://"
Expand Down Expand Up @@ -322,7 +327,11 @@ func CreatePxcReplication(sandboxDef SandboxDef, origin string, nodes int, maste
sandboxDef.ReplOptions = baseReplicationOptions + fmt.Sprintf("\n%s\n", pxcFilledTemplate)

sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplGtidOptions57].Contents)
sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplReplCrashSafeOptions].Contents)
// master-info-repository and relay-log-info-repository removed in 8.4+
skipCrashSafeOpts, _ := common.GreaterOrEqualVersion(sandboxDef.Version, globals.MinimumResetBinaryLogsVersion)
if !skipCrashSafeOpts {
sandboxDef.ReplOptions += fmt.Sprintf("\n%s\n", SingleTemplates[globals.TmplReplCrashSafeOptions].Contents)
}
Comment on lines +330 to +334
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same maintainability concern: this uses MinimumResetBinaryLogsVersion as a proxy for when crash-safe repository options were removed. If these version thresholds diverge later, it’s easy for this check to become incorrect. Consider a dedicated constant for the crash-safe option removal threshold and use it here.

Copilot uses AI. Check for mistakes.
// 8.0.11
isMinimumMySQLXDefault, err := common.HasCapability(sandboxDef.Flavor, common.MySQLXDefault, sandboxDef.Version)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions sandbox/replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func replicationCommands(version string) map[string]string {
"StartReplica": "START SLAVE",
"StopReplica": "STOP SLAVE",
"ResetReplica": "RESET SLAVE",
"ResetMasterCmd": "reset master",
"MasterPosWaitFunc": "master_pos_wait",
"MasterHostParam": "master_host",
"MasterPortParam": "master_port",
Expand Down Expand Up @@ -67,6 +68,11 @@ func replicationCommands(version string) map[string]string {
cmds["ShowMasterStatus"] = "show binary log status"
}

useResetBinaryLogs, _ := common.GreaterOrEqualVersion(version, globals.MinimumResetBinaryLogsVersion)
if useResetBinaryLogs {
cmds["ResetMasterCmd"] = "RESET BINARY LOGS AND GTIDS"
}
Comment on lines +71 to +74
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This introduces a new version gate using common.GreaterOrEqualVersion, which is marked DEPRECATED in common/checks.go and returns false for MariaDB 10+ regardless of the comparison target. Also the returned error is ignored, so any unexpected version format would silently fall back to reset master (which is removed in MySQL 8.4+). Consider switching this new check to VersionToList + GreaterOrEqualVersionList (handling the error) or a flavor-based capability check, so ResetMasterCmd selection can’t silently regress.

Copilot uses AI. Check for mistakes.

return cmds
}

Expand Down Expand Up @@ -269,6 +275,7 @@ func CreateMasterSlaveReplication(sandboxDef SandboxDef, origin string, nodes in
"StartReplica": replCmds["StartReplica"],
"StopReplica": replCmds["StopReplica"],
"ResetReplica": replCmds["ResetReplica"],
"ResetMasterCmd": replCmds["ResetMasterCmd"],
"MasterPosWaitFunc": replCmds["MasterPosWaitFunc"],
"MasterHostParam": replCmds["MasterHostParam"],
"MasterPortParam": replCmds["MasterPortParam"],
Expand Down Expand Up @@ -376,6 +383,7 @@ func CreateMasterSlaveReplication(sandboxDef SandboxDef, origin string, nodes in
"MasterUserParam": replCmds["MasterUserParam"],
"MasterPasswordParam": replCmds["MasterPasswordParam"],
"MasterPosWaitFunc": replCmds["MasterPosWaitFunc"],
"ResetMasterCmd": replCmds["ResetMasterCmd"],
})
sandboxDef.LoadGrants = false
sandboxDef.Prompt = fmt.Sprintf("%s%d", slaveLabel, i)
Expand Down Expand Up @@ -446,6 +454,7 @@ func CreateMasterSlaveReplication(sandboxDef SandboxDef, origin string, nodes in
"MasterUserParam": replCmds["MasterUserParam"],
"MasterPasswordParam": replCmds["MasterPasswordParam"],
"MasterPosWaitFunc": replCmds["MasterPosWaitFunc"],
"ResetMasterCmd": replCmds["ResetMasterCmd"],
}
logger.Printf("Defining replication node data: %v\n", stringMapToJson(dataSlave))
logger.Printf("Create slave script %d\n", i)
Expand Down
4 changes: 2 additions & 2 deletions sandbox/templates/group/group_repl_options.gotxt
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@

# After customization, these options are added to my.sandbox.cnf
binlog_checksum=NONE
log_slave_updates=ON
{{if .UseReplicaUpdates}}log_replica_updates=ON{{else}}log_slave_updates=ON{{end}}
plugin-load-add=group_replication.so
group_replication=FORCE_PLUS_PERMANENT
group_replication_start_on_boot=OFF
group_replication_bootstrap_group=OFF
transaction_write_set_extraction=XXHASH64
{{if not .SkipWriteSetExtraction}}transaction_write_set_extraction=XXHASH64{{end}}
report-host=127.0.0.1
loose-group_replication_group_name="{{.BasePort}}-bbbb-cccc-dddd-eeeeeeeeeeee"
loose-group-replication-local-address={{.LocalAddresses}}
Expand Down
2 changes: 1 addition & 1 deletion sandbox/templates/group/init_nodes.gotxt
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ multi_sb={{.SandboxDir}}
{{end}}
[ -z "$SLEEP_TIME" ] && SLEEP_TIME=1
{{range .Nodes}}
user_cmd='reset master;'
user_cmd='{{.ResetMasterCmd}};'
user_cmd="$user_cmd {{.ChangeMasterTo}} {{.MasterUserParam}}='rsandbox', {{.MasterPasswordParam}}='rsandbox' {{.ChangeMasterExtra}} FOR CHANNEL 'group_replication_recovery';"
echo "# Node {{.Node}} # $user_cmd"
$multi_sb/{{.NodeLabel}}{{.Node}}/use -u root -e "$user_cmd"
Expand Down
2 changes: 1 addition & 1 deletion sandbox/templates/replication/multi_source.gotxt
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
SBDIR={{.SandboxDir}}
cd "$SBDIR"

$SBDIR/use_all 'reset master'
$SBDIR/use_all '{{.ResetMasterCmd}}'

MASTERS="{{.MasterList}}"
SLAVES="{{.SlaveList}}"
Expand Down
7 changes: 6 additions & 1 deletion sandbox/templates/single/clear.gotxt
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@ fi
is_master=$(ls data | grep 'mysql-bin')
if [ -n "$is_master" ]
then
./use -e 'reset master'
minimum_version_reset_binary_logs="008004000"
if [[ "v$sortable_version" > "v$minimum_version_reset_binary_logs" ]] || [[ "v$sortable_version" == "v$minimum_version_reset_binary_logs" ]]; then
./use -e 'RESET BINARY LOGS AND GTIDS'
else
./use -e 'reset master'
fi
Comment on lines +46 to +51
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This logic for choosing the reset master command can be improved for maintainability and readability.

  1. The hardcoded version 008004000 could become out of sync if the corresponding Go constant changes. It would be better to pass this value from the Go code into the template.
  2. The shell condition [[... > ...]] || [[... == ...]] can be simplified to >=.

Since changing the Go code to pass the version is out of scope for this file, I'll suggest simplifying the shell logic for now.

    minimum_version_reset_binary_logs="008004000"
    if [[ "v$sortable_version" >= "v$minimum_version_reset_binary_logs" ]]; then
        ./use -e 'RESET BINARY LOGS AND GTIDS'
    else
        ./use -e 'reset master'
    fi

Comment on lines +46 to +51
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sortable_version is computed only inside the earlier if [ -n "$(is_running)" ] block, but it’s used here to choose between RESET BINARY LOGS AND GTIDS and reset master. If the sandbox isn’t running (or is_running returns empty), sortable_version will be unset and this version check can select the wrong command (notably falling back to reset master on 8.4+). Consider initializing VERSION/sortable_version unconditionally near the top of the script (or computing it again in this block) before doing the comparison.

Copilot uses AI. Check for mistakes.
fi

./stop
Expand Down
Loading