From f09e8345c683a5c27cf55a6507ce15a752ccfaba Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 9 Aug 2026 12:43:31 -0700 Subject: [PATCH] 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. --- .../src/storage/volume_report_hash.rs | 9 +++++---- weed/admin/dash/volume_export.go | 2 -- weed/server/volume_server_handlers_ui.go | 20 +++++++++++++++---- weed/shell/command_ec_encode.go | 6 +++--- weed/shell/command_volume_tier_compact.go | 4 ++-- weed/shell/command_volume_tier_download.go | 2 +- weed/shell/command_volume_tier_upload.go | 2 +- weed/storage/store.go | 2 +- weed/storage/volume_info.go | 12 +++++------ weed/storage/volume_info_intern_test.go | 4 ---- weed/storage/volume_report_hash.go | 1 - weed/topology/topology_test.go | 4 ++-- weed/topology/volume_digest_test.go | 13 ++++++++++++ 13 files changed, 50 insertions(+), 31 deletions(-) diff --git a/seaweed-volume/src/storage/volume_report_hash.rs b/seaweed-volume/src/storage/volume_report_hash.rs index c661d4d7f..26b3d1347 100644 --- a/seaweed-volume/src/storage/volume_report_hash.rs +++ b/seaweed-volume/src/storage/volume_report_hash.rs @@ -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] diff --git a/weed/admin/dash/volume_export.go b/weed/admin/dash/volume_export.go index 58a63d9ef..c6de3264c 100644 --- a/weed/admin/dash/volume_export.go +++ b/weed/admin/dash/volume_export.go @@ -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 { diff --git a/weed/server/volume_server_handlers_ui.go b/weed/server/volume_server_handlers_ui.go index a8dabf507..d5a78a98a 100644 --- a/weed/server/volume_server_handlers_ui.go +++ b/weed/server/volume_server_handlers_ui.go @@ -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 diff --git a/weed/shell/command_ec_encode.go b/weed/shell/command_ec_encode.go index a22c1f1f3..f7e2e47bc 100644 --- a/weed/shell/command_ec_encode.go +++ b/weed/shell/command_ec_encode.go @@ -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 } diff --git a/weed/shell/command_volume_tier_compact.go b/weed/shell/command_volume_tier_compact.go index f26af60a6..1b31ddce1 100644 --- a/weed/shell/command_volume_tier_compact.go +++ b/weed/shell/command_volume_tier_compact.go @@ -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) { diff --git a/weed/shell/command_volume_tier_download.go b/weed/shell/command_volume_tier_download.go index d261747c3..f17199e9d 100644 --- a/weed/shell/command_volume_tier_download.go +++ b/weed/shell/command_volume_tier_download.go @@ -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 } } diff --git a/weed/shell/command_volume_tier_upload.go b/weed/shell/command_volume_tier_upload.go index eda04884c..a48607435 100644 --- a/weed/shell/command_volume_tier_upload.go +++ b/weed/shell/command_volume_tier_upload.go @@ -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) diff --git a/weed/storage/store.go b/weed/storage/store.go index 22e333f5e..6f5fa458c 100644 --- a/weed/storage/store.go +++ b/weed/storage/store.go @@ -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() diff --git a/weed/storage/volume_info.go b/weed/storage/volume_info.go index 5608d596c..40e7f173a 100644 --- a/weed/storage/volume_info.go +++ b/weed/storage/volume_info.go @@ -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, } diff --git a/weed/storage/volume_info_intern_test.go b/weed/storage/volume_info_intern_test.go index 880dc1a8b..5ac7bf52a 100644 --- a/weed/storage/volume_info_intern_test.go +++ b/weed/storage/volume_info_intern_test.go @@ -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) diff --git a/weed/storage/volume_report_hash.go b/weed/storage/volume_report_hash.go index 0b1a24543..bc2522b2d 100644 --- a/weed/storage/volume_report_hash.go +++ b/weed/storage/volume_report_hash.go @@ -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 } diff --git a/weed/topology/topology_test.go b/weed/topology/topology_test.go index 32c3ad7e4..ec3f4585e 100644 --- a/weed/topology/topology_test.go +++ b/weed/topology/topology_test.go @@ -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 diff --git a/weed/topology/volume_digest_test.go b/weed/topology/volume_digest_test.go index 70ded92c8..e40e661fd 100644 --- a/weed/topology/volume_digest_test.go +++ b/weed/topology/volume_digest_test.go @@ -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()) }) }