diff --git a/pkg/controller/replication/switchover.go b/pkg/controller/replication/switchover.go index 6b7a6b97db..940ac0c349 100644 --- a/pkg/controller/replication/switchover.go +++ b/pkg/controller/replication/switchover.go @@ -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) @@ -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 diff --git a/pkg/controller/replication/switchover_test.go b/pkg/controller/replication/switchover_test.go index 44ad9d21e4..604f3abb87 100644 --- a/pkg/controller/replication/switchover_test.go +++ b/pkg/controller/replication/switchover_test.go @@ -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: @@ -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