diff --git a/.github/workflows/mount-windows-conformance.yml b/.github/workflows/mount-windows-conformance.yml index 95b0ea4b9..6d490c5d8 100644 --- a/.github/workflows/mount-windows-conformance.yml +++ b/.github/workflows/mount-windows-conformance.yml @@ -6,6 +6,9 @@ on: paths: - 'weed/mount/**' - 'weed/command/mount*.go' + - 'weed/storage/volume_vacuum*.go' + - 'weed/storage/volume_loading.go' + - 'weed/storage/disk_location.go' - 'test/winfsp-conformance/**' - '.github/workflows/mount-windows-conformance.yml' # No base branch filter: this is the only thing that runs the Windows mount, @@ -14,6 +17,9 @@ on: paths: - 'weed/mount/**' - 'weed/command/mount*.go' + - 'weed/storage/volume_vacuum*.go' + - 'weed/storage/volume_loading.go' + - 'weed/storage/disk_location.go' - 'test/winfsp-conformance/**' - '.github/workflows/mount-windows-conformance.yml' diff --git a/.github/workflows/mount-windows.yml b/.github/workflows/mount-windows.yml index 985f8c85c..cd9db5685 100644 --- a/.github/workflows/mount-windows.yml +++ b/.github/workflows/mount-windows.yml @@ -6,6 +6,9 @@ on: paths: - 'weed/mount/**' - 'weed/command/mount*.go' + - 'weed/storage/volume_vacuum*.go' + - 'weed/storage/volume_loading.go' + - 'weed/storage/disk_location.go' - 'test/winfsp/**' - '.github/workflows/mount-windows.yml' # No base branch filter: this is the only thing that runs the Windows mount, @@ -14,6 +17,9 @@ on: paths: - 'weed/mount/**' - 'weed/command/mount*.go' + - 'weed/storage/volume_vacuum*.go' + - 'weed/storage/volume_loading.go' + - 'weed/storage/disk_location.go' - 'test/winfsp/**' - '.github/workflows/mount-windows.yml' diff --git a/seaweed-volume/src/storage/volume.rs b/seaweed-volume/src/storage/volume.rs index 3c00f1866..01896350e 100644 --- a/seaweed-volume/src/storage/volume.rs +++ b/seaweed-volume/src/storage/volume.rs @@ -3627,7 +3627,7 @@ fn get_append_at_ns(last: u64) -> u64 { /// durable, propagating a sync failure so the commit path can abort rather than /// proceed with an undurable rename or marker. A path with no openable parent is /// tolerated; directory fsync is unsupported on Windows, so it is a no-op there -/// (matching the Go fsyncDir helper, which ignores that error). +/// (matching the Go fsyncDir helper, which skips it too). pub(crate) fn fsync_dir(path: &str) -> io::Result<()> { #[cfg(windows)] { diff --git a/weed/storage/volume_vacuum.go b/weed/storage/volume_vacuum.go index 94ae586a9..a1e2835ed 100644 --- a/weed/storage/volume_vacuum.go +++ b/weed/storage/volume_vacuum.go @@ -187,36 +187,36 @@ func (v *Volume) CommitCompact() error { v.DataBackend = nil stats.VolumeServerVolumeGauge.WithLabelValues(v.Collection, "volume").Dec() - var e error - if e = v.makeupDiff(v.FileName(".cpd"), v.FileName(".cpx"), v.FileName(".dat"), v.FileName(".idx")); e != nil { - glog.V(0).Infof("makeupDiff in CommitCompact volume %d failed %v", v.Id, e) - e = os.Remove(v.FileName(".cpd")) - if e != nil { - return e + if compactErr := v.makeupDiff(v.FileName(".cpd"), v.FileName(".cpx"), v.FileName(".dat"), v.FileName(".idx")); compactErr != nil { + glog.V(0).Infof("makeupDiff in CommitCompact volume %d failed %v", v.Id, compactErr) + if e := os.Remove(v.FileName(".cpd")); e != nil && !os.IsNotExist(e) { + glog.V(0).Infof("remove %s: %v", v.FileName(".cpd"), e) } - e = os.Remove(v.FileName(".cpx")) - if e != nil { - return e - } - } else { - // makeupDiff has fsynced the .cpd/.cpx contents. Persist a durable .cpc - // commit marker BEFORE renaming so the two renames are atomic across a - // crash: a marker on disk means the swap is decided and reconcile rolls - // forward; no marker means roll back. Without it, a crash between the - // two renames leaves a stale .idx that a later vacuum compacts to empty. - if e = v.writeCompactCommitMarker(); e != nil { - return e - } - if e = v.applyCompactSwap(); e != nil { - return e + if e := os.Remove(v.FileName(".cpx")); e != nil && !os.IsNotExist(e) { + glog.V(0).Infof("remove %s: %v", v.FileName(".cpx"), e) } + // Report the abandoned compaction rather than a cleanup failure that + // reconcile rolls back anyway, and never fall through to the reload. + return compactErr + } + + // makeupDiff has fsynced the .cpd/.cpx contents. Persist a durable .cpc + // commit marker BEFORE renaming so the two renames are atomic across a + // crash: a marker on disk means the swap is decided and reconcile rolls + // forward; no marker means roll back. Without it, a crash between the + // two renames leaves a stale .idx that a later vacuum compacts to empty. + if e := v.writeCompactCommitMarker(); e != nil { + return e + } + if e := v.applyCompactSwap(); e != nil { + return e } //glog.V(3).Infof("Pretending to be vacuuming...") //time.Sleep(20 * time.Second) glog.V(3).Infof("Loading volume %d commit file...", v.Id) - if e = v.load(true, false, v.needleMapKind, 0, v.Version()); e != nil { + if e := v.load(true, false, v.needleMapKind, 0, v.Version()); e != nil { return e } glog.V(3).Infof("Finish committing volume %d", v.Id) @@ -377,9 +377,13 @@ func (v *Volume) cleanupCompact() error { } // fsyncDir fsyncs a directory so a rename/create/unlink inside it is durable. -// A failure to open the directory for sync is non-fatal on platforms that do -// not support it. +// Windows has no directory fsync, so the .cpc protocol leans on NTFS metadata +// ordering there. An unopenable directory is tolerated, unlike util.FsyncDir, +// because the caller has already closed the needle map. func fsyncDir(dir string) error { + if runtime.GOOS == "windows" { + return nil + } d, err := os.Open(dir) if err != nil { return nil