From af53c7cc780f9fb9ff3844516cae1d103a32f979 Mon Sep 17 00:00:00 2001 From: Alexander Marshalov <_@marshalov.org> Date: Mon, 26 Jun 2023 14:44:02 +0200 Subject: [PATCH] fix removing storage data dir before restoring from backup (#598) * fix removing storage data dir before restoring from backup Signed-off-by: Alexander Marshalov <_@marshalov.org> * fix review comment Signed-off-by: Alexander Marshalov <_@marshalov.org> * fix review comment Signed-off-by: Alexander Marshalov <_@marshalov.org> * fixes after merge with `enterprise-single-node` branch Signed-off-by: Alexander Marshalov <_@marshalov.org> --------- Signed-off-by: Alexander Marshalov <_@marshalov.org> --- docs/CHANGELOG.md | 1 + lib/backup/actions/backup.go | 8 ++++---- lib/backup/actions/restore.go | 5 ++--- lib/backup/backupnames/filenames.go | 15 +++++++++++++++ lib/backup/fscommon/fscommon.go | 13 ++++--------- 5 files changed, 26 insertions(+), 16 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1fe472030..debbc0a38 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -62,6 +62,7 @@ Released at 2023-06-30 * BUGFIX: [VictoriaMetrics cluster](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html): properly return error from [/api/v1/query](https://docs.victoriametrics.com/keyConcepts.html#instant-query) and [/api/v1/query_range](https://docs.victoriametrics.com/keyConcepts.html#range-query) at `vmselect` when the `-search.maxSamplesPerQuery` or `-search.maxSamplesPerSeries` [limit](https://docs.victoriametrics.com/Cluster-VictoriaMetrics.html#resource-usage-limits) is exceeded. Previously incomplete response could be returned without the error if `vmselect` runs with `-replicationFactor` greater than 1. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4472). * BUGFIX: [storage](https://docs.victoriametrics.com/Single-server-VictoriaMetrics.html): Properly creates `parts.json` after migration from versions below `v1.90.0. It must fix errors on start-up after unclean shutdown. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4336) for details. * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix a memory leak issue associated with chart updates. See [this pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/4455). +* BUGFIX: [vmbackupmanager](https://docs.victoriametrics.com/vmbackupmanager.html): fix removing storage data dir before restoring from backup. ## [v1.91.2](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.91.2) diff --git a/lib/backup/actions/backup.go b/lib/backup/actions/backup.go index b40ec8afb..7801d1bce 100644 --- a/lib/backup/actions/backup.go +++ b/lib/backup/actions/backup.go @@ -8,8 +8,8 @@ import ( "sync/atomic" "time" + "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/backupnames" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/common" - "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fscommon" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fslocal" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fsnil" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" @@ -69,7 +69,7 @@ func (b *Backup) Run() error { origin = &fsnil.FS{} } - if err := dst.DeleteFile(fscommon.BackupCompleteFilename); err != nil { + if err := dst.DeleteFile(backupnames.BackupCompleteFilename); err != nil { return fmt.Errorf("cannot delete `backup complete` file at %s: %w", dst, err) } if err := runBackup(src, dst, origin, concurrency); err != nil { @@ -78,7 +78,7 @@ func (b *Backup) Run() error { if err := storeMetadata(src, dst); err != nil { return fmt.Errorf("cannot store backup metadata: %w", err) } - if err := dst.CreateFile(fscommon.BackupCompleteFilename, []byte("ok")); err != nil { + if err := dst.CreateFile(backupnames.BackupCompleteFilename, []byte{}); err != nil { return fmt.Errorf("cannot create `backup complete` file at %s: %w", dst, err) } @@ -102,7 +102,7 @@ func storeMetadata(src *fslocal.FS, dst common.RemoteFS) error { return fmt.Errorf("cannot marshal metadata: %w", err) } - if err := dst.CreateFile(fscommon.BackupMetadataFilename, metadata); err != nil { + if err := dst.CreateFile(backupnames.BackupMetadataFilename, metadata); err != nil { return fmt.Errorf("cannot create `backup complete` file at %s: %w", dst, err) } diff --git a/lib/backup/actions/restore.go b/lib/backup/actions/restore.go index 14a64e17b..2dd10d073 100644 --- a/lib/backup/actions/restore.go +++ b/lib/backup/actions/restore.go @@ -10,7 +10,6 @@ import ( "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/backupnames" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/common" - "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fscommon" "github.com/VictoriaMetrics/VictoriaMetrics/lib/backup/fslocal" "github.com/VictoriaMetrics/VictoriaMetrics/lib/fs" "github.com/VictoriaMetrics/VictoriaMetrics/lib/logger" @@ -56,13 +55,13 @@ func (r *Restore) Run() error { dst := r.Dst if !r.SkipBackupCompleteCheck { - ok, err := src.HasFile(fscommon.BackupCompleteFilename) + ok, err := src.HasFile(backupnames.BackupCompleteFilename) if err != nil { return err } if !ok { return fmt.Errorf("cannot find %s file in %s; this means either incomplete backup or old backup; "+ - "pass -skipBackupCompleteCheck command-line flag if you still need restoring from this backup", fscommon.BackupCompleteFilename, src) + "pass -skipBackupCompleteCheck command-line flag if you still need restoring from this backup", backupnames.BackupCompleteFilename, src) } } diff --git a/lib/backup/backupnames/filenames.go b/lib/backup/backupnames/filenames.go index 415130f30..3047bb58f 100644 --- a/lib/backup/backupnames/filenames.go +++ b/lib/backup/backupnames/filenames.go @@ -6,4 +6,19 @@ const ( // This file is created at the beginning of the restore process and is deleted at the end of the restore process. // If this file exists, then it is unsafe to read the storage data, since it can be incomplete. RestoreInProgressFilename = "restore-in-progress" + + // RestoreMarkFileName is the filename for "restore mark" file. + // This file is created in backupmanager for starting restore process. + // It is deleted after successful restore. + RestoreMarkFileName = "backup_restore.ignore" + + // ProtectMarkFileName is the filename for "protection mark" file. + // This file is created in backupmanager for protecting backup from deletion via retention policy. + ProtectMarkFileName = "backup_locked.ignore" + + // BackupCompleteFilename is a filename, which is created in the destination fs when backup is complete. + BackupCompleteFilename = "backup_complete.ignore" + + // BackupMetadataFilename is a filename, which contains metadata for the backup. + BackupMetadataFilename = "backup_metadata.ignore" ) diff --git a/lib/backup/fscommon/fscommon.go b/lib/backup/fscommon/fscommon.go index f76eb81ce..e232e6ed6 100644 --- a/lib/backup/fscommon/fscommon.go +++ b/lib/backup/fscommon/fscommon.go @@ -106,7 +106,7 @@ func appendFilesInternal(dst []string, d *os.File) ([]string, error) { } func isSpecialFile(name string) bool { - return name == "flock.lock" || name == backupnames.RestoreInProgressFilename + return name == "flock.lock" || name == backupnames.RestoreInProgressFilename || name == backupnames.RestoreMarkFileName } // RemoveEmptyDirs recursively removes empty directories under the given dir. @@ -165,8 +165,9 @@ func removeEmptyDirsInternal(d *os.File) (bool, error) { continue } if fi.Mode()&os.ModeSymlink != os.ModeSymlink { - if isSpecialFile(name) { - // Do not take into account special files + // isSpecialFile is not suitable for this function, because the root directory must be considered not empty + // i.e. function must consider the markers of the restore in progress as files that are not allowed to be removed by this function. + if name == "flock.lock" { continue } dirEntries++ @@ -232,9 +233,3 @@ func removeEmptyDirsInternal(d *os.File) (bool, error) { func IgnorePath(path string) bool { return strings.HasSuffix(path, ".ignore") } - -// BackupCompleteFilename is a filename, which is created in the destination fs when backup is complete. -const BackupCompleteFilename = "backup_complete.ignore" - -// BackupMetadataFilename is a filename, which contains metadata for the backup. -const BackupMetadataFilename = "backup_metadata.ignore"