From b8843b060a40bb8ae728f9bbdec945f034d43c0e Mon Sep 17 00:00:00 2001 From: akartasov Date: Mon, 6 Mar 2023 08:51:06 -0300 Subject: [PATCH 1/4] fix: logical restore --- .../retrieval/engine/postgres/logical/dump.go | 6 +----- .../engine/postgres/logical/restore.go | 11 ++--------- .../engine/postgres/logical/restore_test.go | 10 +++++----- engine/internal/retrieval/retrieval.go | 18 ++++++++++++++++-- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/engine/internal/retrieval/engine/postgres/logical/dump.go b/engine/internal/retrieval/engine/postgres/logical/dump.go index 6747af38b..50001e8b3 100644 --- a/engine/internal/retrieval/engine/postgres/logical/dump.go +++ b/engine/internal/retrieval/engine/postgres/logical/dump.go @@ -719,17 +719,13 @@ func (d *DumpJob) buildLogicalDumpCommand(dbName string, dump DumpDefinition) [] } func (d *DumpJob) buildLogicalRestoreCommand(dbName string) []string { - restoreCmd := []string{"|", "pg_restore", "--username", d.globalCfg.Database.User(), "--dbname", defaults.DBName} + restoreCmd := []string{"|", "pg_restore", "--username", d.globalCfg.Database.User(), "--dbname", dbName} if dbName != defaults.DBName { // To avoid recreating of the default database. restoreCmd = append(restoreCmd, "--create") } - if d.Restore.ForceInit { - restoreCmd = append(restoreCmd, "--clean", "--if-exists") - } - restoreCmd = append(restoreCmd, d.DumpOptions.Restore.CustomOptions...) return restoreCmd diff --git a/engine/internal/retrieval/engine/postgres/logical/restore.go b/engine/internal/retrieval/engine/postgres/logical/restore.go index 1c6824d38..1c6a70703 100644 --- a/engine/internal/retrieval/engine/postgres/logical/restore.go +++ b/engine/internal/retrieval/engine/postgres/logical/restore.go @@ -204,10 +204,6 @@ func (r *RestoreJob) Run(ctx context.Context) (err error) { } log.Msg(fmt.Sprintf("The data directory %q is not empty. Existing data may be overwritten.", dataDir)) - - if err := updateConfigs(dataDir, r.RestoreOptions.Configs); err != nil { - return fmt.Errorf("failed to update configuration: %w", err) - } } if err := tools.PullImage(ctx, r.dockerClient, r.RestoreOptions.DockerImage); err != nil { @@ -740,17 +736,14 @@ func (r *RestoreJob) buildPlainTextCommand(dumpName string, definition DumpDefin } func (r *RestoreJob) buildPGRestoreCommand(dumpName string, definition DumpDefinition) []string { - restoreCmd := []string{"pg_restore", "--username", r.globalCfg.Database.User(), "--dbname", defaults.DBName} + // TODO: check definition db name. + restoreCmd := []string{"pg_restore", "--username", r.globalCfg.Database.User(), "--dbname", definition.dbName} if definition.dbName != defaults.DBName { // To avoid recreating of the default database. restoreCmd = append(restoreCmd, "--create") } - if r.ForceInit { - restoreCmd = append(restoreCmd, "--clean", "--if-exists") - } - restoreCmd = append(restoreCmd, "--jobs", strconv.Itoa(r.ParallelJobs)) if len(definition.Tables) > 0 { diff --git a/engine/internal/retrieval/engine/postgres/logical/restore_test.go b/engine/internal/retrieval/engine/postgres/logical/restore_test.go index 03e40299d..382795311 100644 --- a/engine/internal/retrieval/engine/postgres/logical/restore_test.go +++ b/engine/internal/retrieval/engine/postgres/logical/restore_test.go @@ -43,14 +43,14 @@ func TestRestoreCommandBuilding(t *testing.T) { DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "1", "/tmp/db.dump", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "1", "/tmp/db.dump", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ ParallelJobs: 4, ForceInit: true, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--clean", "--if-exists", "--jobs", "4"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "4"}, }, { copyOptions: RestoreOptions{ @@ -60,7 +60,7 @@ func TestRestoreCommandBuilding(t *testing.T) { DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "2", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "2", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ @@ -74,7 +74,7 @@ func TestRestoreCommandBuilding(t *testing.T) { DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "1", "--table", "test", "--table", "users", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "1", "--table", "test", "--table", "users", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ @@ -184,7 +184,7 @@ func TestDumpCommandBuilding(t *testing.T) { }, CustomOptions: []string{"--exclude-scheme=test-scheme"}, }, - command: []string{"sh", "-c", "pg_dump --create --host localhost --port 5432 --username john --dbname testDB --jobs 1 --table test --table users --exclude-table test2 --exclude-table users2 --exclude-scheme=test-scheme --format custom | pg_restore --username postgres --dbname postgres --create --no-privileges --no-owner --exit-on-error"}, + command: []string{"sh", "-c", "pg_dump --create --host localhost --port 5432 --username john --dbname testDB --jobs 1 --table test --table users --exclude-table test2 --exclude-table users2 --exclude-scheme=test-scheme --format custom | pg_restore --username postgres --dbname testDB --create --no-privileges --no-owner --exit-on-error"}, }, } diff --git a/engine/internal/retrieval/retrieval.go b/engine/internal/retrieval/retrieval.go index d671c31c9..2c6c5e273 100644 --- a/engine/internal/retrieval/retrieval.go +++ b/engine/internal/retrieval/retrieval.go @@ -221,6 +221,12 @@ func (r *Retrieval) Run(ctx context.Context) error { return nil } + if r.State.Mode == models.Logical { + if err := preparePoolToRefresh(fsManager, r.runner); err != nil { + return fmt.Errorf("failed to prepare the pool to an initial refresh: %w", err) + } + } + if err := r.run(runCtx, fsManager); err != nil { r.State.addAlert(telemetry.Alert{Level: models.RefreshFailed, Message: err.Error()}) // Build a generic message to avoid sending sensitive data. @@ -608,7 +614,7 @@ func (r *Retrieval) FullRefresh(ctx context.Context) error { log.Msg("Pool to a full refresh: ", poolToUpdate.Pool()) - if err := preparePoolToRefresh(poolToUpdate); err != nil { + if err := preparePoolToRefresh(poolToUpdate, r.runner); err != nil { return errors.Wrap(err, "failed to prepare the pool to a full refresh") } @@ -641,7 +647,7 @@ func (r *Retrieval) stopScheduler() { } } -func preparePoolToRefresh(poolToUpdate pool.FSManager) error { +func preparePoolToRefresh(poolToUpdate pool.FSManager, runner runners.Runner) error { cloneList, err := poolToUpdate.ListClonesNames() if err != nil { return errors.Wrap(err, "failed to check running clones") @@ -652,6 +658,10 @@ func preparePoolToRefresh(poolToUpdate pool.FSManager) error { strings.Join(cloneList, " ")) } + if _, err := runner.Run("rm -rf " + poolToUpdate.Pool().DataDir() + "/*"); err != nil { + return errors.Wrap(err, "failed to clean unix socket directory") + } + poolToUpdate.RefreshSnapshotList() snapshots := poolToUpdate.SnapshotList() @@ -660,7 +670,11 @@ func preparePoolToRefresh(poolToUpdate pool.FSManager) error { return nil } + log.Msg("Preparing pool to full data refresh and destroying existing snapshots") + for _, snapshotEntry := range snapshots { + log.Msg("Destroying snapshot:", snapshotEntry.ID) + if err := poolToUpdate.DestroySnapshot(snapshotEntry.ID); err != nil { return errors.Wrap(err, "failed to destroy the existing snapshot") } -- GitLab From cf7d2fb8b0da89874baf34e9f977421c2a17c7a5 Mon Sep 17 00:00:00 2001 From: akartasov Date: Mon, 6 Mar 2023 13:44:09 -0300 Subject: [PATCH 2/4] fix: move preparing commands to refreshing data step --- engine/internal/retrieval/retrieval.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/engine/internal/retrieval/retrieval.go b/engine/internal/retrieval/retrieval.go index 2c6c5e273..a52ed2239 100644 --- a/engine/internal/retrieval/retrieval.go +++ b/engine/internal/retrieval/retrieval.go @@ -221,12 +221,6 @@ func (r *Retrieval) Run(ctx context.Context) error { return nil } - if r.State.Mode == models.Logical { - if err := preparePoolToRefresh(fsManager, r.runner); err != nil { - return fmt.Errorf("failed to prepare the pool to an initial refresh: %w", err) - } - } - if err := r.run(runCtx, fsManager); err != nil { r.State.addAlert(telemetry.Alert{Level: models.RefreshFailed, Message: err.Error()}) // Build a generic message to avoid sending sensitive data. @@ -399,6 +393,12 @@ func (r *Retrieval) RefreshData(ctx context.Context, poolName string) error { r.State.CurrentJob = nil }() + if r.State.Mode == models.Logical { + if err := preparePoolToRefresh(fsm, r.runner); err != nil { + return fmt.Errorf("failed to prepare the pool to an initial refresh: %w", err) + } + } + for _, j := range jobs { r.State.CurrentJob = j -- GitLab From f9e4de789cfbe8f79a0618c125907d789af2ddcc Mon Sep 17 00:00:00 2001 From: akartasov Date: Mon, 6 Mar 2023 15:14:52 -0300 Subject: [PATCH 3/4] fix: change options: add skipStartRefresh, remove forceInit --- .../config.example.logical_generic.yml | 11 +++----- .../config.example.logical_rds_iam.yml | 10 +++----- engine/internal/retrieval/config/config.go | 5 ++-- .../internal/retrieval/dbmarker/dbmarker.go | 7 +++--- .../retrieval/engine/postgres/logical/dump.go | 9 ++----- .../engine/postgres/logical/restore.go | 12 +++------ .../engine/postgres/logical/restore_test.go | 11 +++----- engine/internal/retrieval/retrieval.go | 25 +++++++++++++------ engine/internal/retrieval/validator.go | 2 +- 9 files changed, 41 insertions(+), 51 deletions(-) diff --git a/engine/configs/config.example.logical_generic.yml b/engine/configs/config.example.logical_generic.yml index 0cc979521..4771626e9 100644 --- a/engine/configs/config.example.logical_generic.yml +++ b/engine/configs/config.example.logical_generic.yml @@ -185,6 +185,9 @@ retrieval: # Timetable is to be defined in crontab format: https://en.wikipedia.org/wiki/Cron#Overview timetable: "0 0 * * 1" + # Skip data refresh while the retrieval starts. + skipStartRefresh: false + # The jobs section must not contain physical and logical restore jobs simultaneously. jobs: - logicalDump @@ -252,9 +255,6 @@ retrieval: # immediateRestore: # # Enable immediate restore. # enabled: true - # # Restore data even if the Postgres directory (`global.dataDir`) is not empty. - # # Note the existing data will be overwritten. - # forceInit: false # # Option to adjust PostgreSQL configuration for a logical dump job. # # It's useful if a dumped database contains non-standard extensions. # <<: *db_configs @@ -281,11 +281,6 @@ retrieval: # If your machine with DLE has 4 vCPUs or less, and you don't want to saturate them, use 2 or 1. parallelJobs: 4 - - # Restore data even if the Postgres directory (`global.dataDir`) is not empty. - # Note the existing data will be overwritten. - forceInit: false - # Ignore errors that occurred during logical data restore. Do not ignore by default. ignoreErrors: false diff --git a/engine/configs/config.example.logical_rds_iam.yml b/engine/configs/config.example.logical_rds_iam.yml index 2f1123f81..20f02e83e 100644 --- a/engine/configs/config.example.logical_rds_iam.yml +++ b/engine/configs/config.example.logical_rds_iam.yml @@ -185,6 +185,9 @@ retrieval: # Timetable is to be defined in crontab format: https://en.wikipedia.org/wiki/Cron#Overview timetable: "0 0 * * 1" + # Skip data refresh while the retrieval starts. + skipStartRefresh: false + # The jobs section must not contain physical and logical restore jobs simultaneously. jobs: - logicalDump @@ -249,9 +252,6 @@ retrieval: # immediateRestore: # # Enable immediate restore. # enabled: true - # # Restore data even if the Postgres directory (`global.dataDir`) is not empty. - # # Note the existing data will be overwritten. - # forceInit: false # # Option to adjust PostgreSQL configuration for a logical dump job. # # It's useful if a dumped database contains non-standard extensions. # <<: *db_configs @@ -277,10 +277,6 @@ retrieval: # If your machine with DLE has 4 vCPUs or less, and you don't want to saturate them, use 2 or 1. parallelJobs: 4 - # Restore data even if the Postgres directory (`global.dataDir`) is not empty. - # Note the existing data will be overwritten. - forceInit: false - # Ignore errors that occurred during logical data restore. Do not ignore by default. ignoreErrors: false diff --git a/engine/internal/retrieval/config/config.go b/engine/internal/retrieval/config/config.go index caa859e93..6d2d06d49 100644 --- a/engine/internal/retrieval/config/config.go +++ b/engine/internal/retrieval/config/config.go @@ -14,14 +14,15 @@ import ( // Config describes of data retrieval jobs. type Config struct { - Refresh Refresh `yaml:"refresh"` + Refresh *Refresh `yaml:"refresh"` Jobs []string `yaml:"jobs,flow"` JobsSpec map[string]JobSpec `yaml:"spec"` } // Refresh describes full-refresh options. type Refresh struct { - Timetable string `yaml:"timetable"` + Timetable string `yaml:"timetable"` + SkipStartRefresh bool `yaml:"skipStartRefresh"` } // JobSpec contains details about a job. diff --git a/engine/internal/retrieval/dbmarker/dbmarker.go b/engine/internal/retrieval/dbmarker/dbmarker.go index dee938a70..bf56f28b0 100644 --- a/engine/internal/retrieval/dbmarker/dbmarker.go +++ b/engine/internal/retrieval/dbmarker/dbmarker.go @@ -32,7 +32,8 @@ type Config struct { } const ( - configDir = ".dblab" + // ConfigDir defines the name of the dbMarker configuration directory. + ConfigDir = ".dblab" configFilename = "dbmarker" // LogicalDataType defines a logical data type. @@ -44,7 +45,7 @@ const ( // Init inits DB marker for the data directory. func (m *Marker) initDBLabDirectory() error { - dirname := path.Join(m.dataPath, configDir) + dirname := path.Join(m.dataPath, ConfigDir) if err := os.MkdirAll(dirname, 0755); err != nil { return errors.Wrapf(err, "cannot create a DBMarker directory %s", dirname) } @@ -104,5 +105,5 @@ func (m *Marker) SaveConfig(cfg *Config) error { // buildFileName builds a DBMarker config filename. func (m *Marker) buildFileName() string { - return path.Join(m.dataPath, configDir, configFilename) + return path.Join(m.dataPath, ConfigDir, configFilename) } diff --git a/engine/internal/retrieval/engine/postgres/logical/dump.go b/engine/internal/retrieval/engine/postgres/logical/dump.go index 50001e8b3..00b9a0e25 100644 --- a/engine/internal/retrieval/engine/postgres/logical/dump.go +++ b/engine/internal/retrieval/engine/postgres/logical/dump.go @@ -136,7 +136,6 @@ type Connection struct { // ImmediateRestore contains options for direct data restore without saving the dump file on disk. type ImmediateRestore struct { Enabled bool `yaml:"enabled"` - ForceInit bool `yaml:"forceInit"` Configs map[string]string `yaml:"configs"` CustomOptions []string `yaml:"customOptions"` } @@ -280,11 +279,7 @@ func (d *DumpJob) Run(ctx context.Context) (err error) { } if d.DumpOptions.Restore.Enabled && !isEmpty { - if !d.DumpOptions.Restore.ForceInit { - return errors.New("the data directory is not empty. Use 'forceInit' or empty the data directory") - } - - log.Msg("The data directory is not empty. Existing data may be overwritten.") + log.Warn("The data directory is not empty. Existing data may be overwritten.") if err := updateConfigs(dataDir, d.DumpOptions.Restore.Configs); err != nil { return fmt.Errorf("failed to update configs: %w", err) @@ -719,7 +714,7 @@ func (d *DumpJob) buildLogicalDumpCommand(dbName string, dump DumpDefinition) [] } func (d *DumpJob) buildLogicalRestoreCommand(dbName string) []string { - restoreCmd := []string{"|", "pg_restore", "--username", d.globalCfg.Database.User(), "--dbname", dbName} + restoreCmd := []string{"|", "pg_restore", "--username", d.globalCfg.Database.User(), "--dbname", defaults.DBName} if dbName != defaults.DBName { // To avoid recreating of the default database. diff --git a/engine/internal/retrieval/engine/postgres/logical/restore.go b/engine/internal/retrieval/engine/postgres/logical/restore.go index 1c6a70703..5cacc3434 100644 --- a/engine/internal/retrieval/engine/postgres/logical/restore.go +++ b/engine/internal/retrieval/engine/postgres/logical/restore.go @@ -100,7 +100,6 @@ type RestoreOptions struct { DockerImage string `yaml:"dockerImage"` ContainerConfig map[string]interface{} `yaml:"containerConfig"` Databases map[string]DumpDefinition `yaml:"databases"` - ForceInit bool `yaml:"forceInit"` IgnoreErrors bool `yaml:"ignoreErrors"` ParallelJobs int `yaml:"parallelJobs"` Configs map[string]string `yaml:"configs"` @@ -198,12 +197,7 @@ func (r *RestoreJob) Run(ctx context.Context) (err error) { } if !isEmpty { - if !r.ForceInit { - return fmt.Errorf("the data directory %q is not empty. Use 'forceInit' or empty the data directory: %w", - dataDir, err) - } - - log.Msg(fmt.Sprintf("The data directory %q is not empty. Existing data may be overwritten.", dataDir)) + log.Warn(fmt.Sprintf("The data directory %q is not empty. Existing data may be overwritten.", dataDir)) } if err := tools.PullImage(ctx, r.dockerClient, r.RestoreOptions.DockerImage); err != nil { @@ -736,8 +730,8 @@ func (r *RestoreJob) buildPlainTextCommand(dumpName string, definition DumpDefin } func (r *RestoreJob) buildPGRestoreCommand(dumpName string, definition DumpDefinition) []string { - // TODO: check definition db name. - restoreCmd := []string{"pg_restore", "--username", r.globalCfg.Database.User(), "--dbname", definition.dbName} + // Using the default database name because the database for connection must exist. + restoreCmd := []string{"pg_restore", "--username", r.globalCfg.Database.User(), "--dbname", defaults.DBName} if definition.dbName != defaults.DBName { // To avoid recreating of the default database. diff --git a/engine/internal/retrieval/engine/postgres/logical/restore_test.go b/engine/internal/retrieval/engine/postgres/logical/restore_test.go index 382795311..55d07537a 100644 --- a/engine/internal/retrieval/engine/postgres/logical/restore_test.go +++ b/engine/internal/retrieval/engine/postgres/logical/restore_test.go @@ -34,7 +34,6 @@ func TestRestoreCommandBuilding(t *testing.T) { { copyOptions: RestoreOptions{ ParallelJobs: 1, - ForceInit: false, Databases: map[string]DumpDefinition{ "testDB": { Format: customFormat, @@ -43,24 +42,22 @@ func TestRestoreCommandBuilding(t *testing.T) { DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "1", "/tmp/db.dump", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "1", "/tmp/db.dump", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ ParallelJobs: 4, - ForceInit: true, }, command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "4"}, }, { copyOptions: RestoreOptions{ ParallelJobs: 2, - ForceInit: false, Databases: map[string]DumpDefinition{"testDB": {}}, DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "2", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "2", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ @@ -74,7 +71,7 @@ func TestRestoreCommandBuilding(t *testing.T) { DumpLocation: "/tmp/db.dump", CustomOptions: []string{"--no-privileges", "--no-owner", "--exit-on-error"}, }, - command: []string{"pg_restore", "--username", "john", "--dbname", "", "--create", "--jobs", "1", "--table", "test", "--table", "users", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, + command: []string{"pg_restore", "--username", "john", "--dbname", "postgres", "--create", "--jobs", "1", "--table", "test", "--table", "users", "/tmp/db.dump/testDB", "--no-privileges", "--no-owner", "--exit-on-error"}, }, { copyOptions: RestoreOptions{ @@ -184,7 +181,7 @@ func TestDumpCommandBuilding(t *testing.T) { }, CustomOptions: []string{"--exclude-scheme=test-scheme"}, }, - command: []string{"sh", "-c", "pg_dump --create --host localhost --port 5432 --username john --dbname testDB --jobs 1 --table test --table users --exclude-table test2 --exclude-table users2 --exclude-scheme=test-scheme --format custom | pg_restore --username postgres --dbname testDB --create --no-privileges --no-owner --exit-on-error"}, + command: []string{"sh", "-c", "pg_dump --create --host localhost --port 5432 --username john --dbname testDB --jobs 1 --table test --table users --exclude-table test2 --exclude-table users2 --exclude-scheme=test-scheme --format custom | pg_restore --username postgres --dbname postgres --create --no-privileges --no-owner --exit-on-error"}, }, } diff --git a/engine/internal/retrieval/retrieval.go b/engine/internal/retrieval/retrieval.go index a52ed2239..19c38317a 100644 --- a/engine/internal/retrieval/retrieval.go +++ b/engine/internal/retrieval/retrieval.go @@ -190,6 +190,13 @@ func (r *Retrieval) Run(ctx context.Context) error { return fmt.Errorf("failed to collect content lists from the foundation Docker image of the logicalDump job: %w", err) } + if r.cfg.Refresh != nil && r.cfg.Refresh.SkipStartRefresh { + log.Msg("Continue without performing initial data refreshing because the `skipStartRefresh` option is enabled") + r.setupScheduler(ctx) + + return nil + } + fsManager, err := r.getNextPoolToDataRetrieving() if err != nil { var skipError *SkipRefreshingError @@ -536,7 +543,7 @@ func (r *Retrieval) defineRetrievalMode() { func (r *Retrieval) setupScheduler(ctx context.Context) { r.stopScheduler() - if r.cfg.Refresh.Timetable == "" { + if r.cfg.Refresh == nil || r.cfg.Refresh.Timetable == "" { return } @@ -614,10 +621,6 @@ func (r *Retrieval) FullRefresh(ctx context.Context) error { log.Msg("Pool to a full refresh: ", poolToUpdate.Pool()) - if err := preparePoolToRefresh(poolToUpdate, r.runner); err != nil { - return errors.Wrap(err, "failed to prepare the pool to a full refresh") - } - // Stop service containers: sync-instance, etc. if cleanUpErr := cont.CleanUpControlContainers(runCtx, r.docker, r.engineProps.InstanceID); cleanUpErr != nil { log.Err("Failed to clean up service containers:", cleanUpErr) @@ -658,7 +661,9 @@ func preparePoolToRefresh(poolToUpdate pool.FSManager, runner runners.Runner) er strings.Join(cloneList, " ")) } - if _, err := runner.Run("rm -rf " + poolToUpdate.Pool().DataDir() + "/*"); err != nil { + if _, err := runner.Run(fmt.Sprintf("rm -rf %s %s", + filepath.Join(poolToUpdate.Pool().DataDir(), "*"), + filepath.Join(poolToUpdate.Pool().DataDir(), dbmarker.ConfigDir))); err != nil { return errors.Wrap(err, "failed to clean unix socket directory") } @@ -685,9 +690,15 @@ func preparePoolToRefresh(poolToUpdate pool.FSManager, runner runners.Runner) er // ReportState collects the current restore state. func (r *Retrieval) ReportState() telemetry.Restore { + var refreshingTimetable string + + if r.cfg.Refresh != nil { + refreshingTimetable = r.cfg.Refresh.Timetable + } + return telemetry.Restore{ Mode: r.State.Mode, - Refreshing: r.cfg.Refresh.Timetable, + Refreshing: refreshingTimetable, Jobs: r.cfg.Jobs, } } diff --git a/engine/internal/retrieval/validator.go b/engine/internal/retrieval/validator.go index 2371a884a..0c62935e5 100644 --- a/engine/internal/retrieval/validator.go +++ b/engine/internal/retrieval/validator.go @@ -70,7 +70,7 @@ func validateStructure(r *config.Config) error { } func validateRefreshTimetable(r *config.Config) error { - if r.Refresh.Timetable == "" { + if r.Refresh == nil || r.Refresh.Timetable == "" { return nil } -- GitLab From d13cfca43cf541f97f2bd37a4f42ded114ffbc3e Mon Sep 17 00:00:00 2001 From: Nikolay Samokhvalov Date: Thu, 9 Mar 2023 09:39:40 +0000 Subject: [PATCH 4/4] Apply 5 suggestion(s) to 3 file(s) --- engine/internal/retrieval/engine/postgres/logical/dump.go | 2 +- .../internal/retrieval/engine/postgres/logical/restore.go | 2 +- engine/internal/retrieval/retrieval.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/engine/internal/retrieval/engine/postgres/logical/dump.go b/engine/internal/retrieval/engine/postgres/logical/dump.go index 00b9a0e25..2ab96ba28 100644 --- a/engine/internal/retrieval/engine/postgres/logical/dump.go +++ b/engine/internal/retrieval/engine/postgres/logical/dump.go @@ -279,7 +279,7 @@ func (d *DumpJob) Run(ctx context.Context) (err error) { } if d.DumpOptions.Restore.Enabled && !isEmpty { - log.Warn("The data directory is not empty. Existing data may be overwritten.") + log.Warn("The data directory is not empty. Existing data will be overwritten.") if err := updateConfigs(dataDir, d.DumpOptions.Restore.Configs); err != nil { return fmt.Errorf("failed to update configs: %w", err) diff --git a/engine/internal/retrieval/engine/postgres/logical/restore.go b/engine/internal/retrieval/engine/postgres/logical/restore.go index 5cacc3434..216ad5788 100644 --- a/engine/internal/retrieval/engine/postgres/logical/restore.go +++ b/engine/internal/retrieval/engine/postgres/logical/restore.go @@ -197,7 +197,7 @@ func (r *RestoreJob) Run(ctx context.Context) (err error) { } if !isEmpty { - log.Warn(fmt.Sprintf("The data directory %q is not empty. Existing data may be overwritten.", dataDir)) + log.Warn(fmt.Sprintf("The data directory %q is not empty. Existing data will be overwritten.", dataDir)) } if err := tools.PullImage(ctx, r.dockerClient, r.RestoreOptions.DockerImage); err != nil { diff --git a/engine/internal/retrieval/retrieval.go b/engine/internal/retrieval/retrieval.go index 19c38317a..bcac5b174 100644 --- a/engine/internal/retrieval/retrieval.go +++ b/engine/internal/retrieval/retrieval.go @@ -191,7 +191,7 @@ func (r *Retrieval) Run(ctx context.Context) error { } if r.cfg.Refresh != nil && r.cfg.Refresh.SkipStartRefresh { - log.Msg("Continue without performing initial data refreshing because the `skipStartRefresh` option is enabled") + log.Msg("Continue without performing initial data refresh because the `skipStartRefresh` option is enabled") r.setupScheduler(ctx) return nil @@ -402,7 +402,7 @@ func (r *Retrieval) RefreshData(ctx context.Context, poolName string) error { if r.State.Mode == models.Logical { if err := preparePoolToRefresh(fsm, r.runner); err != nil { - return fmt.Errorf("failed to prepare the pool to an initial refresh: %w", err) + return fmt.Errorf("failed to prepare pool for initial refresh: %w", err) } } @@ -675,7 +675,7 @@ func preparePoolToRefresh(poolToUpdate pool.FSManager, runner runners.Runner) er return nil } - log.Msg("Preparing pool to full data refresh and destroying existing snapshots") + log.Msg("Preparing pool for full data refresh; existing snapshots are to be destroyed") for _, snapshotEntry := range snapshots { log.Msg("Destroying snapshot:", snapshotEntry.ID) -- GitLab