From 7fb36025b3ad105ffa793ed78b4048893c21e133 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 31 Jul 2026 00:52:04 -0700 Subject: [PATCH] wdclient: read the vid map cache link before the live map (#10505) resetVidMap trims the cache chain by storing nil into a node's cache pointer once it ages past vidMapCacheSize. A lookup that missed in its own map and only then loaded that pointer could find the link already severed, reporting "not found" for a volume that stayed resolvable the whole time. Load the link first, while it is still guaranteed live. A published vidMap's cache pointer only ever goes from its ancestor to nil, so reading it earlier can never yield staler history. --- weed/wdclient/vid_map.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/weed/wdclient/vid_map.go b/weed/wdclient/vid_map.go index 648cb614a..1cb917abe 100644 --- a/weed/wdclient/vid_map.go +++ b/weed/wdclient/vid_map.go @@ -129,6 +129,11 @@ func (vc *vidMap) GetVidLocations(vid string) (locations []Location, err error) func (vc *vidMap) GetLocations(vid uint32) (locations []Location, found bool) { // glog.V(4).Infof("~ lookup volume id %d: %+v ec:%+v", vid, vc.vid2Locations, vc.ecVid2Locations) + // Read the cache link before the live map: resetVidMap trims the chain, so a + // link loaded after the local miss may already be severed, turning a lookup + // that could have been served by the cache into a spurious miss. + cachedMap := vc.cache.Load() + locations, found = vc.getLocations(vid) if found { // If volume is explicitly tracked (found=true), return its locations even if empty. @@ -143,7 +148,7 @@ func (vc *vidMap) GetLocations(vid uint32) (locations []Location, found bool) { } // Volume not found in current map - check cache for unknown volumes - if cachedMap := vc.cache.Load(); cachedMap != nil { + if cachedMap != nil { return cachedMap.GetLocations(vid) } @@ -185,13 +190,16 @@ func (vc *vidMap) hasVolumeServer(addr pb.ServerAddress) bool { if key == "" { return false } + // Same ordering requirement as GetLocations: grab the cache link before the + // local lookup so a concurrent reset cannot sever it underneath us. + cachedMap := vc.cache.Load() vc.RLock() count := vc.serverRefCount[key] vc.RUnlock() if count > 0 { return true } - if cachedMap := vc.cache.Load(); cachedMap != nil { + if cachedMap != nil { return cachedMap.hasVolumeServer(addr) } return false