Skip to content

fix: refuse switchover when target replica has fatal replication error #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
xcompass merged 1 commit into from
May 13, 2026
Merged

fix: refuse switchover when target replica has fatal replication error #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
85 changes: 85 additions & 0 deletions pkg/controller/replication/switchover.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,76 @@ func shouldReconcileSwitchover(mdb *mariadbv1alpha1.MariaDB) bool {
return mdb.IsReplicationSwitchoverRequired()
}

// targetReplicaError returns a non-nil error describing a persistent replication
// failure on the switchover target replica, observed from MariaDB.Status. The
// caller should refuse to start (or continue) the switchover when this returns
// an error: locking the primary read-only and entering waitForReplicaSync against
// a replica whose SQL or IO thread is broken just produces a timeout loop while
// leaving the primary degraded. Reading from cached status (rather than a live
// SQL query) means this also works when the target's mariadb container is in
// CrashLoopBackOff — the wedge mode where the live check would itself fail with
// "invalid connection".
//
// Returns nil when the status is missing, the target is the current primary
// (nothing to check), or the target's last observed status had errno == 0.
func targetReplicaError(mdb *mariadbv1alpha1.MariaDB) error {
if mdb.Status.Replication == nil || mdb.Status.Replication.Replicas == nil {
return nil
}
replication := ptr.Deref(mdb.Spec.Replication, mariadbv1alpha1.Replication{})
if replication.Primary.PodIndex == nil {
return nil
}
target := *replication.Primary.PodIndex
if mdb.Status.CurrentPrimaryPodIndex != nil && *mdb.Status.CurrentPrimaryPodIndex == target {
return nil
}
podName := statefulset.PodName(mdb.ObjectMeta, target)
status, ok := mdb.Status.Replication.Replicas[podName]
if !ok {
return nil
}
if errno := ptr.Deref(status.LastSQLErrno, 0); errno != 0 {
msg := ptr.Deref(status.LastSQLError, "")
return fmt.Errorf(
"switchover target replica '%s' has unrecoverable SQL thread error (errno %d): %s; "+
"recover the replica before retrying the switchover",
podName, errno, msg,
)
}
if errno := ptr.Deref(status.LastIOErrno, 0); errno != 0 {
msg := ptr.Deref(status.LastIOError, "")
return fmt.Errorf(
"switchover target replica '%s' has unrecoverable IO thread error (errno %d): %s; "+
"recover the replica before retrying the switchover",
podName, errno, msg,
)
}
// Catch the no-errno-but-thread-stopped case: a replica whose SQL thread
// was halted by `STOP SLAVE SQL_THREAD` (or by `slave_skip_errors` after
// a prior incident) reports Slave_SQL_Running=No with Last_SQL_Errno=0.
// gtid_current_pos still cannot advance, so MASTER_GTID_WAIT in
// waitForReplicaSync would still time out indefinitely. Note: SlaveSQLRunning
// and SlaveIORunning are nil on a replica that has never been configured —
// we default to true (`ptr.Deref(..., true)`) to avoid blocking the very
// first switchover before status has been populated.
if !ptr.Deref(status.SlaveSQLRunning, true) {
return fmt.Errorf(
"switchover target replica '%s' SQL thread is not running (Slave_SQL_Running=No); "+
"recover the replica before retrying the switchover",
podName,
)
}
if !ptr.Deref(status.SlaveIORunning, true) {
return fmt.Errorf(
"switchover target replica '%s' IO thread is not running (Slave_IO_Running=No); "+
"recover the replica before retrying the switchover",
podName,
)
}
return nil
}

func (r *ReplicationReconciler) reconcileSwitchover(ctx context.Context, req *ReconcileRequest, switchoverLogger logr.Logger) error {
logger := switchoverLogger.WithValues("mariadb", req.mariadb.Name)

Expand All @@ -56,6 +126,21 @@ func (r *ReplicationReconciler) reconcileSwitchover(ctx context.Context, req *Re
return nil
}

// Refuse to start the switchover if the target replica's last-observed
// replication state shows a persistent SQL or IO thread error. Without
// this guard, lockPrimaryWithReadLock / setPrimaryReadOnly run first
// (degrading the still-healthy current primary to read-only), and then
// waitForReplicaSync loops on MASTER_GTID_WAIT timeouts forever — because
// a replica whose SQL thread has aborted can never advance its
// gtid_current_pos, regardless of how long we wait. Field incidents on
// moodle-education-{stg,prod}-db hit this exact pattern with errno 1062
// on mdl_task_log, leaving both clusters wedged for 5+ days.
if err := targetReplicaError(req.mariadb); err != nil {
r.recorder.Eventf(req.mariadb, nil, corev1.EventTypeWarning, mariadbv1alpha1.ReasonReplicationReplicaSyncErr,
mariadbv1alpha1.ActionReconciling, "Refusing switchover: %v", err)
return err
}

replication := ptr.Deref(req.mariadb.Spec.Replication, mariadbv1alpha1.Replication{})
primary := req.mariadb.Status.CurrentPrimaryPodIndex
newPrimary := *replication.Primary.PodIndex
Expand Down
275 changes: 275 additions & 0 deletions pkg/controller/replication/switchover_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ package replication

import (
"context"
"strings"
"testing"

"github.com/go-logr/logr"
mariadbv1alpha1 "github.com/mariadb-operator/mariadb-operator/v26/api/v1alpha1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
)

// TestConfigureReplicaOpts_HardFailover_ResetsGtid documents the bug:
Expand Down Expand Up @@ -188,6 +191,278 @@ func TestCurrentPrimaryReady_ToleratesTransientConnectBlips(_ *testing.T) {
// See pkg/controller/replication/switchover.go: currentPrimaryReady.
}

// TestTargetReplicaError_GuardsAgainstWedgedTarget covers the regression closed
// by reconcileSwitchover's early-bail check. Field incidents on both
// moodle-education-stg-db and moodle-education-prod-db wedged for 5+ days with
// the same shape: spec.replication.primary.podIndex pointed at a replica whose
// SQL thread had aborted with errno 1062 (Duplicate entry on mdl_task_log),
// status.replication.replicas[target].lastSQLErrno persisted across reconciles,
// and the switchover state machine repeatedly locked the still-healthy current
// primary read-only and then timed out in MASTER_GTID_WAIT — because a replica
// with a dead SQL thread cannot advance gtid_current_pos, no matter how long
// we wait.
//
// targetReplicaError reads the cached MariaDB.Status (not a live SQL query) so
// the guard also fires when the target's mariadb container is in CrashLoopBackOff
// (the exact wedge mode where live SQL would itself fail with "invalid
// connection" and obscure the real cause).
func TestTargetReplicaError_GuardsAgainstWedgedTarget(t *testing.T) {
t.Parallel()
mdbObjectMeta := metav1.ObjectMeta{Name: "moodle-education-stg-db"}
primaryIndex0 := 0
primaryIndex1 := 1

tests := []struct {
name string
mdb *mariadbv1alpha1.MariaDB
wantErr bool
wantMatch string
}{
{
name: "nil replication status returns nil",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
},
},
},
{
name: "target == current primary returns nil even if other replicas are errored",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex0}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(1062),
LastSQLError: ptr.To("Duplicate entry"),
},
},
},
},
},
},
},
{
name: "healthy target returns nil",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(0),
LastIOErrno: ptr.To(0),
},
},
},
},
},
},
},
{
name: "target with SQL errno 1062 returns actionable error",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(1062),
LastSQLError: ptr.To("Duplicate entry '71306391' for key 'PRIMARY'"),
},
},
},
},
},
},
wantErr: true,
wantMatch: "unrecoverable SQL thread error (errno 1062)",
},
{
name: "target with IO errno 1236 returns actionable error",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastIOErrno: ptr.To(1236),
LastIOError: ptr.To("Got fatal error 1236 from master"),
},
},
},
},
},
},
wantErr: true,
wantMatch: "unrecoverable IO thread error (errno 1236)",
},
{
name: "target with SlaveSQLRunning=false but errno=0 returns actionable error",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(0),
LastIOErrno: ptr.To(0),
SlaveIORunning: ptr.To(true),
SlaveSQLRunning: ptr.To(false),
},
},
},
},
},
},
wantErr: true,
wantMatch: "SQL thread is not running",
},
{
name: "target with SlaveIORunning=false but errno=0 returns actionable error",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(0),
LastIOErrno: ptr.To(0),
SlaveIORunning: ptr.To(false),
SlaveSQLRunning: ptr.To(true),
},
},
},
},
},
},
wantErr: true,
wantMatch: "IO thread is not running",
},
{
name: "nil thread-running fields default to true (uninitialized replica)",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{
"moodle-education-stg-db-1": {
ReplicaStatusVars: mariadbv1alpha1.ReplicaStatusVars{
LastSQLErrno: ptr.To(0),
LastIOErrno: ptr.To(0),
// SlaveIORunning and SlaveSQLRunning unset
},
},
},
},
},
},
},
{
name: "target absent from status map returns nil (no observation yet)",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{ReplicationSpec: mariadbv1alpha1.ReplicationSpec{
Primary: mariadbv1alpha1.PrimaryReplication{PodIndex: &primaryIndex1}},
},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
Replication: &mariadbv1alpha1.ReplicationStatus{
Replicas: map[string]mariadbv1alpha1.ReplicaStatus{},
},
},
},
},
{
name: "nil PodIndex in spec returns nil (operator hasn't defaulted yet)",
mdb: &mariadbv1alpha1.MariaDB{
ObjectMeta: mdbObjectMeta,
Spec: mariadbv1alpha1.MariaDBSpec{
Replication: &mariadbv1alpha1.Replication{},
},
Status: mariadbv1alpha1.MariaDBStatus{
CurrentPrimaryPodIndex: &primaryIndex0,
},
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
err := targetReplicaError(tc.mdb)
if tc.wantErr {
if err == nil {
t.Fatalf("expected error containing %q, got nil", tc.wantMatch)
}
if !strings.Contains(err.Error(), tc.wantMatch) {
t.Fatalf("expected error containing %q, got %q", tc.wantMatch, err.Error())
}
return
}
if err != nil {
t.Fatalf("expected nil, got %v", err)
}
})
}
}

// TestReplicationReconcile_RefreshesRolesBeforeSwitchover documents the bug:
//
// Replication.Reconcile previously short-circuited directly to reconcileSwitchover when
Expand Down
Loading