From 058569c77b82bf4d30454ddd74baba45117a5498 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 7 Jun 2026 11:46:57 -0700 Subject: [PATCH] operation: index VidCache by map instead of slice (#9853) VidCache.cache was a []VidInfo indexed directly by volume id, so caching one volume with a large id grew the backing array to that many entries (each 48 bytes), allocating a zeroed slot for every unused id below it. A single id of 32M cost ~1.5GB resident, plus geometric realloc churn as the append loop doubled the array. Use map[uint32]VidInfo so memory scales with the number of volumes actually cached rather than the largest id seen. Parse ids with ParseUint(.,32) so values outside the uint32 volume-id range are rejected instead of silently wrapping into a key. --- weed/operation/lookup_vid_cache.go | 41 +++++++++++++------------ weed/operation/lookup_vid_cache_test.go | 22 +++++++++++++ 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/weed/operation/lookup_vid_cache.go b/weed/operation/lookup_vid_cache.go index 248fc17de..e2ff405a9 100644 --- a/weed/operation/lookup_vid_cache.go +++ b/weed/operation/lookup_vid_cache.go @@ -17,43 +17,46 @@ type VidInfo struct { } type VidCache struct { sync.RWMutex - cache []VidInfo + cache map[uint32]VidInfo } func (vc *VidCache) Get(vid string) ([]Location, error) { - id, err := strconv.Atoi(vid) + id, err := strconv.ParseUint(vid, 10, 32) if err != nil { glog.V(1).Infof("Unknown volume id %s", vid) return nil, err } + if id == 0 { + return nil, ErrorNotFound + } vc.RLock() defer vc.RUnlock() - if 0 < id && id <= len(vc.cache) { - if vc.cache[id-1].Locations == nil { - return nil, errors.New("not set") - } - if vc.cache[id-1].NextRefreshTime.Before(time.Now()) { - return nil, errors.New("expired") - } - return vc.cache[id-1].Locations, nil + info, found := vc.cache[uint32(id)] + if !found || info.Locations == nil { + return nil, ErrorNotFound } - return nil, ErrorNotFound + if info.NextRefreshTime.Before(time.Now()) { + return nil, errors.New("expired") + } + return info.Locations, nil } + func (vc *VidCache) Set(vid string, locations []Location, duration time.Duration) { - id, err := strconv.Atoi(vid) + id, err := strconv.ParseUint(vid, 10, 32) if err != nil { glog.V(1).Infof("Unknown volume id %s", vid) return } + if id == 0 { + return + } vc.Lock() defer vc.Unlock() - if id > len(vc.cache) { - for i := id - len(vc.cache); i > 0; i-- { - vc.cache = append(vc.cache, VidInfo{}) - } + if vc.cache == nil { + vc.cache = make(map[uint32]VidInfo) } - if id > 0 { - vc.cache[id-1].Locations = locations - vc.cache[id-1].NextRefreshTime = time.Now().Add(duration) + vc.cache[uint32(id)] = VidInfo{ + Locations: locations, + NextRefreshTime: time.Now().Add(duration), } } diff --git a/weed/operation/lookup_vid_cache_test.go b/weed/operation/lookup_vid_cache_test.go index 9c9e2affb..a6ca0a556 100644 --- a/weed/operation/lookup_vid_cache_test.go +++ b/weed/operation/lookup_vid_cache_test.go @@ -24,3 +24,25 @@ func TestCaching(t *testing.T) { t.Fatal("Not found vid 123") } } + +// a single large volume id must not allocate an entry per id below it +func TestCachingLargeVolumeId(t *testing.T) { + var vc VidCache + locations := []Location{{Url: "a.com:8080"}} + vc.Set("32000000", locations, time.Minute) + if got := len(vc.cache); got != 1 { + t.Fatalf("expected 1 cached entry, got %d", got) + } + if ret, _ := vc.Get("32000000"); ret == nil { + t.Fatal("Not found vid 32000000") + } + + // ids beyond uint32 are not real volume ids and must not wrap into the cache + vc.Set("4294967296", locations, time.Minute) + if got := len(vc.cache); got != 1 { + t.Fatalf("out-of-range id must not be cached, got %d entries", got) + } + if ret, _ := vc.Get("4294967296"); ret != nil { + t.Fatal("out-of-range vid 4294967296 should not be found") + } +}