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") + } +}