From cdf2902113349e3daa5b81e9fb9232dcd83e2e36 Mon Sep 17 00:00:00 2001 From: akartasov Date: Wed, 23 Mar 2022 16:47:32 +0700 Subject: [PATCH 1/3] fix(engine): use the correct path to the data directory for the promote instance (#342) --- engine/.gitlab-ci.yml | 4 ++-- engine/.golangci.yml | 2 +- engine/configs/config.example.physical_pgbackrest.yml | 1 + .../retrieval/engine/postgres/physical/pgbackrest.go | 8 +++++--- .../retrieval/engine/postgres/physical/pgbackrest_test.go | 8 ++++---- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/engine/.gitlab-ci.yml b/engine/.gitlab-ci.yml index 1f0cd81e2..f474f6930 100644 --- a/engine/.gitlab-ci.yml +++ b/engine/.gitlab-ci.yml @@ -363,8 +363,8 @@ build-image-swagger-latest: paths: - engine/bin script: - - bash engine/test/1.synthetic.sh - - bash engine/test/2.logical_generic.sh +# - bash engine/test/1.synthetic.sh +# - bash engine/test/2.logical_generic.sh - bash engine/test/4.physical_basebackup.sh after_script: - bash engine/test/_cleanup.sh diff --git a/engine/.golangci.yml b/engine/.golangci.yml index 2b9a46df2..e5a180f7f 100644 --- a/engine/.golangci.yml +++ b/engine/.golangci.yml @@ -82,7 +82,7 @@ linters: - megacheck - misspell - prealloc - - revive +# - revive # temporarily disabled: https://gitlab.com/postgres-ai/database-lab/-/merge_requests/498 - structcheck - stylecheck - unconvert diff --git a/engine/configs/config.example.physical_pgbackrest.yml b/engine/configs/config.example.physical_pgbackrest.yml index d95b81168..97a5d822b 100644 --- a/engine/configs/config.example.physical_pgbackrest.yml +++ b/engine/configs/config.example.physical_pgbackrest.yml @@ -277,6 +277,7 @@ retrieval: # Add PostgreSQL recovery configuration parameters to the promotion container. recovery: + restore_command: 'pgbackrest --pg1-path=${PGDATA} --stanza=my_stanza archive-get %f %p' # Uncomment this only if you are on Postgres version 11 or older. # recovery_target: 'immediate' # recovery_target_action: 'promote' diff --git a/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go b/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go index 1c2f23521..192fa4af7 100644 --- a/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go +++ b/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go @@ -34,17 +34,19 @@ func newPgBackRest(pgDataDir string, options pgbackrestOptions) *pgbackrest { // GetRestoreCommand returns a command to restore data. func (p *pgbackrest) GetRestoreCommand() string { + restoreCmd := fmt.Sprintf("sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=%s restore", p.options.Stanza) + if p.options.ForceInit { - return fmt.Sprintf("sudo -Eu postgres pgbackrest --delta --type=standby --pg1-path=%s --stanza=%s restore", p.pgDataDir, p.options.Stanza) + restoreCmd += " --delta" } - return fmt.Sprintf("sudo -Eu postgres pgbackrest --type=standby --pg1-path=%s --stanza=%s restore", p.pgDataDir, p.options.Stanza) + return restoreCmd } // GetRecoveryConfig returns a recovery config to restore data. func (p *pgbackrest) GetRecoveryConfig(pgVersion float64) map[string]string { recoveryCfg := map[string]string{ - "restore_command": fmt.Sprintf("pgbackrest --pg1-path=%s --stanza=%s archive-get %%f %%p", p.pgDataDir, p.options.Stanza), + "restore_command": fmt.Sprintf("pgbackrest --pg1-path=${PGDATA} --stanza=%s archive-get %%f %%p", p.options.Stanza), } if pgVersion < defaults.PGVersion12 { diff --git a/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go b/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go index abc27f955..e74c0c1b3 100644 --- a/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go +++ b/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go @@ -11,14 +11,14 @@ func TestPgBackRestRecoveryConfig(t *testing.T) { recoveryConfig := pgbackrest.GetRecoveryConfig(11.7) expectedResponse11 := map[string]string{ - "restore_command": "pgbackrest --pg1-path=dataDir --stanza=stanzaName archive-get %f %p", + "restore_command": "pgbackrest --pg1-path=${PGDATA} --stanza=stanzaName archive-get %f %p", "recovery_target_timeline": "latest", } assert.Equal(t, expectedResponse11, recoveryConfig) recoveryConfig = pgbackrest.GetRecoveryConfig(12.3) expectedResponse12 := map[string]string{ - "restore_command": "pgbackrest --pg1-path=dataDir --stanza=stanzaName archive-get %f %p", + "restore_command": "pgbackrest --pg1-path=${PGDATA} --stanza=stanzaName archive-get %f %p", } assert.Equal(t, expectedResponse12, recoveryConfig) } @@ -27,11 +27,11 @@ func TestPgBackRestRestoreCommand(t *testing.T) { pgbackrest := newPgBackRest("dataDir", pgbackrestOptions{Stanza: "stanzaName"}) restoreCmd := pgbackrest.GetRestoreCommand() - expectedResponse := "sudo -Eu postgres pgbackrest --type=standby --pg1-path=dataDir --stanza=stanzaName restore" + expectedResponse := "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore" assert.Equal(t, expectedResponse, restoreCmd) pgbackrest.options.ForceInit = true restoreCmd = pgbackrest.GetRestoreCommand() - expectedResponse = "sudo -Eu postgres pgbackrest --delta --type=standby --pg1-path=dataDir --stanza=stanzaName restore" + expectedResponse = "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore --delta" assert.Equal(t, expectedResponse, restoreCmd) } -- GitLab From 4905ed6641a0aeac28f26ae5246cf00875a5227a Mon Sep 17 00:00:00 2001 From: akartasov Date: Wed, 23 Mar 2022 18:07:15 +0700 Subject: [PATCH 2/3] fix recovery option --- engine/.gitlab-ci.yml | 4 ++-- engine/configs/config.example.physical_pgbackrest.yml | 1 - .../retrieval/engine/postgres/physical/pgbackrest.go | 11 +++++------ .../engine/postgres/physical/pgbackrest_test.go | 10 ++++++---- .../retrieval/engine/postgres/physical/physical.go | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/engine/.gitlab-ci.yml b/engine/.gitlab-ci.yml index f474f6930..1f0cd81e2 100644 --- a/engine/.gitlab-ci.yml +++ b/engine/.gitlab-ci.yml @@ -363,8 +363,8 @@ build-image-swagger-latest: paths: - engine/bin script: -# - bash engine/test/1.synthetic.sh -# - bash engine/test/2.logical_generic.sh + - bash engine/test/1.synthetic.sh + - bash engine/test/2.logical_generic.sh - bash engine/test/4.physical_basebackup.sh after_script: - bash engine/test/_cleanup.sh diff --git a/engine/configs/config.example.physical_pgbackrest.yml b/engine/configs/config.example.physical_pgbackrest.yml index 97a5d822b..d95b81168 100644 --- a/engine/configs/config.example.physical_pgbackrest.yml +++ b/engine/configs/config.example.physical_pgbackrest.yml @@ -277,7 +277,6 @@ retrieval: # Add PostgreSQL recovery configuration parameters to the promotion container. recovery: - restore_command: 'pgbackrest --pg1-path=${PGDATA} --stanza=my_stanza archive-get %f %p' # Uncomment this only if you are on Postgres version 11 or older. # recovery_target: 'immediate' # recovery_target_action: 'promote' diff --git a/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go b/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go index 192fa4af7..07f13c804 100644 --- a/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go +++ b/engine/internal/retrieval/engine/postgres/physical/pgbackrest.go @@ -16,8 +16,7 @@ const ( // pgbackrest defines a pgBackRest as an archival restoration tool. type pgbackrest struct { - pgDataDir string - options pgbackrestOptions + options pgbackrestOptions } type pgbackrestOptions struct { @@ -25,16 +24,16 @@ type pgbackrestOptions struct { ForceInit bool `yaml:"forceInit"` } -func newPgBackRest(pgDataDir string, options pgbackrestOptions) *pgbackrest { +func newPgBackRest(options pgbackrestOptions) *pgbackrest { return &pgbackrest{ - pgDataDir: pgDataDir, - options: options, + options: options, } } // GetRestoreCommand returns a command to restore data. func (p *pgbackrest) GetRestoreCommand() string { - restoreCmd := fmt.Sprintf("sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=%s restore", p.options.Stanza) + restoreCmd := fmt.Sprintf("sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=%[1]s restore "+ + "--recovery-option=restore_command='pgbackrest --pg1-path=${PGDATA} --stanza=%[1]s archive-get %%f %%p'", p.options.Stanza) if p.options.ForceInit { restoreCmd += " --delta" diff --git a/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go b/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go index e74c0c1b3..2b8ca6d86 100644 --- a/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go +++ b/engine/internal/retrieval/engine/postgres/physical/pgbackrest_test.go @@ -7,7 +7,7 @@ import ( ) func TestPgBackRestRecoveryConfig(t *testing.T) { - pgbackrest := newPgBackRest("dataDir", pgbackrestOptions{Stanza: "stanzaName"}) + pgbackrest := newPgBackRest(pgbackrestOptions{Stanza: "stanzaName"}) recoveryConfig := pgbackrest.GetRecoveryConfig(11.7) expectedResponse11 := map[string]string{ @@ -24,14 +24,16 @@ func TestPgBackRestRecoveryConfig(t *testing.T) { } func TestPgBackRestRestoreCommand(t *testing.T) { - pgbackrest := newPgBackRest("dataDir", pgbackrestOptions{Stanza: "stanzaName"}) + pgbackrest := newPgBackRest(pgbackrestOptions{Stanza: "stanzaName"}) restoreCmd := pgbackrest.GetRestoreCommand() - expectedResponse := "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore" + expectedResponse := "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore " + + "--recovery-option=restore_command='pgbackrest --pg1-path=${PGDATA} --stanza=stanzaName archive-get %f %p'" assert.Equal(t, expectedResponse, restoreCmd) pgbackrest.options.ForceInit = true restoreCmd = pgbackrest.GetRestoreCommand() - expectedResponse = "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore --delta" + expectedResponse = "sudo -Eu postgres pgbackrest --type=standby --pg1-path=${PGDATA} --stanza=stanzaName restore " + + "--recovery-option=restore_command='pgbackrest --pg1-path=${PGDATA} --stanza=stanzaName archive-get %f %p' --delta" assert.Equal(t, expectedResponse, restoreCmd) } diff --git a/engine/internal/retrieval/engine/postgres/physical/physical.go b/engine/internal/retrieval/engine/postgres/physical/physical.go index be7a7f5e6..bd03e2618 100644 --- a/engine/internal/retrieval/engine/postgres/physical/physical.go +++ b/engine/internal/retrieval/engine/postgres/physical/physical.go @@ -132,7 +132,7 @@ func (r *RestoreJob) getRestorer(tool string) (restorer, error) { return newWALG(r.fsPool.DataDir(), r.WALG), nil case pgbackrestTool: - return newPgBackRest(r.fsPool.DataDir(), r.PgBackRest), nil + return newPgBackRest(r.PgBackRest), nil case customTool: return newCustomTool(r.CustomTool), nil -- GitLab From d0064cc3f6c8a8b14b12b00f2187bbabf6ff589d Mon Sep 17 00:00:00 2001 From: akartasov Date: Thu, 24 Mar 2022 10:25:27 +0700 Subject: [PATCH 3/3] fix linter --- engine/.golangci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/.golangci.yml b/engine/.golangci.yml index e5a180f7f..2b9a46df2 100644 --- a/engine/.golangci.yml +++ b/engine/.golangci.yml @@ -82,7 +82,7 @@ linters: - megacheck - misspell - prealloc -# - revive # temporarily disabled: https://gitlab.com/postgres-ai/database-lab/-/merge_requests/498 + - revive - structcheck - stylecheck - unconvert -- GitLab