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

@@ -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)

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)
 	}
 

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)
 		}
 	}
 

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"
 )

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) {
+			// isSpecialFile is not suitable for this function, because the root directory must be considered not empty
-				// Do not take into account special files
+			// 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"