lib/fs: do not panic at windows at dir deletion (#4132)

Windows doesn't allow to remove dir with opened files. Usually it's a case for snapshots, hard cannot be removed if file is openned.
With this change, dir will be renamed and properly deleted at the next process start.
It's recommended to restart vmstorage/vmsingle for snapshots deletion completion periodically.
https://github.com/VictoriaMetrics/VictoriaMetrics/issues/70

docs/CHANGELOG.md

@@ -42,6 +42,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 
 * BUGFIX: reduce the probability of sudden increase in the number of small parts on systems with small number of CPU cores.
 * BUGFIX: [vmctl](https://docs.victoriametrics.com/vmctl.html): fix performance issue when migrating data from VictoriaMetrics according to [these docs](https://docs.victoriametrics.com/vmctl.html#migrating-data-from-victoriametrics). Add the ability to speed up the data migration via `--vm-native-disable-retries` command-line flag. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4092).
+* BUGFIX: do not panic at windows during snapshot deletion. Note, at windows process restart required for complete snapshot delete. See [this comment](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/70#issuecomment-1491529183) for details.
 * BUGFIX: [MetricsQL](https://docs.victoriametrics.com/MetricsQL.html): fix a panic when the duration in the query contains uppercase `M` suffix. Such a suffix isn't allowed to use in durations, since it clashes with `a million` suffix, e.g. it isn't clear whether `rate(metric[5M])` means rate over 5 minutes, 5 months or 5 million seconds. See [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3589) and [this](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4120) issues.
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): retry failed read request on the closed connection one more time. This improves rules execution reliability when connection between vmalert and datasource closes unexpectedly.
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): fix the display of the tenant selector. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/4160).

lib/fs/fs.go

@@ -219,17 +219,7 @@ func IsEmptyDir(path string) bool {
 // If the process crashes after the step 1, then the directory must be removed
 // on the next process start by calling MustRemoveTemporaryDirs on the parent directory.
 func MustRemoveDirAtomic(dir string) {
-	if !IsPathExist(dir) {
-		return
-	}
-	n := atomic.AddUint64(&atomicDirRemoveCounter, 1)
-	tmpDir := fmt.Sprintf("%s.must-remove.%d", dir, n)
-	if err := os.Rename(dir, tmpDir); err != nil {
-		logger.Panicf("FATAL: cannot move %s to %s: %s", dir, tmpDir, err)
-	}
-	MustRemoveAll(tmpDir)
-	parentDir := filepath.Dir(dir)
-	MustSyncPath(parentDir)
+	mustRemoveDirAtomic(dir)
 }
 
 var atomicDirRemoveCounter = uint64(time.Now().UnixNano())

lib/fs/fs_nix.go

@@ -4,9 +4,29 @@
 package fs
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"sync/atomic"
+
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"golang.org/x/sys/unix"
 )
 
 func freeSpace(stat unix.Statfs_t) uint64 {
 	return uint64(stat.Bavail) * uint64(stat.Bsize)
 }
+
+func mustRemoveDirAtomic(dir string) {
+	if !IsPathExist(dir) {
+		return
+	}
+	n := atomic.AddUint64(&atomicDirRemoveCounter, 1)
+	tmpDir := fmt.Sprintf("%s.must-remove.%d", dir, n)
+	if err := os.Rename(dir, tmpDir); err != nil {
+		logger.Panicf("FATAL: cannot move %s to %s: %s", dir, tmpDir, err)
+	}
+	MustRemoveAll(tmpDir)
+	parentDir := filepath.Dir(dir)
+	MustSyncPath(parentDir)
+}

lib/fs/fs_openbsd.go

@@ -1,9 +1,29 @@
 package fs
 
 import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"sync/atomic"
+
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"golang.org/x/sys/unix"
 )
 
 func freeSpace(stat unix.Statfs_t) uint64 {
 	return uint64(stat.F_bavail) * uint64(stat.F_bsize)
 }
+
+func mustRemoveDirAtomic(dir string) {
+	if !IsPathExist(dir) {
+		return
+	}
+	n := atomic.AddUint64(&atomicDirRemoveCounter, 1)
+	tmpDir := fmt.Sprintf("%s.must-remove.%d", dir, n)
+	if err := os.Rename(dir, tmpDir); err != nil {
+		logger.Panicf("FATAL: cannot move %s to %s: %s", dir, tmpDir, err)
+	}
+	MustRemoveAll(tmpDir)
+	parentDir := filepath.Dir(dir)
+	MustSyncPath(parentDir)
+}

lib/fs/fs_solaris.go

@@ -3,11 +3,27 @@ package fs
 import (
 	"fmt"
 	"os"
+	"path/filepath"
+	"sync/atomic"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
 	"golang.org/x/sys/unix"
 )
 
+func mustRemoveDirAtomic(dir string) {
+	if !IsPathExist(dir) {
+		return
+	}
+	n := atomic.AddUint64(&atomicDirRemoveCounter, 1)
+	tmpDir := fmt.Sprintf("%s.must-remove.%d", dir, n)
+	if err := os.Rename(dir, tmpDir); err != nil {
+		logger.Panicf("FATAL: cannot move %s to %s: %s", dir, tmpDir, err)
+	}
+	MustRemoveAll(tmpDir)
+	parentDir := filepath.Dir(dir)
+	MustSyncPath(parentDir)
+}
+
 func mmap(fd int, length int) (data []byte, err error) {
 	return unix.Mmap(fd, 0, length, unix.PROT_READ, unix.MAP_SHARED)
 

lib/fs/fs_windows.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"reflect"
 	"sync"
+	"sync/atomic"
 	"unsafe"
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/logger"
@@ -23,6 +24,21 @@ var (
 func mustSyncPath(path string) {
 }
 
+func mustRemoveDirAtomic(dir string) {
+	if !IsPathExist(dir) {
+		return
+	}
+	n := atomic.AddUint64(&atomicDirRemoveCounter, 1)
+	tmpDir := fmt.Sprintf("%s.must-remove.%d", dir, n)
+	if err := os.Rename(dir, tmpDir); err != nil {
+		logger.Panicf("FATAL: cannot move %s to %s: %s", dir, tmpDir, err)
+	}
+	err := os.RemoveAll(tmpDir)
+	if err != nil {
+		logger.Warnf("cannot remove dir: %q: %s, restart VictoriaMetrics process to complete file deletion", tmpDir, err)
+	}
+}
+
 const (
 	lockfileExclusiveLock = 2
 	fileFlagNormal        = 0x00000080