From 386c41a399626a3ada14d064deddc8f38e0a2173 Mon Sep 17 00:00:00 2001 From: Jason Lynch Date: Sun, 21 Dec 2025 16:16:16 -0500 Subject: [PATCH] fix: persist recovery opts in in-place restore It's not possible to persist recovery options outside of the bootstrap process and it's not possible to force Patroni into the bootstrap process if the pgdata directory is non-empty. So, this commit changes the in-place restore process to temporarily move the pgdata directory to a different name. Then, it changes the bootstrap command to simply move the pgdata directory back to its original name. This lets us keep our existing restore process and makes patroni keep the recovery options. PLAT-368 --- changes/unreleased/Fixed-20251221-163411.yaml | 3 +++ server/internal/database/spec.go | 1 + .../orchestrator/swarm/orchestrator.go | 12 +-------- .../orchestrator/swarm/patroni_config.go | 25 ++++++++++++------ .../orchestrator/swarm/pgbackrest_restore.go | 26 +++++++++++++++++++ 5 files changed, 48 insertions(+), 19 deletions(-) create mode 100644 changes/unreleased/Fixed-20251221-163411.yaml diff --git a/changes/unreleased/Fixed-20251221-163411.yaml b/changes/unreleased/Fixed-20251221-163411.yaml new file mode 100644 index 00000000..014a18c3 --- /dev/null +++ b/changes/unreleased/Fixed-20251221-163411.yaml @@ -0,0 +1,3 @@ +kind: Fixed +body: Fixed in-place restores where the recovery target is not the latest point in the repository. +time: 2025-12-21T16:34:11.393466-05:00 diff --git a/server/internal/database/spec.go b/server/internal/database/spec.go index c498e38f..edb18aa4 100644 --- a/server/internal/database/spec.go +++ b/server/internal/database/spec.go @@ -465,6 +465,7 @@ type InstanceSpec struct { PostgreSQLConf map[string]any `json:"postgresql_conf"` ClusterSize int `json:"cluster_size"` OrchestratorOpts *OrchestratorOpts `json:"orchestrator_opts,omitempty"` + InPlaceRestore bool `json:"in_place_restore,omitempty"` } type InstanceSpecChange struct { diff --git a/server/internal/orchestrator/swarm/orchestrator.go b/server/internal/orchestrator/swarm/orchestrator.go index 7b450585..c9c8a503 100644 --- a/server/internal/orchestrator/swarm/orchestrator.go +++ b/server/internal/orchestrator/swarm/orchestrator.go @@ -344,17 +344,7 @@ func (o *Orchestrator) GenerateInstanceRestoreResources(spec *database.InstanceS return nil, fmt.Errorf("missing restore config for node %s instance %s", spec.NodeName, spec.InstanceID) } - // TODO: I'd like Patroni to use the backup config that pgbackrest outputs, - // but it removes the postgresql.auto.conf when it starts bootstrapping the - // cluster. Setting the restore command ourselves is a decent workaround. - restoreCmd := PgBackRestRestoreCmd("archive-get", "%f", `"%p"`).String() - if spec.PostgreSQLConf == nil { - spec.PostgreSQLConf = map[string]any{ - "restore_command": restoreCmd, - } - } else { - spec.PostgreSQLConf["restore_command"] = restoreCmd - } + spec.InPlaceRestore = true instance, resources, err := o.instanceResources(spec) if err != nil { diff --git a/server/internal/orchestrator/swarm/patroni_config.go b/server/internal/orchestrator/swarm/patroni_config.go index a2ba445e..9971a5e8 100644 --- a/server/internal/orchestrator/swarm/patroni_config.go +++ b/server/internal/orchestrator/swarm/patroni_config.go @@ -475,15 +475,24 @@ func generatePatroniConfig( } if spec.RestoreConfig != nil { - restoreOptions := utils.BuildOptionArgs(spec.RestoreConfig.RestoreOptions) - for i, o := range restoreOptions { - restoreOptions[i] = shellescape.Quote(o) - } cfg.Bootstrap.Method = utils.PointerTo(patroni.BootstrapMethodNameRestore) - cfg.Bootstrap.Restore = &patroni.BootstrapMethodConf{ - Command: utils.PointerTo(PgBackRestRestoreCmd("restore", restoreOptions...).String()), - NoParams: utils.PointerTo(true), - KeepExistingRecoveryConf: utils.PointerTo(true), + + if spec.InPlaceRestore { + cfg.Bootstrap.Restore = &patroni.BootstrapMethodConf{ + Command: utils.PointerTo("mv /opt/pgedge/data/pgdata-restore /opt/pgedge/data/pgdata"), + NoParams: utils.PointerTo(true), + KeepExistingRecoveryConf: utils.PointerTo(true), + } + } else { + restoreOptions := utils.BuildOptionArgs(spec.RestoreConfig.RestoreOptions) + for i, o := range restoreOptions { + restoreOptions[i] = shellescape.Quote(o) + } + cfg.Bootstrap.Restore = &patroni.BootstrapMethodConf{ + Command: utils.PointerTo(PgBackRestRestoreCmd("restore", restoreOptions...).String()), + NoParams: utils.PointerTo(true), + KeepExistingRecoveryConf: utils.PointerTo(true), + } } } diff --git a/server/internal/orchestrator/swarm/pgbackrest_restore.go b/server/internal/orchestrator/swarm/pgbackrest_restore.go index c1547d36..e09fd759 100644 --- a/server/internal/orchestrator/swarm/pgbackrest_restore.go +++ b/server/internal/orchestrator/swarm/pgbackrest_restore.go @@ -121,6 +121,11 @@ func (p *PgBackRestRestore) Create(ctx context.Context, rc *resource.Context) er return handleError(err) } + err = p.renameDataDir(rc) + if err != nil { + return handleError(err) + } + err = p.completeTask(ctx, taskSvc, t) if err != nil { return handleError(err) @@ -282,6 +287,27 @@ func (p *PgBackRestRestore) streamLogsAndWait( return nil } +func (p *PgBackRestRestore) renameDataDir(rc *resource.Context) error { + fs, err := do.Invoke[afero.Fs](rc.Injector) + if err != nil { + return err + } + + dataDirPath, err := filesystem.DirResourceFullPath(rc, p.InstanceID+"-data") + if err != nil { + return fmt.Errorf("failed to get data dir from state: %w", err) + } + + pgdataPath := filepath.Join(dataDirPath, "pgdata") + newPgdataPath := filepath.Join(dataDirPath, "pgdata-restore") + + if err := fs.Rename(pgdataPath, newPgdataPath); err != nil { + return fmt.Errorf("failed to rename pgdata for restore: %w", err) + } + + return nil +} + func (p *PgBackRestRestore) Update(ctx context.Context, rc *resource.Context) error { return nil }