diff --git a/weed/storage/remote_tier_integration_test.go b/weed/storage/remote_tier_integration_test.go index 59744a84c..ec08130c3 100644 --- a/weed/storage/remote_tier_integration_test.go +++ b/weed/storage/remote_tier_integration_test.go @@ -147,9 +147,9 @@ func (f *localDirBackendFile) Truncate(off int64) error { return os.Truncate(filepath.Join(f.backend.root, f.key), off) } -func (f *localDirBackendFile) Close() error { return nil } -func (f *localDirBackendFile) Name() string { return f.key } -func (f *localDirBackendFile) Sync() error { return nil } +func (f *localDirBackendFile) Close() error { return nil } +func (f *localDirBackendFile) Name() string { return f.key } +func (f *localDirBackendFile) Sync() error { return nil } func (f *localDirBackendFile) GetStat() (int64, time.Time, error) { files := f.tierInfo.GetFiles() if len(files) == 0 { @@ -174,11 +174,13 @@ func registerTestBackend(t *testing.T, b *localDirBackend) { }) } -// tierUpVolume creates a real on-disk volume, writes a few needles, then -// uploads the .dat to the fake backend and rewrites the volume in remote -// mode (mirrors the production flow in volume_grpc_tier_upload.go but -// in-process). -func tierUpVolume(t *testing.T, dir string, vid needle.VolumeId, b *localDirBackend) (collection string, key string) { +// tierUpVolumeLive creates a real on-disk volume, writes a few needles, then +// uploads the .dat to the fake backend and rewrites the volume in remote mode +// (mirrors the production flow in volume_grpc_tier_upload.go but in-process). +// It returns the still-open volume, exactly as a volume server holds it after a +// live `volume.tier.upload` — no reload. Callers that want the on-disk state a +// server sees after restart use tierUpVolume, which closes it. +func tierUpVolumeLive(t *testing.T, dir string, vid needle.VolumeId, b *localDirBackend) (v *Volume, key string) { t.Helper() v, err := NewVolume(dir, dir, "", vid, NeedleMapInMemory, &super_block.ReplicaPlacement{}, &needle.TTL{}, 0, needle.GetCurrentVersion(), 0, 0) require.NoError(t, err) @@ -208,10 +210,16 @@ func tierUpVolume(t *testing.T, dir string, vid needle.VolumeId, b *localDirBack require.NoError(t, v.LoadRemoteFile()) require.NoError(t, os.Remove(v.FileName(".dat"))) - // Close the volume cleanly. Tests below reload it from disk to mirror - // what a volume server does on restart with a tiered volume. - v.Close() + return v, uploadKey +} +// tierUpVolume runs tierUpVolumeLive then closes the volume. Tests using it +// reload from disk to mirror what a volume server does on restart with a +// tiered volume. +func tierUpVolume(t *testing.T, dir string, vid needle.VolumeId, b *localDirBackend) (collection string, key string) { + t.Helper() + v, uploadKey := tierUpVolumeLive(t, dir, vid, b) + v.Close() return v.Collection, uploadKey } @@ -259,6 +267,33 @@ func TestRemoteTier_DiskScanLoadsRemoteOnlyVolume(t *testing.T) { v.Close() } +// TestRemoteTier_LiveTierUpload_StillReportsToMaster covers a live +// `volume.tier.upload`: the .dat is removed and the volume serves from remote, +// but the same in-memory Volume keeps heartbeating with no reload. The +// phantom-.dat guard must not suppress it just because .dat is gone — +// LoadRemoteFile has flipped it into remote mode, so ToVolumeInformationMessage +// must still report it to the master. If HasRemoteFile stayed false the volume +// would vanish from the topology ("volume not found"). +func TestRemoteTier_LiveTierUpload_StillReportsToMaster(t *testing.T) { + b := newLocalDirBackend(t) + registerTestBackend(t, b) + + dir := t.TempDir() + const vid = needle.VolumeId(67) + v, _ := tierUpVolumeLive(t, dir, vid, b) + defer v.Close() + // A store-owned volume always carries its DiskLocation; NewVolume leaves it + // nil, so give it one for the IsReadOnly disk-space check inside the heartbeat. + v.location = &DiskLocation{Directory: dir, DiskType: types.HddType} + + require.True(t, v.HasRemoteFile(), "a tier-uploaded volume is in remote mode even before any reload") + require.False(t, util.FileExists(v.FileName(".dat")), "tier-up should have removed the local .dat") + + _, msg := v.ToVolumeInformationMessage() + require.NotNil(t, msg, "tier-uploaded volume must still report to master") + require.NotEmpty(t, msg.RemoteStorageName, "reported volume must carry its remote backend name") +} + // TestRemoteTier_Move_KeepsRemoteObject simulates the move-on-source-after-copy // step of a balance: Destroy(onlyEmpty=false, keepRemoteData=true). The remote // object must survive — the destination's freshly-copied .vif points at it. diff --git a/weed/storage/store_load_balancing_test.go b/weed/storage/store_load_balancing_test.go index 90fa7ff7f..c4ca73178 100644 --- a/weed/storage/store_load_balancing_test.go +++ b/weed/storage/store_load_balancing_test.go @@ -102,7 +102,7 @@ func TestLocalVolumesLen(t *testing.T) { // Mark some as remote if i < tc.remoteVolumes { - vol.hasRemoteFile = true + vol.hasRemoteFile.Store(true) vol.volumeInfo.Files = []*volume_server_pb.RemoteFile{ {BackendType: "s3", BackendId: "test", Key: "test-key"}, } @@ -250,7 +250,7 @@ func createTestVolume(vid needle.VolumeId, isRemote bool) *Volume { } if isRemote { - vol.hasRemoteFile = true + vol.hasRemoteFile.Store(true) vol.volumeInfo.Files = []*volume_server_pb.RemoteFile{ {BackendType: "s3", BackendId: "test", Key: "remote-key-" + strconv.Itoa(int(vid))}, } diff --git a/weed/storage/volume.go b/weed/storage/volume.go index f7a5423f2..e2b81fb17 100644 --- a/weed/storage/volume.go +++ b/weed/storage/volume.go @@ -32,7 +32,7 @@ type Volume struct { noWriteOrDelete bool // if readonly, either noWriteOrDelete or noWriteCanDelete noWriteCanDelete bool // if readonly, either noWriteOrDelete or noWriteCanDelete noWriteLock sync.RWMutex - hasRemoteFile bool // if the volume has a remote file + hasRemoteFile atomic.Bool // if the volume is tiered: data lives in a remote backend MemoryMapMaxSizeMb uint32 super_block.SuperBlock diff --git a/weed/storage/volume_tier.go b/weed/storage/volume_tier.go index 775db67f3..2704b9e60 100644 --- a/weed/storage/volume_tier.go +++ b/weed/storage/volume_tier.go @@ -19,13 +19,15 @@ func (v *Volume) GetVolumeInfo() *volume_server_pb.VolumeInfo { func (v *Volume) maybeLoadVolumeInfo() (found bool) { var err error - v.volumeInfo, v.hasRemoteFile, found, err = volume_info.MaybeLoadVolumeInfo(v.FileName(".vif")) + var hasRemoteFile bool + v.volumeInfo, hasRemoteFile, found, err = volume_info.MaybeLoadVolumeInfo(v.FileName(".vif")) + v.hasRemoteFile.Store(hasRemoteFile) if v.volumeInfo.Version == 0 { v.volumeInfo.Version = uint32(needle.GetCurrentVersion()) } - if v.hasRemoteFile { + if hasRemoteFile { glog.V(0).Infof("volume %d is tiered to %s as %s and read only", v.Id, v.volumeInfo.Files[0].BackendName(), v.volumeInfo.Files[0].Key) } else { @@ -55,7 +57,7 @@ func (v *Volume) maybeLoadVolumeInfo() (found bool) { } func (v *Volume) HasRemoteFile() bool { - return v.hasRemoteFile + return v.hasRemoteFile.Load() } func (v *Volume) LoadRemoteFile() error { @@ -65,11 +67,14 @@ func (v *Volume) LoadRemoteFile() error { return fmt.Errorf("backend storage %s not found", tierFile.BackendName()) } - if v.DataBackend != nil { - v.DataBackend.Close() - } - - v.DataBackend = backendStorage.NewStorageFile(tierFile.Key, v.volumeInfo) + // 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) return nil } diff --git a/weed/storage/volume_write.go b/weed/storage/volume_write.go index 0d632142f..9e139c757 100644 --- a/weed/storage/volume_write.go +++ b/weed/storage/volume_write.go @@ -274,7 +274,7 @@ func (v *Volume) doDeleteRequest(n *needle.Needle) (Size, error) { var offset uint64 var err error size := nv.Size - if !v.hasRemoteFile { + if !v.HasRemoteFile() { n.Data = nil n.UpdateAppendAtNs(v.lastAppendAtNs) offset, _, _, err = n.Append(v.DataBackend, v.Version())