diff --git a/.github/workflows/functional.yml b/.github/workflows/functional.yml index 809083a8..f6e0b47f 100644 --- a/.github/workflows/functional.yml +++ b/.github/workflows/functional.yml @@ -278,7 +278,6 @@ jobs: - name: Run PostgreSQL tests run: bash tests/functional/test-postgresql.sh - continue-on-error: true # PG support is new — iterating on CI stability - name: Collect orchestrator logs if: always() diff --git a/docs/database-providers.md b/docs/database-providers.md index dc53adc9..4a17ba16 100644 --- a/docs/database-providers.md +++ b/docs/database-providers.md @@ -180,8 +180,14 @@ GRANT pg_monitor TO orchestrator; - **`client_port` from `pg_stat_replication`** is an ephemeral port, not the PostgreSQL listen port. Orchestrator uses `DefaultInstancePort` for standby discovery, so all instances must listen on the same port. -- **Graceful master takeover** (planned switchover) is not yet implemented for - PostgreSQL. Only unplanned failover (dead primary) is supported. +- **Graceful master takeover** (planned switchover) is supported for PostgreSQL. + The same CLI commands and API endpoints (`graceful-master-takeover`, + `graceful-master-takeover-auto`) work for PostgreSQL clusters. The switchover + sets the primary read-only, waits for the designated standby to catch up via + WAL LSN, promotes the standby, reconfigures remaining standbys, and configures + the demoted primary's `primary_conninfo`. The demoted primary requires an + operator-managed restart with `standby.signal` — use + `PostGracefulTakeoverProcesses` hooks to automate this step. ## Implementing a New Provider diff --git a/docs/superpowers/plans/2026-04-18-postgresql-graceful-switchover.md b/docs/superpowers/plans/2026-04-18-postgresql-graceful-switchover.md new file mode 100644 index 00000000..284e8d69 --- /dev/null +++ b/docs/superpowers/plans/2026-04-18-postgresql-graceful-switchover.md @@ -0,0 +1,799 @@ +# PostgreSQL Graceful Primary Switchover — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add planned/graceful primary switchover for PostgreSQL, reusing the same CLI/API surface as MySQL's graceful master takeover. + +**Architecture:** New PostgreSQL-specific instance operations (`SetReadOnly`, `GetCurrentWALLSN`, `WaitForStandbyLSN`, `RepositionAsStandby`) in `go/inst/instance_topology_postgresql.go`, a new orchestration function `PostgreSQLGracefulPrimarySwitchover` in `go/logic/topology_recovery_postgresql.go`, and a provider dispatch in the existing `GracefulMasterTakeover` in `go/logic/topology_recovery.go`. + +**Tech Stack:** Go, PostgreSQL system functions (`pg_current_wal_lsn`, `pg_last_wal_replay_lsn`, `pg_is_in_recovery`, `pg_stat_activity`, `pg_terminate_backend`, `pg_reload_conf`), `database/sql` + +**Spec:** `docs/superpowers/specs/2026-04-18-postgresql-graceful-switchover-design.md` + +--- + +### Task 1: `PostgreSQLSetReadOnly` — Unit Test + Implementation + +**Files:** +- Modify: `go/inst/instance_topology_postgresql.go` (append new function after line 181) +- Create: `go/inst/instance_topology_postgresql_test.go` + +- [ ] **Step 1: Write the failing test** + +Create `go/inst/instance_topology_postgresql_test.go`: + +```go +/* + Copyright 2024 Orchestrator Authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package inst + +import ( + "testing" +) + +func TestPostgreSQLSetReadOnlyNilKey(t *testing.T) { + _, err := PostgreSQLSetReadOnly(nil, true) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./go/inst/... -run TestPostgreSQLSetReadOnlyNilKey -v` +Expected: FAIL — `PostgreSQLSetReadOnly` is not defined. + +- [ ] **Step 3: Write minimal implementation** + +Append to `go/inst/instance_topology_postgresql.go` (after line 181, before the closing of the file): + +```go +// PostgreSQLSetReadOnly sets or unsets read-only mode on a PostgreSQL instance +// by updating default_transaction_read_only via ALTER SYSTEM and reloading. +// When setting read-only, it also terminates existing non-replication connections +// to prevent stray writes during graceful switchover. +func PostgreSQLSetReadOnly(instanceKey *InstanceKey, readOnly bool) (*Instance, error) { + if instanceKey == nil { + return nil, fmt.Errorf("PostgreSQLSetReadOnly: nil instanceKey") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return nil, log.Errore(err) + } + defer func() { _ = db.Close() }() + + readOnlyValue := "off" + if readOnly { + readOnlyValue = "on" + } + + log.Infof("PostgreSQLSetReadOnly: setting default_transaction_read_only = %s on %+v", readOnlyValue, *instanceKey) + + if _, err := db.Exec(fmt.Sprintf("ALTER SYSTEM SET default_transaction_read_only = %s", readOnlyValue)); err != nil { + return nil, log.Errore(fmt.Errorf("PostgreSQLSetReadOnly: ALTER SYSTEM failed on %+v: %v", *instanceKey, err)) + } + + if _, err := db.Exec("SELECT pg_reload_conf()"); err != nil { + return nil, log.Errore(fmt.Errorf("PostgreSQLSetReadOnly: pg_reload_conf() failed on %+v: %v", *instanceKey, err)) + } + + if readOnly { + // Terminate existing non-replication, non-orchestrator connections to close + // the write window. Replication backends (walsender) and our own connection + // are excluded. + log.Infof("PostgreSQLSetReadOnly: terminating non-replication backends on %+v", *instanceKey) + _, err := db.Exec(` + SELECT pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE pid <> pg_backend_pid() + AND backend_type NOT IN ('walsender', 'walreceiver', 'autovacuum worker', 'logical replication launcher') + AND backend_type <> 'background worker' + AND datname IS NOT NULL + `) + if err != nil { + // Non-fatal: log warning but continue — read-only is already set + _ = log.Warningf("PostgreSQLSetReadOnly: error terminating backends on %+v: %v", *instanceKey, err) + } + } + + return ReadPostgreSQLTopologyInstance(instanceKey) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./go/inst/... -run TestPostgreSQLSetReadOnlyNilKey -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add go/inst/instance_topology_postgresql.go go/inst/instance_topology_postgresql_test.go +git commit -m "feat(postgresql): add PostgreSQLSetReadOnly for graceful switchover" +``` + +--- + +### Task 2: `PostgreSQLGetCurrentWALLSN` — Unit Test + Implementation + +**Files:** +- Modify: `go/inst/instance_topology_postgresql.go` (append after PostgreSQLSetReadOnly) +- Modify: `go/inst/instance_topology_postgresql_test.go` (add test) + +- [ ] **Step 1: Write the failing test** + +Add to `go/inst/instance_topology_postgresql_test.go`: + +```go +func TestPostgreSQLGetCurrentWALLSNNilKey(t *testing.T) { + _, err := PostgreSQLGetCurrentWALLSN(nil) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./go/inst/... -run TestPostgreSQLGetCurrentWALLSNNilKey -v` +Expected: FAIL — `PostgreSQLGetCurrentWALLSN` is not defined. + +- [ ] **Step 3: Write minimal implementation** + +Append to `go/inst/instance_topology_postgresql.go`: + +```go +// PostgreSQLGetCurrentWALLSN returns the current WAL write position (LSN) +// of a PostgreSQL primary instance. +func PostgreSQLGetCurrentWALLSN(instanceKey *InstanceKey) (string, error) { + if instanceKey == nil { + return "", fmt.Errorf("PostgreSQLGetCurrentWALLSN: nil instanceKey") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return "", log.Errore(err) + } + defer func() { _ = db.Close() }() + + var lsn string + if err := db.QueryRow("SELECT pg_current_wal_lsn()::text").Scan(&lsn); err != nil { + return "", log.Errore(fmt.Errorf("PostgreSQLGetCurrentWALLSN: failed on %+v: %v", *instanceKey, err)) + } + + log.Infof("PostgreSQLGetCurrentWALLSN: %+v current LSN is %s", *instanceKey, lsn) + return lsn, nil +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./go/inst/... -run TestPostgreSQLGetCurrentWALLSNNilKey -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add go/inst/instance_topology_postgresql.go go/inst/instance_topology_postgresql_test.go +git commit -m "feat(postgresql): add PostgreSQLGetCurrentWALLSN for WAL position tracking" +``` + +--- + +### Task 3: `PostgreSQLWaitForStandbyLSN` — Unit Test + Implementation + +**Files:** +- Modify: `go/inst/instance_topology_postgresql.go` (append after PostgreSQLGetCurrentWALLSN) +- Modify: `go/inst/instance_topology_postgresql_test.go` (add test) + +- [ ] **Step 1: Write the failing test** + +Add to `go/inst/instance_topology_postgresql_test.go`: + +```go +func TestPostgreSQLWaitForStandbyLSNNilKey(t *testing.T) { + err := PostgreSQLWaitForStandbyLSN(nil, "0/0", 1*time.Second) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} + +func TestPostgreSQLWaitForStandbyLSNEmptyLSN(t *testing.T) { + key := &InstanceKey{Hostname: "localhost", Port: 5432} + err := PostgreSQLWaitForStandbyLSN(key, "", 1*time.Second) + if err == nil { + t.Fatal("expected error for empty targetLSN") + } +} +``` + +Add `"time"` to the imports in the test file. + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./go/inst/... -run TestPostgreSQLWaitForStandbyLSN -v` +Expected: FAIL — `PostgreSQLWaitForStandbyLSN` is not defined. + +- [ ] **Step 3: Write minimal implementation** + +Append to `go/inst/instance_topology_postgresql.go`: + +```go +// PostgreSQLWaitForStandbyLSN polls a PostgreSQL standby until its replay LSN +// reaches or exceeds the target LSN, or the timeout expires. +func PostgreSQLWaitForStandbyLSN(standbyKey *InstanceKey, targetLSN string, timeout time.Duration) error { + if standbyKey == nil { + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: nil standbyKey") + } + if targetLSN == "" { + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: empty targetLSN") + } + + db, err := openPostgreSQLTopology(*standbyKey) + if err != nil { + return log.Errore(err) + } + defer func() { _ = db.Close() }() + + log.Infof("PostgreSQLWaitForStandbyLSN: waiting for %+v to reach LSN %s (timeout %v)", *standbyKey, targetLSN, timeout) + + pollInterval := 500 * time.Millisecond + deadline := time.Now().Add(timeout) + + for time.Now().Before(deadline) { + var reached bool + err := db.QueryRow( + "SELECT pg_last_wal_replay_lsn() >= $1::pg_lsn", targetLSN, + ).Scan(&reached) + if err != nil { + _ = log.Warningf("PostgreSQLWaitForStandbyLSN: error polling %+v: %v", *standbyKey, err) + } else if reached { + log.Infof("PostgreSQLWaitForStandbyLSN: %+v reached target LSN %s", *standbyKey, targetLSN) + return nil + } + time.Sleep(pollInterval) + } + + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: %+v did not reach LSN %s within %v", *standbyKey, targetLSN, timeout) +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./go/inst/... -run TestPostgreSQLWaitForStandbyLSN -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add go/inst/instance_topology_postgresql.go go/inst/instance_topology_postgresql_test.go +git commit -m "feat(postgresql): add PostgreSQLWaitForStandbyLSN for catch-up polling" +``` + +--- + +### Task 4: `PostgreSQLRepositionAsStandby` — Unit Test + Implementation + +**Files:** +- Modify: `go/inst/instance_topology_postgresql.go` (append after PostgreSQLWaitForStandbyLSN) +- Modify: `go/inst/instance_topology_postgresql_test.go` (add test) + +- [ ] **Step 1: Write the failing test** + +Add to `go/inst/instance_topology_postgresql_test.go`: + +```go +func TestPostgreSQLRepositionAsStandbyNilKeys(t *testing.T) { + key := &InstanceKey{Hostname: "localhost", Port: 5432} + if err := PostgreSQLRepositionAsStandby(nil, key); err == nil { + t.Fatal("expected error for nil instanceKey") + } + if err := PostgreSQLRepositionAsStandby(key, nil); err == nil { + t.Fatal("expected error for nil newPrimaryKey") + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `go test ./go/inst/... -run TestPostgreSQLRepositionAsStandbyNilKeys -v` +Expected: FAIL — `PostgreSQLRepositionAsStandby` is not defined. + +- [ ] **Step 3: Write minimal implementation** + +Append to `go/inst/instance_topology_postgresql.go`: + +```go +// PostgreSQLRepositionAsStandby configures a (demoted) PostgreSQL primary to +// replicate from a new primary by updating primary_conninfo via ALTER SYSTEM. +// The actual restart and standby.signal creation is the operator's +// responsibility, typically via PostGracefulTakeoverProcesses hooks. +func PostgreSQLRepositionAsStandby(instanceKey *InstanceKey, newPrimaryKey *InstanceKey) error { + if instanceKey == nil || newPrimaryKey == nil { + return fmt.Errorf("PostgreSQLRepositionAsStandby: nil key provided") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return log.Errore(err) + } + defer func() { _ = db.Close() }() + + newConnInfo := fmt.Sprintf( + "host=%s port=%d user=%s password=%s sslmode=%s application_name=orchestrator", + newPrimaryKey.Hostname, + newPrimaryKey.Port, + config.Config.PostgreSQLTopologyUser, + config.Config.PostgreSQLTopologyPassword, + config.Config.PostgreSQLSSLMode, + ) + + log.Infof("PostgreSQLRepositionAsStandby: configuring %+v to replicate from %+v", *instanceKey, *newPrimaryKey) + + if _, err := db.Exec(fmt.Sprintf("ALTER SYSTEM SET primary_conninfo = '%s'", newConnInfo)); err != nil { + return log.Errore(fmt.Errorf("PostgreSQLRepositionAsStandby: ALTER SYSTEM failed on %+v: %v", *instanceKey, err)) + } + + if _, err := db.Exec("SELECT pg_reload_conf()"); err != nil { + return log.Errore(fmt.Errorf("PostgreSQLRepositionAsStandby: pg_reload_conf() failed on %+v: %v", *instanceKey, err)) + } + + log.Infof("PostgreSQLRepositionAsStandby: %+v configured. Operator must restart with standby.signal to complete demotion.", *instanceKey) + return nil +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `go test ./go/inst/... -run TestPostgreSQLRepositionAsStandbyNilKeys -v` +Expected: PASS + +- [ ] **Step 5: Commit** + +```bash +git add go/inst/instance_topology_postgresql.go go/inst/instance_topology_postgresql_test.go +git commit -m "feat(postgresql): add PostgreSQLRepositionAsStandby for demoted primary" +``` + +--- + +### Task 5: `PostgreSQLGracefulPrimarySwitchover` — Implementation + +**Files:** +- Modify: `go/logic/topology_recovery_postgresql.go` (append new function) + +- [ ] **Step 1: Write the orchestration function** + +Append to `go/logic/topology_recovery_postgresql.go`. First update imports at the top of the file: + +Replace the import block: +```go +import ( + "fmt" + + "github.com/proxysql/orchestrator/go/config" + "github.com/proxysql/orchestrator/go/inst" +) +``` + +With: +```go +import ( + "fmt" + "time" + + "github.com/proxysql/golib/log" + "github.com/proxysql/orchestrator/go/config" + "github.com/proxysql/orchestrator/go/inst" +) +``` + +Then append the function: + +```go +// PostgreSQLGracefulPrimarySwitchover performs a planned switchover of a +// PostgreSQL primary to a designated standby. It sets the primary read-only, +// waits for the standby to catch up, promotes the standby, reconfigures +// remaining standbys, and configures the demoted primary for repositioning. +func PostgreSQLGracefulPrimarySwitchover(clusterName string, designatedKey *inst.InstanceKey, auto bool) (topologyRecovery *TopologyRecovery, err error) { + // --- Validate: read primary and standbys --- + clusterMasters, err := inst.ReadClusterMaster(clusterName) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: cannot deduce cluster primary for %+v: %v", clusterName, err) + } + if len(clusterMasters) != 1 { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: found %d potential primaries for %+v, expected 1", len(clusterMasters), clusterName) + } + clusterPrimary := clusterMasters[0] + + standbys, err := inst.ReadReplicaInstances(&clusterPrimary.Key) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: error reading standbys: %v", err) + } + if len(standbys) == 0 { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: primary %+v has no standbys", clusterPrimary.Key) + } + + // --- Determine designated standby --- + var designatedStandby *inst.Instance + if designatedKey != nil && designatedKey.IsValid() { + // User specified a standby — verify it's a direct replica + for _, s := range standbys { + if s.Key.Equals(designatedKey) { + designatedStandby = s + break + } + } + if designatedStandby == nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated instance %+v is not a standby of %+v", *designatedKey, clusterPrimary.Key) + } + } else if len(standbys) == 1 { + designatedStandby = standbys[0] + } else if auto { + designatedStandby, err = inst.PostgreSQLGetBestStandbyForPromotion(standbys, nil) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: cannot auto-select standby: %v", err) + } + } else { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: multiple standbys and no designated instance specified (use auto or specify -d)") + } + + if designatedStandby.IsDowntimed { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v is downtimed", designatedStandby.Key) + } + if !designatedStandby.IsLastCheckValid { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v has invalid last check", designatedStandby.Key) + } + if !designatedStandby.HasReasonableMaintenanceReplicationLag() { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v has excessive replication lag", designatedStandby.Key) + } + + log.Infof("PostgreSQLGracefulPrimarySwitchover: will demote %+v and promote %+v", clusterPrimary.Key, designatedStandby.Key) + + // --- Register recovery and build analysis entry --- + analysisEntry, err := forceAnalysisEntry(clusterName, inst.DeadPrimary, inst.GracefulMasterTakeoverCommandHint, &clusterPrimary.Key) + if err != nil { + return nil, err + } + + // --- Execute PreGracefulTakeoverProcesses --- + preGracefulTakeoverTopologyRecovery := &TopologyRecovery{ + SuccessorKey: &designatedStandby.Key, + AnalysisEntry: analysisEntry, + } + if err := executeProcesses(config.Config.PreGracefulTakeoverProcesses, "PreGracefulTakeoverProcesses", preGracefulTakeoverTopologyRecovery, true); err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: PreGracefulTakeoverProcesses failed: %v", err) + } + + // --- Set primary read-only --- + log.Infof("PostgreSQLGracefulPrimarySwitchover: setting %+v read-only", clusterPrimary.Key) + if _, err := inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, true); err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: failed to set read-only on %+v: %v", clusterPrimary.Key, err) + } + + // --- Capture target LSN --- + targetLSN, err := inst.PostgreSQLGetCurrentWALLSN(&clusterPrimary.Key) + if err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: failed to get WAL LSN from %+v: %v", clusterPrimary.Key, err) + } + log.Infof("PostgreSQLGracefulPrimarySwitchover: target LSN is %s", targetLSN) + + // --- Wait for standby to catch up --- + catchUpTimeout := time.Duration(config.Config.ReasonableMaintenanceReplicationLagSeconds) * time.Second + if err := inst.PostgreSQLWaitForStandbyLSN(&designatedStandby.Key, targetLSN, catchUpTimeout); err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: standby catch-up failed: %v", err) + } + + // --- Promote designated standby --- + promotedInstance, err := inst.PostgreSQLPromoteStandby(&designatedStandby.Key) + if err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: promotion of %+v failed: %v", designatedStandby.Key, err) + } + log.Infof("PostgreSQLGracefulPrimarySwitchover: promoted %+v to primary", promotedInstance.Key) + + // --- Register recovery --- + topologyRecovery, err = AttemptRecoveryRegistration(&analysisEntry, false, false) + if err != nil || topologyRecovery == nil { + _ = log.Warningf("PostgreSQLGracefulPrimarySwitchover: error registering recovery: %v", err) + // Continue — promotion already happened + topologyRecovery = &TopologyRecovery{ + AnalysisEntry: analysisEntry, + } + } + topologyRecovery.SuccessorKey = &promotedInstance.Key + + // --- Reconfigure remaining standbys --- + for _, standby := range standbys { + if standby.Key.Equals(&promotedInstance.Key) { + continue + } + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("reconfiguring standby %+v to replicate from new primary %+v", standby.Key, promotedInstance.Key)) + if err := inst.PostgreSQLReconfigureStandby(&standby.Key, &promotedInstance.Key); err != nil { + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("error reconfiguring standby %+v: %v (continuing)", standby.Key, err)) + topologyRecovery.LostReplicas.AddKey(standby.Key) + } + } + + // --- Configure demoted primary for repositioning --- + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("configuring demoted primary %+v to replicate from %+v", clusterPrimary.Key, promotedInstance.Key)) + if err := inst.PostgreSQLRepositionAsStandby(&clusterPrimary.Key, &promotedInstance.Key); err != nil { + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("error configuring demoted primary %+v: %v", clusterPrimary.Key, err)) + } + + // --- Resolve recovery --- + resolveRecovery(topologyRecovery, promotedInstance) + + // --- Execute PostGracefulTakeoverProcesses --- + executeProcesses(config.Config.PostGracefulTakeoverProcesses, "PostGracefulTakeoverProcesses", topologyRecovery, false) + + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("PostgreSQLGracefulPrimarySwitchover: completed. New primary: %+v", promotedInstance.Key)) + return topologyRecovery, nil +} +``` + +- [ ] **Step 2: Verify compilation** + +Run: `go build ./go/...` +Expected: compiles without errors. + +- [ ] **Step 3: Commit** + +```bash +git add go/logic/topology_recovery_postgresql.go +git commit -m "feat(postgresql): add PostgreSQLGracefulPrimarySwitchover orchestration" +``` + +--- + +### Task 6: Provider Dispatch in `GracefulMasterTakeover` + +**Files:** +- Modify: `go/logic/topology_recovery.go` (lines 2114-2122, add dispatch after reading cluster master) + +- [ ] **Step 1: Add PostgreSQL dispatch** + +In `go/logic/topology_recovery.go`, after line 2122 (`clusterMaster := clusterMasters[0]`), add: + +```go + // PostgreSQL: dispatch to PostgreSQL-specific implementation + if clusterMaster.ProviderType == "postgresql" { + topologyRecovery, err := PostgreSQLGracefulPrimarySwitchover(clusterName, designatedKey, auto) + return topologyRecovery, nil, err + } +``` + +This goes right after `clusterMaster := clusterMasters[0]` and before `clusterMasterDirectReplicas, err := inst.ReadReplicaInstances(...)`. + +- [ ] **Step 2: Verify compilation** + +Run: `go build ./go/...` +Expected: compiles without errors. + +- [ ] **Step 3: Verify existing tests still pass** + +Run: `go test ./go/logic/... -v -count=1 2>&1 | tail -20` +Expected: all existing tests pass (or skip if they require a running backend). + +- [ ] **Step 4: Commit** + +```bash +git add go/logic/topology_recovery.go +git commit -m "feat(postgresql): dispatch graceful-master-takeover to PostgreSQL switchover" +``` + +--- + +### Task 7: Functional Test — Graceful Switchover + +**Files:** +- Modify: `tests/functional/test-postgresql.sh` (append graceful switchover test section) + +- [ ] **Step 1: Add graceful switchover test to the functional test script** + +Append to `tests/functional/test-postgresql.sh`, before the final summary section (before `echo "=== POSTGRESQL TESTS COMPLETE ==="`): + +```bash +# ---------------------------------------------------------------- +echo "" +echo "--- Graceful primary switchover tests ---" + +# Re-discover topology after any prior failover tests +echo "Re-discovering PostgreSQL topology..." +curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.20/5432" > /dev/null 2>&1 +curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.21/5432" > /dev/null 2>&1 +sleep 10 + +# Identify current primary +CURRENT_PRIMARY=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +instances = json.load(sys.stdin) +for inst in instances: + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname'] + ':' + str(inst['Key']['Port'])) + sys.exit(0) +print('') +" 2>/dev/null || echo "") + +if [ -z "$CURRENT_PRIMARY" ]; then + fail "Cannot identify current PostgreSQL primary" +else + echo "Current primary: $CURRENT_PRIMARY" + + # Execute graceful-master-takeover-auto via API + echo "Executing graceful-master-takeover-auto on cluster $PG_CLUSTER..." + TAKEOVER_RESULT=$(curl -s --max-time 60 "$ORC_URL/api/graceful-master-takeover-auto/$PG_CLUSTER" 2>/dev/null) + TAKEOVER_CODE=$(echo "$TAKEOVER_RESULT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('Code','ERROR'))" 2>/dev/null || echo "ERROR") + + if [ "$TAKEOVER_CODE" = "OK" ]; then + pass "Graceful master takeover API returned OK" + else + fail "Graceful master takeover API returned: $TAKEOVER_CODE — $TAKEOVER_RESULT" + fi + + # Wait for topology to settle + sleep 10 + + # Verify primary has changed + NEW_PRIMARY=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +instances = json.load(sys.stdin) +for inst in instances: + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname'] + ':' + str(inst['Key']['Port'])) + sys.exit(0) +print('') +" 2>/dev/null || echo "") + + if [ -n "$NEW_PRIMARY" ] && [ "$NEW_PRIMARY" != "$CURRENT_PRIMARY" ]; then + pass "Primary switched from $CURRENT_PRIMARY to $NEW_PRIMARY" + else + fail "Primary did not change: was $CURRENT_PRIMARY, now ${NEW_PRIMARY:-unknown}" + fi +fi +``` + +- [ ] **Step 2: Verify the test script has valid syntax** + +Run: `bash -n tests/functional/test-postgresql.sh` +Expected: no syntax errors. + +- [ ] **Step 3: Commit** + +```bash +git add tests/functional/test-postgresql.sh +git commit -m "test(postgresql): add functional test for graceful primary switchover" +``` + +--- + +### Task 8: Update Documentation + +**Files:** +- Modify: `docs/database-providers.md` (remove "not yet implemented" note) +- Modify: `docs/user-manual.md` (remove "not yet supported" note, add graceful switchover section) + +- [ ] **Step 1: Update database-providers.md** + +Find the line (around line 183-184) that says: +``` +Graceful master takeover (planned switchover) is not yet implemented for PostgreSQL. Only unplanned failover (dead primary) is supported. +``` + +Replace with: +``` +### Graceful Primary Switchover + +Graceful master takeover (planned switchover) is supported for PostgreSQL. The same CLI commands +and API endpoints used for MySQL (`graceful-master-takeover`, `graceful-master-takeover-auto`) work +for PostgreSQL clusters. The switchover flow: + +1. Sets the primary read-only via `ALTER SYSTEM SET default_transaction_read_only = on` and terminates + existing non-replication connections. +2. Captures the primary's current WAL LSN and waits for the designated standby to catch up. +3. Promotes the designated standby via `pg_promote()`. +4. Reconfigures remaining standbys to replicate from the new primary. +5. Configures the demoted primary's `primary_conninfo` to point at the new primary. + +**Important:** The demoted primary requires a restart with `standby.signal` to complete its conversion +to a standby. Use `PostGracefulTakeoverProcesses` hooks to automate this step. +``` + +- [ ] **Step 2: Update user-manual.md** + +Find the line (around line 699) that says: +``` +Graceful master takeover is not yet supported for PostgreSQL. +``` + +Replace with: +``` +Graceful master takeover is supported for PostgreSQL. Use `graceful-master-takeover` or +`graceful-master-takeover-auto` CLI commands, or the equivalent API endpoints. The demoted +primary requires an operator-managed restart with `standby.signal` — configure this via +`PostGracefulTakeoverProcesses` hooks. +``` + +- [ ] **Step 3: Commit** + +```bash +git add docs/database-providers.md docs/user-manual.md +git commit -m "docs: update PostgreSQL docs to reflect graceful switchover support" +``` + +--- + +### Task 9: Create GitHub Issue + +**Files:** None (GitHub CLI only) + +- [ ] **Step 1: Create the GitHub issue** + +```bash +gh issue create \ + --title "feat: PostgreSQL graceful primary switchover (planned failover)" \ + --body "$(cat <<'BODY' +## Summary + +Implement planned/graceful primary switchover for PostgreSQL. Currently only unplanned failover (dead primary detection → promotion) is supported. This adds the ability to gracefully demote a running primary and promote a designated standby with zero data loss. + +## Motivation + +Operators need planned switchover for maintenance windows, version upgrades, and host migrations. The MySQL path already supports this via `graceful-master-takeover` / `graceful-master-takeover-auto` — PostgreSQL should have parity. + +## Design + +See `docs/superpowers/specs/2026-04-18-postgresql-graceful-switchover-design.md` for the full spec. + +### New instance operations (go/inst/instance_topology_postgresql.go): +- `PostgreSQLSetReadOnly` — ALTER SYSTEM + pg_reload_conf + pg_terminate_backend +- `PostgreSQLGetCurrentWALLSN` — pg_current_wal_lsn() +- `PostgreSQLWaitForStandbyLSN` — poll pg_last_wal_replay_lsn() until caught up +- `PostgreSQLRepositionAsStandby` — ALTER SYSTEM SET primary_conninfo for demoted primary + +### Switchover orchestration (go/logic/topology_recovery_postgresql.go): +- `PostgreSQLGracefulPrimarySwitchover` — full flow: validate → pre-hooks → set read-only → wait catch-up → promote → reconfigure standbys → reposition demoted primary → post-hooks + +### CLI/API dispatch (go/logic/topology_recovery.go): +- Provider check in `GracefulMasterTakeover()` dispatches to PostgreSQL path when cluster is PostgreSQL +- Same commands/endpoints: `graceful-master-takeover`, `graceful-master-takeover-auto` + +### Key decisions: +- Demoted primary restart is operator's responsibility via `PostGracefulTakeoverProcesses` hooks +- No `pg_rewind` needed — primary is read-only before switchover, so no timeline divergence +- Separate implementation function (not branching in MySQL code) — follows existing pattern + +## Test Plan +- [ ] Unit tests for nil/empty input validation on all 4 new instance operations +- [ ] Functional test: graceful switchover against real PostgreSQL topology +- [ ] Verify existing MySQL graceful takeover still works (no regression) +BODY +)" +``` + +- [ ] **Step 2: Note the issue number for commit references** + +Record the issue URL/number returned by `gh issue create`. + +- [ ] **Step 3: Commit** (no files changed, skip) diff --git a/docs/superpowers/specs/2026-04-18-postgresql-graceful-switchover-design.md b/docs/superpowers/specs/2026-04-18-postgresql-graceful-switchover-design.md new file mode 100644 index 00000000..35070236 --- /dev/null +++ b/docs/superpowers/specs/2026-04-18-postgresql-graceful-switchover-design.md @@ -0,0 +1,102 @@ +# PostgreSQL Graceful Primary Switchover + +**Date:** 2026-04-18 +**Status:** Approved + +## Summary + +Implement planned/graceful primary switchover for PostgreSQL in orchestrator. Currently only unplanned failover (dead primary) is supported. This adds the ability to gracefully demote a running primary and promote a designated standby with zero data loss, reusing the same CLI commands and API endpoints as MySQL's graceful master takeover. + +## Background + +Orchestrator already supports PostgreSQL unplanned failover via `checkAndRecoverDeadPrimary()` in `topology_recovery_postgresql.go`. The graceful takeover commands (`graceful-master-takeover`, `graceful-master-takeover-auto`) exist but dispatch only to MySQL-specific code in `GracefulMasterTakeover()`. PostgreSQL needs its own implementation following the established pattern of separate provider-specific recovery functions. + +## Design + +### New Instance Operations + +Three new functions in `go/inst/instance_topology_postgresql.go`: + +#### `PostgreSQLSetReadOnly(instanceKey *InstanceKey, readOnly bool) (*Instance, error)` + +- Connects to the instance via SQL +- Executes `ALTER SYSTEM SET default_transaction_read_only = on|off` +- Calls `SELECT pg_reload_conf()` +- When setting read-only: queries `pg_stat_activity` for non-replication, non-orchestrator backends and calls `pg_terminate_backend()` on each to close the write window +- Re-reads and returns the instance + +#### `PostgreSQLGetCurrentWALLSN(instanceKey *InstanceKey) (string, error)` + +- Connects to the primary +- Returns result of `SELECT pg_current_wal_lsn()::text` + +#### `PostgreSQLWaitForStandbyLSN(standbyKey *InstanceKey, targetLSN string, timeout time.Duration) error` + +- Polls `SELECT pg_last_wal_replay_lsn()` on the standby +- Compares using `SELECT '%s'::pg_lsn >= '%s'::pg_lsn` (PostgreSQL-native LSN comparison) +- Polls every 500ms up to the configured timeout +- Returns error on timeout + +#### `PostgreSQLRepositionAsStandby(instanceKey *InstanceKey, newPrimaryKey *InstanceKey) error` + +- Connects to the old primary +- Sets `primary_conninfo` via `ALTER SYSTEM` pointing to the new primary (same logic as existing `PostgreSQLReconfigureStandby`) +- Calls `SELECT pg_reload_conf()` +- Does NOT restart the instance — the actual restart and `standby.signal` creation is the operator's responsibility via `PostGracefulTakeoverProcesses` hook + +### Switchover Orchestration + +New function `PostgreSQLGracefulPrimarySwitchover(clusterName string, designatedKey *InstanceKey, auto bool) (*TopologyRecovery, *inst.BinlogCoordinates, error)` in `go/logic/topology_recovery_postgresql.go`: + +**Flow:** + +1. **Validate** — Read cluster primary, read standbys, determine designated standby. When `auto=true`, use `PostgreSQLGetBestStandbyForPromotion()`. When manual, require the user to specify or have exactly one standby. +2. **Execute `PreGracefulTakeoverProcesses`** — Abort on failure. +3. **Set primary read-only** — `PostgreSQLSetReadOnly(primary, true)`. Terminates existing non-replication connections. +4. **Capture target LSN** — `PostgreSQLGetCurrentWALLSN(primary)`. +5. **Wait for standby catch-up** — `PostgreSQLWaitForStandbyLSN(designated, targetLSN, timeout)`. +6. **Promote designated standby** — `PostgreSQLPromoteStandby(designated)` (existing function). +7. **Reconfigure remaining standbys** — `PostgreSQLReconfigureStandby(standby, newPrimary)` for each non-designated standby (existing function). +8. **Configure demoted primary** — `PostgreSQLRepositionAsStandby(oldPrimary, newPrimary)`. Sets `primary_conninfo` only; operator handles restart via hook. +9. **Execute `PostGracefulTakeoverProcesses`**. +10. **Return** TopologyRecovery with successor key. + +**Error handling:** + +- If promotion fails after setting read-only: undo via `PostgreSQLSetReadOnly(primary, false)`, abort. +- If standby catch-up times out: undo read-only, abort with error. + +**Note on flat topology:** PostgreSQL topologies are flat (no cascading replication detected), so there is no sibling relocation step (unlike MySQL which may need to reparent replicas under the designated replica). + +### CLI/API Dispatch + +No changes to `cli.go` or `api.go`. The existing commands and endpoints are reused. + +**In `go/logic/topology_recovery.go`:** + +Add a provider check in `GracefulMasterTakeover()`: + +```go +if clusterMaster.DatabaseProvider == inst.PostgreSQL { + return PostgreSQLGracefulPrimarySwitchover(clusterName, designatedKey, auto) +} +``` + +Add a PostgreSQL branch in `getGracefulMasterTakeoverDesignatedInstance()` to use `PostgreSQLGetBestStandbyForPromotion()` when `auto=true`. + +### Testing + +- Unit tests for the 3 new instance operations +- Unit test for `PostgreSQLGracefulPrimarySwitchover` flow +- Functional test in `tests/functional/` — run a graceful switchover against a real PostgreSQL topology and verify the new primary is writable and old primary is configured for standby + +## Decisions + +| Decision | Choice | Rationale | +|----------|--------|-----------| +| Both manual and auto variants | Yes | Match MySQL parity for consistent UX | +| Read-only mechanism | `ALTER SYSTEM` + `pg_terminate_backend()` | Mirrors MySQL `SET GLOBAL read_only=1` behavior | +| Standby catch-up | Poll `pg_last_wal_replay_lsn()` on standby | Simple, doesn't depend on primary staying up during wait | +| Implementation location | Separate `PostgreSQLGracefulPrimarySwitchover()` | Follows existing pattern of separate provider functions | +| Demoted primary restart | Operator's responsibility via hook | Orchestrator connects via SQL only, no shell access assumed | +| Old primary repositioning | `ALTER SYSTEM SET primary_conninfo` only | No timeline divergence since primary is read-only before switchover, so `pg_rewind` is unnecessary | diff --git a/docs/user-manual.md b/docs/user-manual.md index b82d1f50..aad65db5 100644 --- a/docs/user-manual.md +++ b/docs/user-manual.md @@ -696,7 +696,10 @@ When `ProviderType` is `"postgresql"`, orchestrator performs PostgreSQL-specific - Promotion is a single `pg_promote()` call rather than a sequence of `STOP SLAVE` / `RESET SLAVE` / `CHANGE MASTER`. - Standby reconfiguration uses `ALTER SYSTEM` instead of `CHANGE MASTER TO`. - Standbys that fail to reconfigure are added to `LostReplicas` but do not block the recovery. -- Graceful master takeover is not yet supported for PostgreSQL. +- Graceful master takeover is supported for PostgreSQL. Use `graceful-master-takeover` + or `graceful-master-takeover-auto` CLI commands, or the equivalent API endpoints. + The demoted primary requires an operator-managed restart with `standby.signal` — + configure this via `PostGracefulTakeoverProcesses` hooks. --- diff --git a/go/inst/instance_topology_postgresql.go b/go/inst/instance_topology_postgresql.go index c522a35d..369deee3 100644 --- a/go/inst/instance_topology_postgresql.go +++ b/go/inst/instance_topology_postgresql.go @@ -18,6 +18,7 @@ package inst import ( "fmt" + "strings" "time" "github.com/proxysql/golib/log" @@ -109,7 +110,9 @@ func PostgreSQLReconfigureStandby(standbyKey *InstanceKey, newPrimaryKey *Instan log.Infof("PostgreSQLReconfigureStandby: reconfiguring %+v to replicate from %+v", *standbyKey, *newPrimaryKey) // Update primary_conninfo using ALTER SYSTEM - _, err = db.Exec(fmt.Sprintf("ALTER SYSTEM SET primary_conninfo = '%s'", newConnInfo)) + // Escape single quotes to prevent SQL breakage from passwords containing quotes + escapedConnInfo := strings.ReplaceAll(newConnInfo, "'", "''") + _, err = db.Exec(fmt.Sprintf("ALTER SYSTEM SET primary_conninfo = '%s'", escapedConnInfo)) if err != nil { return log.Errore(fmt.Errorf("PostgreSQLReconfigureStandby: ALTER SYSTEM failed on %+v: %v", *standbyKey, err)) } @@ -179,3 +182,155 @@ func PostgreSQLGetBestStandbyForPromotion(replicas [](*Instance), candidateKey * } return bestStandby, nil } + +// PostgreSQLSetReadOnly sets or unsets read-only mode on a PostgreSQL instance +// by updating default_transaction_read_only via ALTER SYSTEM and reloading. +// When setting read-only, it also terminates existing non-replication connections +// to prevent stray writes during graceful switchover. +func PostgreSQLSetReadOnly(instanceKey *InstanceKey, readOnly bool) (*Instance, error) { + if instanceKey == nil { + return nil, fmt.Errorf("PostgreSQLSetReadOnly: nil instanceKey") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return nil, log.Errore(err) + } + defer func() { _ = db.Close() }() + + readOnlyValue := "off" + if readOnly { + readOnlyValue = "on" + } + + log.Infof("PostgreSQLSetReadOnly: setting default_transaction_read_only = %s on %+v", readOnlyValue, *instanceKey) + + if _, err := db.Exec(fmt.Sprintf("ALTER SYSTEM SET default_transaction_read_only = %s", readOnlyValue)); err != nil { + return nil, log.Errore(fmt.Errorf("PostgreSQLSetReadOnly: ALTER SYSTEM failed on %+v: %v", *instanceKey, err)) + } + + if _, err := db.Exec("SELECT pg_reload_conf()"); err != nil { + return nil, log.Errore(fmt.Errorf("PostgreSQLSetReadOnly: pg_reload_conf() failed on %+v: %v", *instanceKey, err)) + } + + if readOnly { + // Terminate existing non-replication, non-orchestrator connections to close + // the write window. Replication backends (walsender) and our own connection + // are excluded. + log.Infof("PostgreSQLSetReadOnly: terminating non-replication backends on %+v", *instanceKey) + _, err := db.Exec(` + SELECT pg_terminate_backend(pid) + FROM pg_stat_activity + WHERE pid <> pg_backend_pid() + AND backend_type NOT IN ('walsender', 'walreceiver', 'autovacuum worker', 'logical replication launcher') + AND backend_type <> 'background worker' + AND datname IS NOT NULL + `) + if err != nil { + // Non-fatal: log warning but continue — read-only is already set + _ = log.Warningf("PostgreSQLSetReadOnly: error terminating backends on %+v: %v", *instanceKey, err) + } + } + + return ReadPostgreSQLTopologyInstance(instanceKey) +} + +// PostgreSQLGetCurrentWALLSN returns the current WAL write position (LSN) +// of a PostgreSQL primary instance. +func PostgreSQLGetCurrentWALLSN(instanceKey *InstanceKey) (string, error) { + if instanceKey == nil { + return "", fmt.Errorf("PostgreSQLGetCurrentWALLSN: nil instanceKey") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return "", log.Errore(err) + } + defer func() { _ = db.Close() }() + + var lsn string + if err := db.QueryRow("SELECT pg_current_wal_lsn()::text").Scan(&lsn); err != nil { + return "", log.Errore(fmt.Errorf("PostgreSQLGetCurrentWALLSN: failed on %+v: %v", *instanceKey, err)) + } + + log.Infof("PostgreSQLGetCurrentWALLSN: %+v current LSN is %s", *instanceKey, lsn) + return lsn, nil +} + +// PostgreSQLWaitForStandbyLSN polls a PostgreSQL standby until its replay LSN +// reaches or exceeds the target LSN, or the timeout expires. +func PostgreSQLWaitForStandbyLSN(standbyKey *InstanceKey, targetLSN string, timeout time.Duration) error { + if standbyKey == nil { + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: nil standbyKey") + } + if targetLSN == "" { + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: empty targetLSN") + } + + db, err := openPostgreSQLTopology(*standbyKey) + if err != nil { + return log.Errore(err) + } + defer func() { _ = db.Close() }() + + log.Infof("PostgreSQLWaitForStandbyLSN: waiting for %+v to reach LSN %s (timeout %v)", *standbyKey, targetLSN, timeout) + + pollInterval := 500 * time.Millisecond + deadline := time.Now().Add(timeout) + + for time.Now().Before(deadline) { + var reached bool + err := db.QueryRow( + "SELECT pg_last_wal_replay_lsn() >= $1::pg_lsn", targetLSN, + ).Scan(&reached) + if err != nil { + _ = log.Warningf("PostgreSQLWaitForStandbyLSN: error polling %+v: %v", *standbyKey, err) + } else if reached { + log.Infof("PostgreSQLWaitForStandbyLSN: %+v reached target LSN %s", *standbyKey, targetLSN) + return nil + } + time.Sleep(pollInterval) + } + + return fmt.Errorf("PostgreSQLWaitForStandbyLSN: %+v did not reach LSN %s within %v", *standbyKey, targetLSN, timeout) +} + +// PostgreSQLRepositionAsStandby configures a (demoted) PostgreSQL primary to +// replicate from a new primary by updating primary_conninfo via ALTER SYSTEM. +// The actual restart and standby.signal creation is the operator's +// responsibility, typically via PostGracefulTakeoverProcesses hooks. +func PostgreSQLRepositionAsStandby(instanceKey *InstanceKey, newPrimaryKey *InstanceKey) error { + if instanceKey == nil || newPrimaryKey == nil { + return fmt.Errorf("PostgreSQLRepositionAsStandby: nil key provided") + } + + db, err := openPostgreSQLTopology(*instanceKey) + if err != nil { + return log.Errore(err) + } + defer func() { _ = db.Close() }() + + newConnInfo := fmt.Sprintf( + "host=%s port=%d user=%s password=%s sslmode=%s application_name=orchestrator", + newPrimaryKey.Hostname, + newPrimaryKey.Port, + config.Config.PostgreSQLTopologyUser, + config.Config.PostgreSQLTopologyPassword, + config.Config.PostgreSQLSSLMode, + ) + + log.Infof("PostgreSQLRepositionAsStandby: configuring %+v to replicate from %+v", *instanceKey, *newPrimaryKey) + + // Escape single quotes to prevent SQL breakage from passwords containing quotes + escapedConnInfo := strings.ReplaceAll(newConnInfo, "'", "''") + if _, err := db.Exec(fmt.Sprintf("ALTER SYSTEM SET primary_conninfo = '%s'", escapedConnInfo)); err != nil { + return log.Errore(fmt.Errorf("PostgreSQLRepositionAsStandby: ALTER SYSTEM failed on %+v: %v", *instanceKey, err)) + } + + if _, err := db.Exec("SELECT pg_reload_conf()"); err != nil { + return log.Errore(fmt.Errorf("PostgreSQLRepositionAsStandby: pg_reload_conf() failed on %+v: %v", *instanceKey, err)) + } + + log.Infof("PostgreSQLRepositionAsStandby: %+v configured. Operator must restart with standby.signal to complete demotion.", *instanceKey) + return nil +} diff --git a/go/inst/instance_topology_postgresql_test.go b/go/inst/instance_topology_postgresql_test.go new file mode 100644 index 00000000..e8288f41 --- /dev/null +++ b/go/inst/instance_topology_postgresql_test.go @@ -0,0 +1,61 @@ +/* + Copyright 2024 Orchestrator Authors + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +*/ + +package inst + +import ( + "testing" + "time" +) + +func TestPostgreSQLSetReadOnlyNilKey(t *testing.T) { + _, err := PostgreSQLSetReadOnly(nil, true) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} + +func TestPostgreSQLGetCurrentWALLSNNilKey(t *testing.T) { + _, err := PostgreSQLGetCurrentWALLSN(nil) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} + +func TestPostgreSQLWaitForStandbyLSNNilKey(t *testing.T) { + err := PostgreSQLWaitForStandbyLSN(nil, "0/0", 1*time.Second) + if err == nil { + t.Fatal("expected error for nil instanceKey") + } +} + +func TestPostgreSQLWaitForStandbyLSNEmptyLSN(t *testing.T) { + key := &InstanceKey{Hostname: "localhost", Port: 5432} + err := PostgreSQLWaitForStandbyLSN(key, "", 1*time.Second) + if err == nil { + t.Fatal("expected error for empty targetLSN") + } +} + +func TestPostgreSQLRepositionAsStandbyNilKeys(t *testing.T) { + key := &InstanceKey{Hostname: "localhost", Port: 5432} + if err := PostgreSQLRepositionAsStandby(nil, key); err == nil { + t.Fatal("expected error for nil instanceKey") + } + if err := PostgreSQLRepositionAsStandby(key, nil); err == nil { + t.Fatal("expected error for nil newPrimaryKey") + } +} diff --git a/go/logic/topology_recovery.go b/go/logic/topology_recovery.go index 95cede8e..af8cce1a 100644 --- a/go/logic/topology_recovery.go +++ b/go/logic/topology_recovery.go @@ -2121,6 +2121,12 @@ func GracefulMasterTakeover(clusterName string, designatedKey *inst.InstanceKey, } clusterMaster := clusterMasters[0] + // PostgreSQL: dispatch to PostgreSQL-specific implementation + if clusterMaster.ProviderType == "postgresql" { + topologyRecovery, err := PostgreSQLGracefulPrimarySwitchover(clusterName, designatedKey, auto) + return topologyRecovery, nil, err + } + clusterMasterDirectReplicas, err := inst.ReadReplicaInstances(&clusterMaster.Key) if err != nil { return nil, nil, log.Errore(err) diff --git a/go/logic/topology_recovery_postgresql.go b/go/logic/topology_recovery_postgresql.go index 3b0fa17d..48583c21 100644 --- a/go/logic/topology_recovery_postgresql.go +++ b/go/logic/topology_recovery_postgresql.go @@ -18,7 +18,9 @@ package logic import ( "fmt" + "time" + "github.com/proxysql/golib/log" "github.com/proxysql/orchestrator/go/config" "github.com/proxysql/orchestrator/go/inst" ) @@ -108,3 +110,144 @@ func checkAndRecoverDeadPrimary(analysisEntry inst.ReplicationAnalysis, candidat return true, topologyRecovery, err } + +// PostgreSQLGracefulPrimarySwitchover performs a planned switchover of a +// PostgreSQL primary to a designated standby. It sets the primary read-only, +// waits for the standby to catch up, promotes the standby, reconfigures +// remaining standbys, and configures the demoted primary for repositioning. +func PostgreSQLGracefulPrimarySwitchover(clusterName string, designatedKey *inst.InstanceKey, auto bool) (topologyRecovery *TopologyRecovery, err error) { + // --- Validate: read primary and standbys --- + clusterMasters, err := inst.ReadClusterMaster(clusterName) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: cannot deduce cluster primary for %+v: %v", clusterName, err) + } + if len(clusterMasters) != 1 { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: found %d potential primaries for %+v, expected 1", len(clusterMasters), clusterName) + } + clusterPrimary := clusterMasters[0] + + standbys, err := inst.ReadReplicaInstances(&clusterPrimary.Key) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: error reading standbys: %v", err) + } + if len(standbys) == 0 { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: primary %+v has no standbys", clusterPrimary.Key) + } + + // --- Determine designated standby --- + var designatedStandby *inst.Instance + if designatedKey != nil && designatedKey.IsValid() { + // User specified a standby — verify it's a direct replica + for _, s := range standbys { + if s.Key.Equals(designatedKey) { + designatedStandby = s + break + } + } + if designatedStandby == nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated instance %+v is not a standby of %+v", *designatedKey, clusterPrimary.Key) + } + } else if len(standbys) == 1 { + designatedStandby = standbys[0] + } else if auto { + designatedStandby, err = inst.PostgreSQLGetBestStandbyForPromotion(standbys, nil) + if err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: cannot auto-select standby: %v", err) + } + } else { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: multiple standbys and no designated instance specified (use auto or specify -d)") + } + + if designatedStandby.IsDowntimed { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v is downtimed", designatedStandby.Key) + } + if !designatedStandby.IsLastCheckValid { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v has invalid last check", designatedStandby.Key) + } + if !designatedStandby.HasReasonableMaintenanceReplicationLag() { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: designated standby %+v has excessive replication lag", designatedStandby.Key) + } + + log.Infof("PostgreSQLGracefulPrimarySwitchover: will demote %+v and promote %+v", clusterPrimary.Key, designatedStandby.Key) + + // --- Register recovery and build analysis entry --- + analysisEntry, err := forceAnalysisEntry(clusterName, inst.DeadPrimary, inst.GracefulMasterTakeoverCommandHint, &clusterPrimary.Key) + if err != nil { + return nil, err + } + + // --- Register recovery to prevent concurrent switchover attempts --- + topologyRecovery, err = AttemptRecoveryRegistration(&analysisEntry, false, false) + if topologyRecovery == nil { + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("found an active or recent recovery on %+v. Will not issue another graceful switchover.", clusterName)) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: active or recent recovery exists for %+v, aborting", clusterName) + } + topologyRecovery.SuccessorKey = &designatedStandby.Key + + // --- Execute PreGracefulTakeoverProcesses --- + if err := executeProcesses(config.Config.PreGracefulTakeoverProcesses, "PreGracefulTakeoverProcesses", topologyRecovery, true); err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: PreGracefulTakeoverProcesses failed: %v", err) + } + + // --- Set primary read-only --- + log.Infof("PostgreSQLGracefulPrimarySwitchover: setting %+v read-only", clusterPrimary.Key) + if _, err := inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, true); err != nil { + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: failed to set read-only on %+v: %v", clusterPrimary.Key, err) + } + + // --- Capture target LSN --- + targetLSN, err := inst.PostgreSQLGetCurrentWALLSN(&clusterPrimary.Key) + if err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: failed to get WAL LSN from %+v: %v", clusterPrimary.Key, err) + } + log.Infof("PostgreSQLGracefulPrimarySwitchover: target LSN is %s", targetLSN) + + // --- Wait for standby to catch up --- + // Use 3x the maintenance lag threshold since the primary is already read-only + // and waiting longer only extends the maintenance window, not data risk. + catchUpTimeout := 3 * time.Duration(config.Config.ReasonableMaintenanceReplicationLagSeconds) * time.Second + if err := inst.PostgreSQLWaitForStandbyLSN(&designatedStandby.Key, targetLSN, catchUpTimeout); err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: standby catch-up failed: %v", err) + } + + // --- Promote designated standby --- + promotedInstance, err := inst.PostgreSQLPromoteStandby(&designatedStandby.Key) + if err != nil { + // Undo read-only before aborting + _, _ = inst.PostgreSQLSetReadOnly(&clusterPrimary.Key, false) + return nil, fmt.Errorf("PostgreSQLGracefulPrimarySwitchover: promotion of %+v failed: %v", designatedStandby.Key, err) + } + log.Infof("PostgreSQLGracefulPrimarySwitchover: promoted %+v to primary", promotedInstance.Key) + topologyRecovery.SuccessorKey = &promotedInstance.Key + + // --- Reconfigure remaining standbys --- + for _, standby := range standbys { + if standby.Key.Equals(&promotedInstance.Key) { + continue + } + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("reconfiguring standby %+v to replicate from new primary %+v", standby.Key, promotedInstance.Key)) + if err := inst.PostgreSQLReconfigureStandby(&standby.Key, &promotedInstance.Key); err != nil { + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("error reconfiguring standby %+v: %v (continuing)", standby.Key, err)) + topologyRecovery.LostReplicas.AddKey(standby.Key) + } + } + + // --- Configure demoted primary for repositioning --- + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("configuring demoted primary %+v to replicate from %+v", clusterPrimary.Key, promotedInstance.Key)) + if err := inst.PostgreSQLRepositionAsStandby(&clusterPrimary.Key, &promotedInstance.Key); err != nil { + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("error configuring demoted primary %+v: %v", clusterPrimary.Key, err)) + } + + // --- Resolve recovery --- + resolveRecovery(topologyRecovery, promotedInstance) + + // --- Execute PostGracefulTakeoverProcesses --- + executeProcesses(config.Config.PostGracefulTakeoverProcesses, "PostGracefulTakeoverProcesses", topologyRecovery, false) + + AuditTopologyRecovery(topologyRecovery, fmt.Sprintf("PostgreSQLGracefulPrimarySwitchover: completed. New primary: %+v", promotedInstance.Key)) + return topologyRecovery, nil +} diff --git a/tests/functional/test-postgresql.sh b/tests/functional/test-postgresql.sh index 392ccb43..560d789d 100755 --- a/tests/functional/test-postgresql.sh +++ b/tests/functional/test-postgresql.sh @@ -117,7 +117,214 @@ test_body_contains "/api/clusters contains PG cluster" "$ORC_URL/api/clusters" " # ---------------------------------------------------------------- echo "" -echo "--- Failover test: kill pgprimary ---" +echo "--- Graceful primary switchover tests ---" + +# Identify current primary before switchover +CURRENT_PRIMARY=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +instances = json.load(sys.stdin) +for inst in instances: + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname'] + ':' + str(inst['Key']['Port'])) + sys.exit(0) +print('') +" 2>/dev/null || echo "") + +if [ -z "$CURRENT_PRIMARY" ]; then + fail "Cannot identify current PostgreSQL primary for graceful switchover" +else + echo "Current primary: $CURRENT_PRIMARY" + + # Execute graceful-master-takeover-auto via API + echo "Executing graceful-master-takeover-auto on cluster $PG_CLUSTER..." + TAKEOVER_RESULT=$(curl -s --max-time 60 "$ORC_URL/api/graceful-master-takeover-auto/$PG_CLUSTER" 2>/dev/null) + TAKEOVER_CODE=$(echo "$TAKEOVER_RESULT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('Code','ERROR'))" 2>/dev/null || echo "ERROR") + + if [ "$TAKEOVER_CODE" = "OK" ]; then + pass "Graceful master takeover API returned OK" + else + fail "Graceful master takeover API returned: $TAKEOVER_CODE — $TAKEOVER_RESULT" + fi + + # Wait for topology to settle and re-discover + sleep 10 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.20/5432" > /dev/null 2>&1 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.21/5432" > /dev/null 2>&1 + sleep 5 + + # Verify primary has changed + NEW_PRIMARY=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +instances = json.load(sys.stdin) +for inst in instances: + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname'] + ':' + str(inst['Key']['Port'])) + sys.exit(0) +print('') +" 2>/dev/null || echo "") + + if [ -n "$NEW_PRIMARY" ] && [ "$NEW_PRIMARY" != "$CURRENT_PRIMARY" ]; then + pass "Primary switched from $CURRENT_PRIMARY to $NEW_PRIMARY" + else + fail "Primary did not change: was $CURRENT_PRIMARY, now ${NEW_PRIMARY:-unknown}" + fi + + # Verify new primary is actually writable (not just flagged read_only=false) + if $COMPOSE exec -T pgstandby1 psql -U postgres -v ON_ERROR_STOP=1 -tAc \ + "CREATE TABLE IF NOT EXISTS orc_switchover_probe (id int); INSERT INTO orc_switchover_probe VALUES (1);" >/dev/null 2>&1; then + pass "New primary (pgstandby1) accepts writes" + else + fail "New primary (pgstandby1) does not accept writes" + fi + + # Verify demoted primary has primary_conninfo pointing at the new primary + DEMOTED_CONNINFO=$($COMPOSE exec -T pgprimary psql -U postgres -tAc \ + "SELECT setting FROM pg_settings WHERE name='primary_conninfo';" 2>/dev/null | tr -d '\r') + if echo "$DEMOTED_CONNINFO" | grep -q "172.30.0.21"; then + pass "Demoted primary has primary_conninfo pointing at new primary" + else + fail "Demoted primary primary_conninfo missing new primary IP (got: '$DEMOTED_CONNINFO')" + fi +fi + +# ---------------------------------------------------------------- +echo "" +echo "--- Graceful switchover negative cases ---" + +# Negative case 1: bogus cluster name +BOGUS_RESULT=$(curl -s --max-time 30 "$ORC_URL/api/graceful-master-takeover-auto/no-such-cluster-xyz" 2>/dev/null) +BOGUS_CODE=$(echo "$BOGUS_RESULT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('Code','ERROR'))" 2>/dev/null || echo "ERROR") +if [ "$BOGUS_CODE" != "OK" ]; then + pass "graceful-master-takeover-auto on bogus cluster rejected (Code=$BOGUS_CODE)" +else + fail "graceful-master-takeover-auto on bogus cluster unexpectedly returned OK" +fi + +# Negative case 2: bogus designated host (PG_CLUSTER is valid, target is not) +BADHOST_RESULT=$(curl -s --max-time 30 "$ORC_URL/api/graceful-master-takeover/$PG_CLUSTER/no.such.host.invalid/5432" 2>/dev/null) +BADHOST_CODE=$(echo "$BADHOST_RESULT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('Code','ERROR'))" 2>/dev/null || echo "ERROR") +if [ "$BADHOST_CODE" != "OK" ]; then + pass "graceful-master-takeover with bogus designated host rejected (Code=$BADHOST_CODE)" +else + fail "graceful-master-takeover with bogus designated host unexpectedly returned OK" +fi + +# ---------------------------------------------------------------- +echo "" +echo "--- Graceful switchover round-trip (switch back) ---" +# After the first switchover, pgprimary has primary_conninfo set but is still +# running as a (read-only) primary — it needs standby.signal + restart to +# actually stream WAL from the new primary. Simulate what a +# PostGracefulTakeoverProcesses hook would do. + +if [ -n "${NEW_PRIMARY:-}" ] && [ "${NEW_PRIMARY:-}" != "${CURRENT_PRIMARY:-}" ]; then + echo "Converting demoted pgprimary into a live standby of pgstandby1..." + $COMPOSE exec -T pgprimary bash -c 'touch /var/lib/postgresql/data/standby.signal && chown postgres:postgres /var/lib/postgresql/data/standby.signal' || true + $COMPOSE restart pgprimary + echo "Waiting for pgprimary to become a healthy standby..." + STANDBY_READY=false + for i in $(seq 1 60); do + IN_RECOVERY=$($COMPOSE exec -T pgprimary psql -U postgres -tAc "SELECT pg_is_in_recovery();" 2>/dev/null | tr -d '[:space:]') + if [ "$IN_RECOVERY" = "t" ]; then + STANDBY_READY=true + echo "pgprimary is in recovery (standby mode) after ${i}s" + break + fi + sleep 1 + done + + if [ "$STANDBY_READY" != "true" ]; then + fail "pgprimary did not enter standby/recovery mode after restart" + else + pass "pgprimary restarted as a standby" + + # Let orchestrator re-discover the flipped topology + sleep 5 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.20/5432" > /dev/null 2>&1 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.21/5432" > /dev/null 2>&1 + sleep 8 + + # Verify orchestrator sees pgstandby1 as primary and pgprimary as standby + TOPOLOGY_OK=false + for i in $(seq 1 30); do + PRIMARY_HOST=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +for inst in json.load(sys.stdin): + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname']) + sys.exit(0) +" 2>/dev/null || echo "") + if [ "$PRIMARY_HOST" = "172.30.0.21" ] || [ "$PRIMARY_HOST" = "pgstandby1" ]; then + TOPOLOGY_OK=true + break + fi + sleep 1 + done + + if [ "$TOPOLOGY_OK" = "true" ]; then + pass "Orchestrator sees pgstandby1 as primary after round-trip setup" + else + fail "Orchestrator does not see pgstandby1 as primary (got: ${PRIMARY_HOST:-unknown})" + fi + + # Now switch back: pgstandby1 → pgprimary + if [ "$TOPOLOGY_OK" = "true" ]; then + echo "Executing graceful-master-takeover-auto to switch back..." + BACK_RESULT=$(curl -s --max-time 60 "$ORC_URL/api/graceful-master-takeover-auto/$PG_CLUSTER" 2>/dev/null) + BACK_CODE=$(echo "$BACK_RESULT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('Code','ERROR'))" 2>/dev/null || echo "ERROR") + + if [ "$BACK_CODE" = "OK" ]; then + pass "Round-trip graceful takeover API returned OK" + else + fail "Round-trip graceful takeover returned: $BACK_CODE — $BACK_RESULT" + fi + + sleep 10 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.20/5432" > /dev/null 2>&1 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.21/5432" > /dev/null 2>&1 + sleep 5 + + FINAL_PRIMARY=$(curl -s --max-time 10 "$ORC_URL/api/cluster/$PG_CLUSTER" 2>/dev/null | python3 -c " +import json, sys +for inst in json.load(sys.stdin): + if not inst.get('ReadOnly', True): + print(inst['Key']['Hostname']) + sys.exit(0) +" 2>/dev/null || echo "") + + if [ "$FINAL_PRIMARY" = "172.30.0.20" ] || [ "$FINAL_PRIMARY" = "pgprimary" ]; then + pass "Round-trip complete: pgprimary is primary again" + else + fail "Round-trip incomplete: primary is '$FINAL_PRIMARY' (expected pgprimary)" + fi + + # After round-trip, pgstandby1 is the demoted primary — reactivate + # it as a live standby so the downstream failover-kill test has a + # replica to promote. + echo "Reactivating pgstandby1 as a live standby of pgprimary..." + $COMPOSE exec -T pgstandby1 bash -c 'touch /var/lib/postgresql/data/standby.signal && chown postgres:postgres /var/lib/postgresql/data/standby.signal' || true + $COMPOSE restart pgstandby1 + for i in $(seq 1 60); do + IN_RECOVERY=$($COMPOSE exec -T pgstandby1 psql -U postgres -tAc "SELECT pg_is_in_recovery();" 2>/dev/null | tr -d '[:space:]') + if [ "$IN_RECOVERY" = "t" ]; then + echo "pgstandby1 is streaming as a standby after ${i}s" + break + fi + sleep 1 + done + sleep 5 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.20/5432" > /dev/null 2>&1 + curl -s --max-time 10 "$ORC_URL/api/discover/172.30.0.21/5432" > /dev/null 2>&1 + sleep 5 + fi + fi +else + skip "Round-trip test skipped — first switchover did not complete" +fi + +# ---------------------------------------------------------------- +echo "" +echo "--- Failover test: kill current primary ---" # Static IPs are assigned in docker-compose.yml so the standby remains reachable # even when the primary container stops.