storage: stop keeping the remote storage key on the master (#10672)

A master decides nothing from it. Every caller that read it was asking whether
a volume is remote, which the backend name answers, and the value itself is
reported on demand by the server holding the volume, through the volume info in
ReadVolumeFileStatus.

It is also the one string here that cannot be shared: unique per volume, so
unlike the collection and backend names it carries its own characters for every
volume a master tracks.

VolumeInfo goes from 136 bytes to 120. 800k volumes registered from a heartbeat
that has been over the wire go from 214 to 163 B/volume when tiered.

The volume server's own status page keeps showing the key, now read from the
volume it holds rather than relayed through a master, which is also where the
other volume server implementation reads it.

The heartbeat digest drops it on the same grounds: a change to something the
master does not hold cannot make its copy stale. Both implementations and their
shared vectors move together, and the field-coverage test now names what is
deliberately not retained rather than being loosened.
This commit is contained in:
Chris Lu
2026-08-09 12:43:31 -07:00
committed by GitHub
parent 0f7a64c596
commit f09e8345c6
13 changed files with 50 additions and 31 deletions
@@ -35,8 +35,9 @@ pub fn report_hash(m: &master_pb::VolumeInformationMessage) -> u64 {
h = fold(h, xxh64(&(m.modified_at_second as u64).to_le_bytes(), 0));
h = fold(h, xxh64(m.collection.as_bytes(), 0));
h = fold(h, xxh64(m.disk_type.as_bytes(), 0));
// Not the remote storage key: the master does not keep it, so a change to
// it alters nothing its copy holds.
h = fold(h, xxh64(m.remote_storage_name.as_bytes(), 0));
h = fold(h, xxh64(m.remote_storage_key.as_bytes(), 0));
h
}
@@ -67,11 +68,11 @@ mod tests {
#[test]
fn report_hash_vectors() {
let empty = master_pb::VolumeInformationMessage::default();
assert_eq!(report_hash(&empty), 17122085700329870549);
assert_eq!(report_hash(&empty), 10988706248825469653);
let mut one = master_pb::VolumeInformationMessage::default();
one.id = 1;
assert_eq!(report_hash(&one), 12867601919960834066);
assert_eq!(report_hash(&one), 2035849960016744285);
let full = master_pb::VolumeInformationMessage {
id: 42,
@@ -91,7 +92,7 @@ mod tests {
disk_type: "ssd".to_string(),
disk_id: 2,
};
assert_eq!(report_hash(&full), 12500327696413250175);
assert_eq!(report_hash(&full), 2748844479819636032);
}
#[test]
-2
View File
@@ -93,7 +93,6 @@ type ExportVolume struct {
DiskType string `json:"disk_type"`
DiskId uint32 `json:"disk_id"`
RemoteStorageName string `json:"remote_storage_name,omitempty"`
RemoteStorageKey string `json:"remote_storage_key,omitempty"`
}
type ExportEcShard struct {
@@ -251,7 +250,6 @@ func buildExportVolume(m *master_pb.VolumeInformationMessage, volumeSizeLimit ui
DiskType: m.DiskType,
DiskId: m.DiskId,
RemoteStorageName: m.RemoteStorageName,
RemoteStorageKey: m.RemoteStorageKey,
}
// Decode replica placement and TTL the way volume.list does.
if vi, err := storage.NewVolumeInfo(m); err == nil {
+16 -4
View File
@@ -15,6 +15,13 @@ import (
"github.com/seaweedfs/seaweedfs/weed/storage"
)
// remoteVolumeRow carries the remote key the store no longer keeps per volume,
// read from the volume this server holds rather than relayed by a master.
type remoteVolumeRow struct {
*storage.VolumeInfo
RemoteStorageKey string
}
func (vs *VolumeServer) uiStatusHandler(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Server", "SeaweedFS Volume "+version.VERSION)
infos := make(map[string]interface{})
@@ -28,13 +35,18 @@ func (vs *VolumeServer) uiStatusHandler(w http.ResponseWriter, r *http.Request)
}
}
volumeInfos := vs.store.VolumeInfos()
var normalVolumeInfos, remoteVolumeInfos []*storage.VolumeInfo
var normalVolumeInfos []*storage.VolumeInfo
var remoteVolumeInfos []remoteVolumeRow
for _, vinfo := range volumeInfos {
if vinfo.IsRemote() {
remoteVolumeInfos = append(remoteVolumeInfos, vinfo)
} else {
if !vinfo.IsRemote() {
normalVolumeInfos = append(normalVolumeInfos, vinfo)
continue
}
row := remoteVolumeRow{VolumeInfo: vinfo}
if v := vs.store.GetVolume(vinfo.Id); v != nil {
_, row.RemoteStorageKey = v.RemoteStorageNameKey()
}
remoteVolumeInfos = append(remoteVolumeInfos, row)
}
args := struct {
Version string
+3 -3
View File
@@ -964,11 +964,11 @@ func selectVolumeIdsFromTopology(topologyInfo *master_pb.TopologyInfo, volumeSiz
totalVolumes++
// ignore remote volumes
if v.RemoteStorageName != "" && v.RemoteStorageKey != "" {
if v.RemoteStorageName != "" {
remoteVolumes++
if verbose {
fmt.Printf("skip volume %d on %s: remote volume (storage: %s, key: %s)\n",
v.Id, dn.Id, v.RemoteStorageName, v.RemoteStorageKey)
fmt.Printf("skip volume %d on %s: remote volume (storage: %s)\n",
v.Id, dn.Id, v.RemoteStorageName)
}
continue
}
+2 -2
View File
@@ -140,7 +140,7 @@ func findRemoteVolumeInTopology(topoInfo *master_pb.TopologyInfo, vid needle.Vol
}
for _, diskInfo := range dn.DiskInfos {
for _, v := range diskInfo.VolumeInfos {
if needle.VolumeId(v.Id) == vid && v.RemoteStorageName != "" && v.RemoteStorageKey != "" {
if needle.VolumeId(v.Id) == vid && v.RemoteStorageName != "" {
if !matchesCollection(v.Collection) {
continue
}
@@ -171,7 +171,7 @@ func collectRemoteVolumesWithInfo(topoInfo *master_pb.TopologyInfo, collectionPa
eachDataNode(topoInfo, func(dc DataCenterId, rack RackId, dn *master_pb.DataNodeInfo) {
for _, diskInfo := range dn.DiskInfos {
for _, v := range diskInfo.VolumeInfos {
if v.RemoteStorageName == "" || v.RemoteStorageKey == "" {
if v.RemoteStorageName == "" {
continue
}
if !collectionRegex.MatchString(v.Collection) {
+1 -1
View File
@@ -104,7 +104,7 @@ func collectRemoteVolumes(topoInfo *master_pb.TopologyInfo, collectionPattern st
eachDataNode(topoInfo, func(dc DataCenterId, rack RackId, dn *master_pb.DataNodeInfo) {
for _, diskInfo := range dn.DiskInfos {
for _, v := range diskInfo.VolumeInfos {
if collectionRegex.MatchString(v.Collection) && v.RemoteStorageKey != "" && v.RemoteStorageName != "" {
if collectionRegex.MatchString(v.Collection) && v.RemoteStorageName != "" {
vidMap[v.Id] = true
}
}
+1 -1
View File
@@ -178,7 +178,7 @@ func collectVolumeTierUploadLocations(topoInfo *master_pb.TopologyInfo, vid need
GrpcPort: int(dn.GrpcPort),
DataCenter: string(dc),
}
if vi.RemoteStorageKey != "" {
if vi.RemoteStorageName != "" {
tiered = append(tiered, loc)
} else {
local = append(local, loc)
+1 -1
View File
@@ -393,7 +393,7 @@ func collectStatForOneVolume(vid needle.VolumeId, v *Volume) (s *VolumeInfo) {
DiskType: v.DiskType().String(),
DiskId: v.diskId,
}
s.RemoteStorageName, s.RemoteStorageKey = v.RemoteStorageNameKey()
s.RemoteStorageName, _ = v.RemoteStorageNameKey()
v.dataFileAccessLock.RLock()
defer v.dataFileAccessLock.RUnlock()
+6 -6
View File
@@ -13,10 +13,12 @@ import (
// Held for every volume replica, so the fields are grouped by size rather than
// by meaning: interleaved, each one-byte field rounds up to a whole word.
type VolumeInfo struct {
Collection string
DiskType string
Collection string
DiskType string
// The backend a remote volume lives in. Not the key within it: a master
// decides nothing from the key and would hold one per volume, unique and so
// unshareable, while the server holding the volume reports it on demand.
RemoteStorageName string
RemoteStorageKey string
ReplicaPlacement *super_block.ReplicaPlacement
Ttl *needle.TTL
@@ -48,7 +50,6 @@ func NewVolumeInfo(m *master_pb.VolumeInformationMessage) (vi VolumeInfo, err er
CompactRevision: m.CompactRevision,
ModifiedAtSecond: m.ModifiedAtSecond,
RemoteStorageName: internVolumeString(m.RemoteStorageName),
RemoteStorageKey: m.RemoteStorageKey,
DiskType: internVolumeString(m.DiskType),
DiskId: m.DiskId,
}
@@ -125,7 +126,7 @@ func (vi VolumeInfo) String() string {
s := fmt.Sprintf("Id:%d, Size:%d, ReplicaPlacement:%s, Collection:%s, Version:%v, Ttl:%s, FileCount:%d, DeleteCount:%d, DeletedByteCount:%d, ReadOnly:%v, ModifiedAtSecond:%d",
vi.Id, vi.Size, vi.ReplicaPlacement, vi.Collection, vi.Version, vi.Ttl.String(), vi.FileCount, vi.DeleteCount, vi.DeletedByteCount, vi.ReadOnly, vi.ModifiedAtSecond)
if vi.IsRemote() {
s += fmt.Sprintf(", RemoteStorageName:%s, RemoteStorageKey:%s", vi.RemoteStorageName, vi.RemoteStorageKey)
s += fmt.Sprintf(", RemoteStorageName:%s", vi.RemoteStorageName)
}
return s
}
@@ -145,7 +146,6 @@ func (vi VolumeInfo) ToVolumeInformationMessage() *master_pb.VolumeInformationMe
CompactRevision: vi.CompactRevision,
ModifiedAtSecond: vi.ModifiedAtSecond,
RemoteStorageName: vi.RemoteStorageName,
RemoteStorageKey: vi.RemoteStorageKey,
DiskType: vi.DiskType,
DiskId: vi.DiskId,
}
-4
View File
@@ -27,7 +27,6 @@ func TestRepeatedVolumeStringsAreShared(t *testing.T) {
Collection: string([]byte("somecollection")),
DiskType: string([]byte("ssd")),
RemoteStorageName: string([]byte("s3cold")),
RemoteStorageKey: string([]byte("seaweed/somecollection/1.dat")),
}
}
@@ -48,9 +47,6 @@ func TestRepeatedVolumeStringsAreShared(t *testing.T) {
{"Collection", first.Collection, second.Collection, true},
{"DiskType", first.DiskType, second.DiskType, true},
{"RemoteStorageName", first.RemoteStorageName, second.RemoteStorageName, true},
// Unique per volume: interning it would fill the table rather than
// share anything.
{"RemoteStorageKey", first.RemoteStorageKey, second.RemoteStorageKey, false},
} {
if tc.a != tc.b {
t.Fatalf("%s: values differ, %q vs %q", tc.name, tc.a, tc.b)
-1
View File
@@ -37,7 +37,6 @@ func (vi VolumeInfo) ReportHash() uint64 {
h = foldReportHash(h, xxhash.Sum64String(vi.Collection))
h = foldReportHash(h, xxhash.Sum64String(vi.DiskType))
h = foldReportHash(h, xxhash.Sum64String(vi.RemoteStorageName))
h = foldReportHash(h, xxhash.Sum64String(vi.RemoteStorageKey))
return h
}
+2 -2
View File
@@ -320,7 +320,7 @@ func TestVolumeReadOnlyAndRemoteStatusChange(t *testing.T) {
// Simultaneously change to read-only AND remote
v.ReadOnly = true
v.RemoteStorageName = "s3"
v.RemoteStorageKey = "key1"
v.RemoteStorageName = "s3.default"
dn.UpdateVolumes([]storage.VolumeInfo{v})
// Check counts after both changes
@@ -340,7 +340,7 @@ func TestVolumeReadOnlyAndRemoteStatusChange(t *testing.T) {
// Change back to local AND read-only simultaneously
v.ReadOnly = true
v.RemoteStorageName = ""
v.RemoteStorageKey = ""
v.RemoteStorageName = ""
dn.UpdateVolumes([]storage.VolumeInfo{v})
// Check final counts
+13
View File
@@ -75,6 +75,12 @@ func TestVolumeDigestIsIndependentOfReportOrder(t *testing.T) {
// skips is a change the master would never be told about. Enumerated from the
// message rather than listed here, so a field added later cannot quietly fall
// outside the digest while this still passes.
// Fields a server reports that the master deliberately does not keep, so a
// change to one alters nothing it holds and the digest is right not to move.
var notRetainedByTheMaster = map[string]string{
"remote_storage_key": "unique per volume, and the master decides nothing from it",
}
func TestVolumeDigestTracksEveryReportedField(t *testing.T) {
base := digestTestVolume(1)
baseInfo, err := storage.NewVolumeInfo(base)
@@ -86,6 +92,7 @@ func TestVolumeDigestTracksEveryReportedField(t *testing.T) {
for i := 0; i < fields.Len(); i++ {
fd := fields.Get(i)
t.Run(string(fd.Name()), func(t *testing.T) {
why, skipped := notRetainedByTheMaster[string(fd.Name())]
candidates := distinctValuesFor(t, fd, base.ProtoReflect().Get(fd))
for _, candidate := range candidates {
changed := proto.Clone(base).(*master_pb.VolumeInformationMessage)
@@ -95,9 +102,15 @@ func TestVolumeDigestTracksEveryReportedField(t *testing.T) {
t.Fatal(err)
}
if baseInfo.ReportHash() != changedInfo.ReportHash() {
if skipped {
t.Errorf("%s moves the digest, but is listed as not retained by the master (%s)", fd.Name(), why)
}
return
}
}
if skipped {
return
}
t.Errorf("no change to %s moves the digest, so the master would never be told about one", fd.Name())
})
}