volume: clear remote flag when tiering a volume back to local (#10262)

VolumeTierMoveDatFromRemote downloads the .dat, trims the .vif, and swaps
the data backend to the local file, but left hasRemoteFile set. The
volume.tier.download command masks this by unmounting and remounting
right after, which reloads the flag from the trimmed .vif — but in the
window before the remount the in-memory flag is wrong: doDeleteRequest
would skip appending the tombstone to the freshly local .dat, and the
phantom-.dat guard stays disabled.

Give SwapDataBackend a hasRemoteFile argument so the backend swap and the
flag move together under one lock, and route both tier directions through
it: the tier-down download passes false, LoadRemoteFile passes true. The
flag can no longer disagree with the live backend.

weed/server/volume_grpc_tier_download.go

@@ -85,10 +85,10 @@ func (vs *VolumeServer) VolumeTierMoveDatFromRemote(req *volume_server_pb.Volume
 	}
 
 	// Trim the remote file reference and persist the .vif (util.WriteFile fsyncs it)
-	// BEFORE deleting the remote object. After this point hasRemoteFile is false, so a
-	// crash before DeleteFile merely leaks the remote object while the volume reloads
-	// its local .dat. The volume must NEVER be left with a .vif referencing the remote
-	// object while that object is deleted.
+	// BEFORE deleting the remote object. After this the .vif no longer references the
+	// remote object, so a crash before DeleteFile merely leaks the remote object while
+	// the volume reloads its local .dat as non-remote. The volume must NEVER be left
+	// with a .vif referencing the remote object while that object is deleted.
 	v.GetVolumeInfo().Files = v.GetVolumeInfo().Files[1:]
 	if err := v.SaveVolumeInfo(); err != nil {
 		return fmt.Errorf("volume %d failed to save remote file info: %v", v.Id, err)
@@ -127,8 +127,9 @@ func swapToLocalDatBackend(v *storage.Volume, datFileName string) error {
 		return err
 	}
 	// Swap under the volume's data lock so concurrent reads never see a closed
-	// or half-swapped backend.
-	v.SwapDataBackend(backend.NewDiskFile(dataFile))
+	// or half-swapped backend, clearing hasRemoteFile in the same step since the
+	// volume now serves from the local .dat.
+	v.SwapDataBackend(backend.NewDiskFile(dataFile), false)
 	return nil
 }
 

weed/server/volume_grpc_tier_download_test.go

@@ -121,8 +121,8 @@ func (f *tierTestBackendFile) ReadAt(p []byte, off int64) (int, error) {
 func (f *tierTestBackendFile) WriteAt(p []byte, off int64) (int, error) { panic("not implemented") }
 func (f *tierTestBackendFile) Truncate(off int64) error                 { panic("not implemented") }
 func (f *tierTestBackendFile) Close() error                             { return nil }
-func (f *tierTestBackendFile) Name() string                            { return f.key }
-func (f *tierTestBackendFile) Sync() error                             { return nil }
+func (f *tierTestBackendFile) Name() string                             { return f.key }
+func (f *tierTestBackendFile) Sync() error                              { return nil }
 func (f *tierTestBackendFile) GetStat() (int64, time.Time, error) {
 	files := f.tierInfo.GetFiles()
 	if len(files) == 0 {
@@ -136,7 +136,9 @@ type fakeTierStream struct {
 	grpc.ServerStream
 }
 
-func (s *fakeTierStream) Send(*volume_server_pb.VolumeTierMoveDatFromRemoteResponse) error { return nil }
+func (s *fakeTierStream) Send(*volume_server_pb.VolumeTierMoveDatFromRemoteResponse) error {
+	return nil
+}
 
 func newTierTestStore(t *testing.T, dir string) *storage.Store {
 	t.Helper()
@@ -276,6 +278,12 @@ func TestTierMoveDatFromRemote_KeepRemote_LeavesReplicaLocal(t *testing.T) {
 	if _, ok := v.DataBackend.(*backend.DiskFile); !ok {
 		t.Fatalf("after keep-remote download the data backend must be local DiskFile, got %T", v.DataBackend)
 	}
+	// The live volume must also report non-remote immediately, without waiting for
+	// a remount: a delete arriving before the remount would otherwise skip the .dat
+	// tombstone, and the phantom-.dat guard would stay disabled.
+	if v.HasRemoteFile() {
+		t.Fatal("after download the live volume must no longer be remote-backed")
+	}
 
 	// Remount and confirm the volume is no longer remote-backed and reads come
 	// from the local .dat (matching the bytes uploaded before tiering).

weed/storage/volume.go

@@ -312,17 +312,22 @@ func (v *Volume) Close() {
 	v.doClose()
 }
 
-// SwapDataBackend atomically replaces the data backend (e.g. swapping a
-// remote-tier backend for a freshly downloaded local .dat), closing the old
-// one. Held under dataFileAccessLock so a concurrent read/write never observes
-// a half-swapped or closed backend.
-func (v *Volume) SwapDataBackend(newBackend backend.BackendStorageFile) {
+// SwapDataBackend atomically replaces the data backend and updates the
+// remote-tier flag under dataFileAccessLock, closing the old backend. Both tier
+// directions go through here so hasRemoteFile always matches the live backend:
+// tier-down passes hasRemoteFile=false (now serving a local .dat), tier-up
+// passes true. Keeping the swap and the flag under one lock means the heartbeat
+// never observes a half-swapped backend or a flag that disagrees with it — a
+// stale-false flag would make doDeleteRequest skip the new .dat's tombstones and
+// disable the phantom-.dat guard.
+func (v *Volume) SwapDataBackend(newBackend backend.BackendStorageFile, hasRemoteFile bool) {
 	v.dataFileAccessLock.Lock()
 	defer v.dataFileAccessLock.Unlock()
 	if v.DataBackend != nil {
 		v.DataBackend.Close()
 	}
 	v.DataBackend = newBackend
+	v.hasRemoteFile.Store(hasRemoteFile)
 }
 
 func (v *Volume) doClose() {

weed/storage/volume_tier.go

@@ -68,13 +68,12 @@ func (v *Volume) LoadRemoteFile() error {
 	}
 
 	// Swap under dataFileAccessLock (via SwapDataBackend) so the heartbeat's
-	// concurrent DataBackend read never races this reassignment. Then mark the
-	// volume tiered so a later heartbeat does not treat the just-removed local
-	// .dat as a phantom volume and stop reporting it to the master. On disk-scan
-	// load this is already true; here it flips a volume that was tier-uploaded
-	// in-process without a reload.
-	v.SwapDataBackend(backendStorage.NewStorageFile(tierFile.Key, v.volumeInfo))
-	v.hasRemoteFile.Store(true)
+	// concurrent DataBackend read never races this reassignment, and mark the
+	// volume tiered in the same locked step so a later heartbeat does not treat
+	// the just-removed local .dat as a phantom volume and stop reporting it to
+	// the master. On disk-scan load this is already true; here it flips a volume
+	// that was tier-uploaded in-process without a reload.
+	v.SwapDataBackend(backendStorage.NewStorageFile(tierFile.Key, v.volumeInfo), true)
 	return nil
 }