diff --git a/weed/server/volume_grpc_tier_download.go b/weed/server/volume_grpc_tier_download.go index af07f80d6..81ef87505 100644 --- a/weed/server/volume_grpc_tier_download.go +++ b/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 } diff --git a/weed/server/volume_grpc_tier_download_test.go b/weed/server/volume_grpc_tier_download_test.go index 43401ca26..f346b5257 100644 --- a/weed/server/volume_grpc_tier_download_test.go +++ b/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). diff --git a/weed/storage/volume.go b/weed/storage/volume.go index e2b81fb17..25c8bd038 100644 --- a/weed/storage/volume.go +++ b/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() { diff --git a/weed/storage/volume_tier.go b/weed/storage/volume_tier.go index 2704b9e60..21c442ee9 100644 --- a/weed/storage/volume_tier.go +++ b/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 }