volume: keep tier-uploaded volume reporting to master after volume.tier.upload (#10259)

volume: keep tier-uploaded volume reporting to master

A live volume.tier.upload removes the local .dat and swaps the data
backend to remote, but v.hasRemoteFile was only ever set when a volume
is loaded from disk, so the running volume kept it false. The phantom
.dat guard then saw fileCount>0, !HasRemoteFile, and a missing .dat, and
stopped reporting the volume to the master. The volume vanished from the
topology even though the upload succeeded and the data was in cloud
storage.

Set hasRemoteFile in LoadRemoteFile, the single point where a volume's
backend becomes remote, so it is true both on disk-scan load and after
an in-process tier upload. Route the backend reassignment through
SwapDataBackend so it happens under dataFileAccessLock, closing the old
backend and never racing the heartbeat's concurrent DataBackend read.
Make the field atomic since the heartbeat now reads it concurrently with
the tier-upload handler that writes it.
This commit is contained in:
Chris Lu
2026-07-07 23:00:22 -07:00
committed by GitHub
parent bcc528a517
commit 2d2fdeac3d
5 changed files with 63 additions and 23 deletions
+46 -11
View File
@@ -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.
+2 -2
View File
@@ -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))},
}
+1 -1
View File
@@ -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
+13 -8
View File
@@ -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
}
+1 -1
View File
@@ -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())