wdclient: age vid map entries by generation instead of chaining snapshots (#10506)

* wdclient: age vid map entries by generation instead of chaining snapshots The vid map kept its history as a linked list of past snapshots, trimmed in place by storing nil into a node's cache pointer. That cost up to six full copies of the volume-location map, a recursive walk taking a different lock per level, and deletes that had to cascade through every generation. It also had to special-case explicitly-empty entries, or fallback would resurrect locations a newer snapshot had cleared. Keep one map instead, and stamp each entry with the generation it was learned in. resetVidMap bumps the generation and drops entries that were not relearned within the retained window, which is the same retention the chain provided: an entry survives DefaultVidMapCacheSize resets. The first write of a generation replaces an entry rather than merging into it, so a volume that moved answers with where it is now — the property a fresh map per reset used to give for free. Entries are copy-on-write, so locations handed to a caller are no longer shifted underneath it by a concurrent delete. The map is never swapped now, so the client-side lock and its stable / current accessors go away with it. * wdclient: make vid map entries immutable and drop them once emptied Review follow-up. Updating an entry in place left the copy-on-write guarantee resting on callers never holding the entry pointer; install a new entry instead, so the rule is simply that a stored entry never changes. Deleting a volume's last location now drops the entry rather than keeping an empty one, which a client that never resets would otherwise hold for every volume it ever saw deleted. Lookups already treat an empty entry as a miss, so nothing observable changes. * wdclient: let the newest generation decide between regular and EC locations GetLocations checked the regular map first whatever its generation, so a volume that was EC encoded kept answering with the regular copies the previous master knew until they expired — for as long as the retained window, since nothing relearns a copy that no longer exists. The snapshot chain did not have this problem: the newest map was consulted first and only a volume it knew nothing about fell through to older ones. Restore that by comparing generations, with regular copies winning a tie, since a tie means one generation reported both.
2026-08-16 12:16:36 +00:00 · 2026-07-31 02:16:33 -07:00
parent ae4839e005
commit 7b8188fc41
4 changed files with 574 additions and 375 deletions
@@ -8,7 +8,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"

 	"github.com/seaweedfs/seaweedfs/weed/pb"

@@ -32,10 +31,19 @@ func (l Location) ServerAddress() pb.ServerAddress {
 	return pb.NewServerAddressWithGrpcPort(l.Url, l.GrpcPort)
 }

+// locationsEntry is what a volume id maps to: the locations themselves plus the
+// generation they were learned in. An entry is immutable once stored; every
+// update installs a new one, so locations handed to a reader are never
+// rewritten underneath it.
+type locationsEntry struct {
+	locations  []Location
+	generation uint64
+}
+
 type vidMap struct {
 	sync.RWMutex
-	vid2Locations   map[uint32][]Location
-	ecVid2Locations map[uint32][]Location
+	vid2Locations   map[uint32]*locationsEntry
+	ecVid2Locations map[uint32]*locationsEntry
 	// serverRefCount tracks how many vid locations (regular + EC) currently
 	// reference each volume server address. Maintaining it incrementally lets
 	// hasVolumeServer answer in O(1) instead of walking every volume entry.
@@ -43,15 +51,25 @@ type vidMap struct {
 	// pass either "host:port" or "host:port.grpc" find the same entry.
 	serverRefCount map[string]int
 	DataCenter     string
-	cache          atomic.Pointer[vidMap]
+	// generation counts resets. Each entry remembers the generation it was
+	// learned in, so history expires per volume rather than by keeping
+	// snapshot copies of the whole map.
+	generation uint64
+	// retainGenerations is how many resets an entry survives without being
+	// refreshed before reset drops it.
+	retainGenerations uint64
 }

-func newVidMap(dataCenter string) *vidMap {
+func newVidMap(dataCenter string, retainGenerations int) *vidMap {
+	if retainGenerations <= 0 {
+		retainGenerations = DefaultVidMapCacheSize
+	}
 	return &vidMap{
-		vid2Locations:   make(map[uint32][]Location),
-		ecVid2Locations: make(map[uint32][]Location),
-		serverRefCount:  make(map[string]int),
-		DataCenter:      dataCenter,
+		vid2Locations:     make(map[uint32]*locationsEntry),
+		ecVid2Locations:   make(map[uint32]*locationsEntry),
+		serverRefCount:    make(map[string]int),
+		DataCenter:        dataCenter,
+		retainGenerations: uint64(retainGenerations),
 	}
 }

@@ -128,31 +146,42 @@ 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()
+	vc.RLock()
+	defer vc.RUnlock()

-	locations, found = vc.getLocations(vid)
-	if found {
-		// If volume is explicitly tracked (found=true), return its locations even if empty.
-		// An empty array means "volume has no locations" (e.g., during pod restart),
-		// which is different from "volume never existed" (found=false).
-		// Don't fall back to stale cache for explicitly empty volumes.
-		if len(locations) > 0 {
-			return locations, found
+	regular, hasRegular := lookupEntry(vc.vid2Locations, vid)
+	ec, hasEc := lookupEntry(vc.ecVid2Locations, vid)
+
+	switch {
+	case hasRegular && hasEc:
+		// Whichever was learned last wins: once a volume is EC encoded, the
+		// regular copies a previous generation knew must stop answering for
+		// it, and a decoded volume must stop answering with its shards. A tie
+		// means one generation reported both, where the regular copies serve.
+		if ec.generation > regular.generation {
+			return ec.locations, true
 		}
-		// Volume exists but has no locations - return empty, don't check cache
+		return regular.locations, true
+	case hasRegular:
+		return regular.locations, true
+	case hasEc:
+		return ec.locations, true
+	}
+
+	// Nothing older to fall back to: a volume's history lives in its own entry,
+	// so a volume whose locations are all gone (a pod restarting, say) is a
+	// miss rather than a reason to serve what it used to have.
+	return nil, false
+}
+
+// lookupEntry returns vid's entry when it still holds locations. Callers must
+// hold the lock.
+func lookupEntry(vid2Locations map[uint32]*locationsEntry, vid uint32) (*locationsEntry, bool) {
+	entry, found := vid2Locations[vid]
+	if !found || len(entry.locations) == 0 {
 		return nil, false
 	}
-
-	// Volume not found in current map - check cache for unknown volumes
-	if cachedMap != nil {
-		return cachedMap.GetLocations(vid)
-	}
-
-	return nil, false
+	return entry, true
 }

 func (vc *vidMap) GetLocationsClone(vid uint32) (locations []Location, found bool) {
@@ -168,41 +197,17 @@ func (vc *vidMap) GetLocationsClone(vid uint32) (locations []Location, found boo
 	return nil, false
 }

-func (vc *vidMap) getLocations(vid uint32) (locations []Location, found bool) {
-	vc.RLock()
-	defer vc.RUnlock()
-
-	locations, found = vc.vid2Locations[vid]
-	if found && len(locations) > 0 {
-		return
-	}
-	locations, found = vc.ecVid2Locations[vid]
-	return
-}
-
 // hasVolumeServer reports whether any tracked volume (regular or EC) is hosted
-// on addr. It walks the cache chain so recently expired maps are still
-// considered. Used to gate admission of operations targeting a volume server.
-// The lookup is O(1) thanks to serverRefCount; we still consult the cache
-// chain to keep covering volume servers that just rolled out of the live map.
+// on addr, including volumes still held from earlier generations. Used to gate
+// admission of operations targeting a volume server.
 func (vc *vidMap) hasVolumeServer(addr pb.ServerAddress) bool {
 	key := addr.ToHttpAddress()
 	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 != nil {
-		return cachedMap.hasVolumeServer(addr)
-	}
-	return false
+	defer vc.RUnlock()
+	return vc.serverRefCount[key] > 0
 }

 func (vc *vidMap) addLocation(vid uint32, location Location) {
@@ -211,22 +216,7 @@ func (vc *vidMap) addLocation(vid uint32, location Location) {

 	glog.V(4).Infof("+ volume id %d: %+v", vid, location)

-	locations, found := vc.vid2Locations[vid]
-	if !found {
-		vc.vid2Locations[vid] = []Location{location}
-		vc.incrementServerRef(locationServerKey(location))
-		return
-	}
-
-	for _, loc := range locations {
-		if loc.Url == location.Url {
-			return
-		}
-	}
-
-	vc.vid2Locations[vid] = append(locations, location)
-	vc.incrementServerRef(locationServerKey(location))
-
+	vc.addLocationToMap(vc.vid2Locations, vid, location)
 }

 func (vc *vidMap) addEcLocation(vid uint32, location Location) {
@@ -235,88 +225,133 @@ func (vc *vidMap) addEcLocation(vid uint32, location Location) {

 	glog.V(4).Infof("+ ec volume id %d: %+v", vid, location)

-	locations, found := vc.ecVid2Locations[vid]
-	if !found {
-		vc.ecVid2Locations[vid] = []Location{location}
+	vc.addLocationToMap(vc.ecVid2Locations, vid, location)
+}
+
+// addLocationToMap records location for vid. The first write of a generation
+// replaces what an earlier one held instead of merging with it: after a reset
+// the new master is the authority, so a volume that moved must not keep
+// answering with the server it moved off. Callers must hold the write lock.
+func (vc *vidMap) addLocationToMap(vid2Locations map[uint32]*locationsEntry, vid uint32, location Location) {
+	entry, found := vid2Locations[vid]
+	if !found || entry.generation != vc.generation {
+		if found {
+			vc.releaseEntry(entry)
+		}
+		vid2Locations[vid] = &locationsEntry{
+			locations:  []Location{location},
+			generation: vc.generation,
+		}
 		vc.incrementServerRef(locationServerKey(location))
 		return
 	}

-	for _, loc := range locations {
+	for _, loc := range entry.locations {
 		if loc.Url == location.Url {
 			return
 		}
 	}

-	vc.ecVid2Locations[vid] = append(locations, location)
+	locations := make([]Location, 0, len(entry.locations)+1)
+	locations = append(locations, entry.locations...)
+	locations = append(locations, location)
+	vid2Locations[vid] = &locationsEntry{locations: locations, generation: entry.generation}
 	vc.incrementServerRef(locationServerKey(location))
-
 }

 func (vc *vidMap) deleteLocation(vid uint32, location Location) {
-	if cachedMap := vc.cache.Load(); cachedMap != nil {
-		cachedMap.deleteLocation(vid, location)
-	}
-
 	vc.Lock()
 	defer vc.Unlock()

 	glog.V(4).Infof("- volume id %d: %+v", vid, location)

-	locations, found := vc.vid2Locations[vid]
-	if !found {
-		return
-	}
-
-	for i, loc := range locations {
-		if loc.Url == location.Url {
-			vc.vid2Locations[vid] = append(locations[0:i], locations[i+1:]...)
-			vc.decrementServerRef(locationServerKey(loc))
-			break
-		}
-	}
+	vc.deleteLocationFromMap(vc.vid2Locations, vid, location)
 }

 func (vc *vidMap) deleteEcLocation(vid uint32, location Location) {
-	if cachedMap := vc.cache.Load(); cachedMap != nil {
-		cachedMap.deleteEcLocation(vid, location)
-	}
-
 	vc.Lock()
 	defer vc.Unlock()

 	glog.V(4).Infof("- ec volume id %d: %+v", vid, location)

-	locations, found := vc.ecVid2Locations[vid]
+	vc.deleteLocationFromMap(vc.ecVid2Locations, vid, location)
+}
+
+// deleteLocationFromMap drops one location from vid's entry, and the entry
+// itself once its last location is gone. The generation is untouched: a delete
+// only speaks about the location it names, it does not make the rest of the
+// entry any fresher. Callers must hold the write lock.
+func (vc *vidMap) deleteLocationFromMap(vid2Locations map[uint32]*locationsEntry, vid uint32, location Location) {
+	entry, found := vid2Locations[vid]
 	if !found {
 		return
 	}

-	for i, loc := range locations {
-		if loc.Url == location.Url {
-			vc.ecVid2Locations[vid] = append(locations[0:i], locations[i+1:]...)
-			vc.decrementServerRef(locationServerKey(loc))
-			break
+	for i, loc := range entry.locations {
+		if loc.Url != location.Url {
+			continue
 		}
+		vc.decrementServerRef(locationServerKey(loc))
+		if len(entry.locations) == 1 {
+			delete(vid2Locations, vid)
+			return
+		}
+		remaining := make([]Location, 0, len(entry.locations)-1)
+		remaining = append(remaining, entry.locations[:i]...)
+		remaining = append(remaining, entry.locations[i+1:]...)
+		vid2Locations[vid] = &locationsEntry{locations: remaining, generation: entry.generation}
+		return
 	}
 }

 func (vc *vidMap) deleteVid(vid uint32) {
-	if cachedMap := vc.cache.Load(); cachedMap != nil {
-		cachedMap.deleteVid(vid)
-	}
-
 	vc.Lock()
 	defer vc.Unlock()

-	for _, loc := range vc.vid2Locations[vid] {
+	if entry, found := vc.vid2Locations[vid]; found {
+		vc.releaseEntry(entry)
+		delete(vc.vid2Locations, vid)
+	}
+	if entry, found := vc.ecVid2Locations[vid]; found {
+		vc.releaseEntry(entry)
+		delete(vc.ecVid2Locations, vid)
+	}
+}
+
+// reset starts a new generation, as when the master changes and everything it
+// told us has to be relearned. Entries stay readable while they are relearned
+// and are dropped once they fall out of the retained window.
+func (vc *vidMap) reset() {
+	vc.Lock()
+	defer vc.Unlock()
+
+	vc.generation++
+	if vc.generation <= vc.retainGenerations {
+		return
+	}
+	oldest := vc.generation - vc.retainGenerations
+	vc.expire(vc.vid2Locations, oldest)
+	vc.expire(vc.ecVid2Locations, oldest)
+}
+
+// expire drops entries last refreshed before oldest. Callers must hold the
+// write lock.
+func (vc *vidMap) expire(vid2Locations map[uint32]*locationsEntry, oldest uint64) {
+	for vid, entry := range vid2Locations {
+		if entry.generation >= oldest {
+			continue
+		}
+		vc.releaseEntry(entry)
+		delete(vid2Locations, vid)
+	}
+}
+
+// releaseEntry drops the server references an entry holds. Callers must hold
+// the write lock.
+func (vc *vidMap) releaseEntry(entry *locationsEntry) {
+	for _, loc := range entry.locations {
 		vc.decrementServerRef(locationServerKey(loc))
 	}
-	for _, loc := range vc.ecVid2Locations[vid] {
-		vc.decrementServerRef(locationServerKey(loc))
-	}
-	delete(vc.vid2Locations, vid)
-	delete(vc.ecVid2Locations, vid)
 }

 // incrementServerRef increases the refcount for key. Empty keys are skipped
@@ -0,0 +1,330 @@
+package wdclient
+
+import (
+	"runtime"
+	"sync"
+	"testing"
+
+	"github.com/seaweedfs/seaweedfs/weed/pb"
+)
+
+func urlsOf(locations []Location) []string {
+	urls := make([]string, 0, len(locations))
+	for _, loc := range locations {
+		urls = append(urls, loc.Url)
+	}
+	return urls
+}
+
+// A volume that moved while we were talking to the previous master must answer
+// with where it is now, not with both servers: the first write of a generation
+// replaces the entry instead of merging into it.
+func TestAddLocationReplacesEarlierGeneration(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vid := uint32(3)
+	movedFrom := Location{Url: "10.0.0.1:8080"}
+	movedTo := Location{Url: "10.0.0.2:8080"}
+
+	vm.addLocation(vid, movedFrom)
+	vm.reset()
+	vm.addLocation(vid, movedTo)
+
+	locs, found := vm.GetLocations(vid)
+	if !found || len(locs) != 1 || locs[0].Url != movedTo.Url {
+		t.Fatalf("expected only %s after the move, got %v", movedTo.Url, urlsOf(locs))
+	}
+	if vm.hasVolumeServer(pb.ServerAddress(movedFrom.Url)) {
+		t.Errorf("server %s should no longer be referenced after the volume moved", movedFrom.Url)
+	}
+	if !vm.hasVolumeServer(pb.ServerAddress(movedTo.Url)) {
+		t.Errorf("server %s should be referenced after the volume moved", movedTo.Url)
+	}
+}
+
+// Replicas reported within one generation accumulate, and a repeated report of
+// the same server is not counted twice.
+func TestAddLocationMergesWithinGeneration(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vid := uint32(4)
+	first := Location{Url: "10.0.0.1:8080"}
+	second := Location{Url: "10.0.0.2:8080"}
+
+	vm.addLocation(vid, first)
+	vm.addLocation(vid, second)
+	vm.addLocation(vid, first)
+
+	locs, found := vm.GetLocations(vid)
+	if !found || len(locs) != 2 {
+		t.Fatalf("expected both replicas, got %v", urlsOf(locs))
+	}
+
+	// One delete must be enough to evict the server: the duplicate report of
+	// `first` must not have taken a second reference.
+	vm.deleteLocation(vid, first)
+	if vm.hasVolumeServer(pb.ServerAddress(first.Url)) {
+		t.Errorf("server %s should be evicted after a single delete", first.Url)
+	}
+}
+
+// An entry survives exactly retainGenerations resets without being relearned.
+func TestResetRetainsUntilWindowExpires(t *testing.T) {
+	const retain = 2
+	vm := newVidMap("", retain)
+	vid := uint32(5)
+	location := Location{Url: "10.0.0.1:8080"}
+	vm.addLocation(vid, location)
+
+	for i := 0; i < retain; i++ {
+		vm.reset()
+		if _, found := vm.GetLocations(vid); !found {
+			t.Fatalf("location should still be retained after %d reset(s)", i+1)
+		}
+	}
+
+	vm.reset()
+	if _, found := vm.GetLocations(vid); found {
+		t.Errorf("location should be dropped after %d resets", retain+1)
+	}
+	if vm.hasVolumeServer(pb.ServerAddress(location.Url)) {
+		t.Errorf("expiry should release the server reference for %s", location.Url)
+	}
+	if len(vm.serverRefCount) != 0 {
+		t.Errorf("expiry left %d dangling server refcounts", len(vm.serverRefCount))
+	}
+}
+
+// Relearning an entry restarts its retention window.
+func TestResetWindowRestartsOnRelearn(t *testing.T) {
+	const retain = 2
+	vm := newVidMap("", retain)
+	vid := uint32(6)
+	location := Location{Url: "10.0.0.1:8080"}
+
+	for i := 0; i < retain+3; i++ {
+		vm.addLocation(vid, location)
+		vm.reset()
+	}
+
+	if _, found := vm.GetLocations(vid); !found {
+		t.Error("a location relearned every generation must never expire")
+	}
+}
+
+// A delete names one location; it must not make the rest of a stale entry look
+// freshly learned and so outlive its retention window.
+func TestDeleteLocationDoesNotRefreshGeneration(t *testing.T) {
+	const retain = 2
+	vm := newVidMap("", retain)
+	vid := uint32(7)
+	first := Location{Url: "10.0.0.1:8080"}
+	second := Location{Url: "10.0.0.2:8080"}
+
+	vm.addLocation(vid, first)
+	vm.addLocation(vid, second)
+	for i := 0; i < retain; i++ {
+		vm.reset()
+	}
+	vm.deleteLocation(vid, first)
+
+	locs, found := vm.GetLocations(vid)
+	if !found || len(locs) != 1 || locs[0].Url != second.Url {
+		t.Fatalf("expected %s to remain, got found=%v %v", second.Url, found, urlsOf(locs))
+	}
+
+	vm.reset()
+	if _, found := vm.GetLocations(vid); found {
+		t.Error("the surviving location was learned in the expired generation and should be dropped")
+	}
+}
+
+// Encoding a volume must stop the regular copies a previous generation knew
+// from answering for it, and decoding it must stop its shards from answering.
+func TestNewestGenerationWinsAcrossEcTransition(t *testing.T) {
+	regular := Location{Url: "10.0.0.1:8080"}
+	ecShard := Location{Url: "10.0.0.2:8080"}
+
+	t.Run("encoded", func(t *testing.T) {
+		vm := newVidMap("", DefaultVidMapCacheSize)
+		vm.addLocation(1, regular)
+		vm.reset()
+		vm.addEcLocation(1, ecShard)
+
+		locs, found := vm.GetLocations(1)
+		if !found || len(locs) != 1 || locs[0].Url != ecShard.Url {
+			t.Fatalf("expected the freshly learned EC shard, got found=%v %v", found, urlsOf(locs))
+		}
+	})
+
+	t.Run("decoded", func(t *testing.T) {
+		vm := newVidMap("", DefaultVidMapCacheSize)
+		vm.addEcLocation(1, ecShard)
+		vm.reset()
+		vm.addLocation(1, regular)
+
+		locs, found := vm.GetLocations(1)
+		if !found || len(locs) != 1 || locs[0].Url != regular.Url {
+			t.Fatalf("expected the freshly learned regular copy, got found=%v %v", found, urlsOf(locs))
+		}
+	})
+
+	t.Run("same generation prefers regular", func(t *testing.T) {
+		vm := newVidMap("", DefaultVidMapCacheSize)
+		vm.addEcLocation(1, ecShard)
+		vm.addLocation(1, regular)
+
+		locs, found := vm.GetLocations(1)
+		if !found || len(locs) != 1 || locs[0].Url != regular.Url {
+			t.Fatalf("expected the regular copy, got found=%v %v", found, urlsOf(locs))
+		}
+	})
+}
+
+// Losing the last location drops the entry: a client that never resets should
+// not accumulate one empty entry per volume it has ever seen deleted.
+func TestDeleteLastLocationDropsEntry(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vid := uint32(12)
+	location := Location{Url: "10.0.0.1:8080"}
+
+	vm.addLocation(vid, location)
+	vm.deleteLocation(vid, location)
+
+	if _, found := vm.GetLocations(vid); found {
+		t.Error("a volume with no locations left must not resolve")
+	}
+	if len(vm.vid2Locations) != 0 {
+		t.Errorf("expected the emptied entry to be dropped, got %v", vm.vid2Locations)
+	}
+	if len(vm.serverRefCount) != 0 {
+		t.Errorf("server refcounts leaked: %v", vm.serverRefCount)
+	}
+}
+
+// EC locations follow the same rules as regular ones, and back a volume whose
+// regular locations are gone.
+func TestEcLocationsFollowGenerationRules(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vid := uint32(8)
+	regular := Location{Url: "10.0.0.1:8080"}
+	ecShard := Location{Url: "10.0.0.2:8080"}
+	movedEcShard := Location{Url: "10.0.0.3:8080"}
+
+	vm.addLocation(vid, regular)
+	vm.addEcLocation(vid, ecShard)
+
+	locs, found := vm.GetLocations(vid)
+	if !found || len(locs) != 1 || locs[0].Url != regular.Url {
+		t.Fatalf("regular locations should win while they exist, got %v", urlsOf(locs))
+	}
+
+	// Regular copy goes away: the EC shards still serve the volume.
+	vm.deleteLocation(vid, regular)
+	locs, found = vm.GetLocations(vid)
+	if !found || len(locs) != 1 || locs[0].Url != ecShard.Url {
+		t.Fatalf("expected EC shard location, got found=%v %v", found, urlsOf(locs))
+	}
+
+	// EC shards move under a new master: no merging with the old report.
+	vm.reset()
+	vm.addEcLocation(vid, movedEcShard)
+	locs, found = vm.GetLocations(vid)
+	if !found || len(locs) != 1 || locs[0].Url != movedEcShard.Url {
+		t.Fatalf("expected only the moved EC shard, got found=%v %v", found, urlsOf(locs))
+	}
+}
+
+// Locations handed to a caller are never rewritten underneath it.
+func TestGetLocationsResultIsStable(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vid := uint32(9)
+	first := Location{Url: "10.0.0.1:8080"}
+	second := Location{Url: "10.0.0.2:8080"}
+
+	vm.addLocation(vid, first)
+	vm.addLocation(vid, second)
+
+	locs, found := vm.GetLocations(vid)
+	if !found || len(locs) != 2 {
+		t.Fatalf("expected both replicas, got %v", urlsOf(locs))
+	}
+	snapshot := append([]Location(nil), locs...)
+
+	vm.deleteLocation(vid, first)
+	vm.addLocation(vid, Location{Url: "10.0.0.3:8080"})
+
+	for i := range snapshot {
+		if locs[i] != snapshot[i] {
+			t.Errorf("location %d changed under the caller: %v became %v", i, snapshot[i], locs[i])
+		}
+	}
+}
+
+func TestNewVidMapDefaultsRetention(t *testing.T) {
+	for _, retain := range []int{0, -1} {
+		if got := newVidMap("", retain).retainGenerations; got != DefaultVidMapCacheSize {
+			t.Errorf("newVidMap(%d) retention = %d, want %d", retain, got, DefaultVidMapCacheSize)
+		}
+	}
+}
+
+// Readers must never see a spurious miss for a volume that stays live, and the
+// bookkeeping must survive concurrent writers. Run with -race.
+func TestConcurrentResetAndUpdates(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	live := Location{Url: "10.0.0.1:8080"}
+	churn := Location{Url: "10.0.0.2:8080"}
+	const liveVid, churnVid = 1, 2
+
+	vm.addLocation(liveVid, live)
+
+	var wg sync.WaitGroup
+	stop := make(chan struct{})
+
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for {
+				select {
+				case <-stop:
+					return
+				default:
+					if _, found := vm.GetLocations(liveVid); !found {
+						t.Error("a volume that is relearned every generation must always resolve")
+						return
+					}
+					vm.hasVolumeServer(pb.ServerAddress(live.Url))
+					vm.GetLocationsClone(churnVid)
+					runtime.Gosched()
+				}
+			}
+		}()
+	}
+
+	for i := 0; i < 4; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < 200; j++ {
+				vm.addEcLocation(churnVid, churn)
+				vm.deleteEcLocation(churnVid, churn)
+				vm.deleteVid(churnVid)
+			}
+		}()
+	}
+
+	for i := 0; i < 300; i++ {
+		vm.reset()
+		vm.addLocation(liveVid, live)
+	}
+	close(stop)
+	wg.Wait()
+
+	if _, found := vm.GetLocations(liveVid); !found {
+		t.Fatal("live volume lost after the churn")
+	}
+	vm.deleteVid(liveVid)
+	if len(vm.serverRefCount) != 0 {
+		t.Errorf("server refcounts leaked: %v", vm.serverRefCount)
+	}
+}
@@ -8,7 +8,6 @@ import (
 	"sort"
 	"strconv"
 	"strings"
-	"sync"

 	"golang.org/x/sync/singleflight"

@@ -27,28 +26,23 @@ type VolumeLocationProvider interface {
 // vidMapClient provides volume location caching with pluggable lookup
 // It wraps the battle-tested vidMap with customizable volume lookup strategies
 type vidMapClient struct {
-	vidMap          *vidMap
-	vidMapLock      sync.RWMutex
-	vidMapCacheSize int
-	provider        VolumeLocationProvider
-	vidLookupGroup  singleflight.Group
+	vidMap         *vidMap
+	provider       VolumeLocationProvider
+	vidLookupGroup singleflight.Group
 }

 const (
-	// DefaultVidMapCacheSize is the default number of historical vidMap snapshots to keep
-	// This provides cache history when volumes move between servers
+	// DefaultVidMapCacheSize is the default number of resets a volume location
+	// survives without being relearned. This provides cache history when
+	// volumes move between servers.
 	DefaultVidMapCacheSize = 5
 )

 // newVidMapClient creates a new client with the given provider and data center
 func newVidMapClient(provider VolumeLocationProvider, dataCenter string, cacheSize int) *vidMapClient {
-	if cacheSize <= 0 {
-		cacheSize = DefaultVidMapCacheSize
-	}
 	return &vidMapClient{
-		vidMap:          newVidMap(dataCenter),
-		vidMapCacheSize: cacheSize,
-		provider:        provider,
+		vidMap:   newVidMap(dataCenter, cacheSize),
+		provider: provider,
 	}
 }

@@ -59,12 +53,9 @@ func (vc *vidMapClient) GetLookupFileIdFunction() LookupFileIdFunctionType {

 // LookupFileIdWithFallback looks up a file ID, checking cache first, then using provider
 func (vc *vidMapClient) LookupFileIdWithFallback(ctx context.Context, fileId string) (fullUrls []string, err error) {
-	// Try cache first - hold read lock during entire vidMap access to prevent swap during operation
-	vc.vidMapLock.RLock()
-	vm := vc.vidMap
-	dataCenter := vm.DataCenter
-	fullUrls, err = vm.LookupFileId(ctx, fileId)
-	vc.vidMapLock.RUnlock()
+	// Try cache first
+	dataCenter := vc.vidMap.DataCenter
+	fullUrls, err = vc.vidMap.LookupFileId(ctx, fileId)

 	// Cache hit - return immediately
 	if err == nil && len(fullUrls) > 0 {
@@ -146,9 +137,6 @@ func (vc *vidMapClient) LookupVolumeIdsWithFallback(ctx context.Context, volumeI
 	// Check cache first and parse volume IDs once
 	vidStringToUint := make(map[string]uint32, len(volumeIds))

-	// Get stable pointer to vidMap with minimal lock hold time
-	vm := vc.getStableVidMap()
-
 	for _, vidString := range volumeIds {
 		vid, err := strconv.ParseUint(vidString, 10, 32)
 		if err != nil {
@@ -156,7 +144,7 @@ func (vc *vidMapClient) LookupVolumeIdsWithFallback(ctx context.Context, volumeI
 		}
 		vidStringToUint[vidString] = uint32(vid)

-		locations, found := vm.GetLocations(uint32(vid))
+		locations, found := vc.vidMap.GetLocations(uint32(vid))
 		if found && len(locations) > 0 {
 			result[vidString] = locations
 		} else {
@@ -178,12 +166,9 @@ func (vc *vidMapClient) LookupVolumeIdsWithFallback(ctx context.Context, volumeI
 		stillNeedLookup := make([]string, 0, len(needsLookup))
 		batchResult := make(map[string][]Location)

-		// Get stable pointer with minimal lock hold time
-		vm := vc.getStableVidMap()
-
 		for _, vidString := range needsLookup {
 			vid := vidStringToUint[vidString] // Use pre-parsed value
-			if locations, found := vm.GetLocations(vid); found && len(locations) > 0 {
+			if locations, found := vc.vidMap.GetLocations(vid); found && len(locations) > 0 {
 				batchResult[vidString] = locations
 			} else {
 				stillNeedLookup = append(stillNeedLookup, vidString)
@@ -244,117 +229,74 @@ func (vc *vidMapClient) LookupVolumeIdsWithFallback(ctx context.Context, volumeI
 	return result, errors.Join(lookupErrors...)
 }

-// getStableVidMap gets a stable pointer to the vidMap, releasing the lock immediately.
-// WARNING: Use with caution. The returned vidMap pointer is stable (won't be garbage collected
-// due to cache chain), but the vidMapClient.vidMap field may be swapped by resetVidMap().
-// For operations that must use the current vidMap atomically, use withCurrentVidMap() instead.
-func (vc *vidMapClient) getStableVidMap() *vidMap {
-	vc.vidMapLock.RLock()
-	vm := vc.vidMap
-	vc.vidMapLock.RUnlock()
-	return vm
-}
-
-// withCurrentVidMap executes a function with the current vidMap under a read lock.
-// This guarantees the vidMap instance cannot be swapped during the function execution.
-// Use this when you need atomic access to the current vidMap for multiple operations.
-func (vc *vidMapClient) withCurrentVidMap(f func(vm *vidMap)) {
-	vc.vidMapLock.RLock()
-	defer vc.vidMapLock.RUnlock()
-	f(vc.vidMap)
-}
-
 // Public methods for external access
+//
+// The vidMap itself is never replaced, so these all read the one map under its
+// own lock. Resets bump its generation instead of swapping in a fresh instance.

 // GetLocations safely retrieves volume locations
 func (vc *vidMapClient) GetLocations(vid uint32) (locations []Location, found bool) {
-	return vc.getStableVidMap().GetLocations(vid)
+	return vc.vidMap.GetLocations(vid)
 }

 // GetLocationsClone safely retrieves a clone of volume locations
 func (vc *vidMapClient) GetLocationsClone(vid uint32) (locations []Location, found bool) {
-	return vc.getStableVidMap().GetLocationsClone(vid)
+	return vc.vidMap.GetLocationsClone(vid)
 }

 // GetVidLocations safely retrieves volume locations by string ID
 func (vc *vidMapClient) GetVidLocations(vid string) (locations []Location, err error) {
-	return vc.getStableVidMap().GetVidLocations(vid)
+	return vc.vidMap.GetVidLocations(vid)
 }

 // LookupFileId safely looks up URLs for a file ID
 func (vc *vidMapClient) LookupFileId(ctx context.Context, fileId string) (fullUrls []string, err error) {
-	return vc.getStableVidMap().LookupFileId(ctx, fileId)
+	return vc.vidMap.LookupFileId(ctx, fileId)
 }

 // LookupVolumeServerUrl safely looks up volume server URLs
 func (vc *vidMapClient) LookupVolumeServerUrl(vid string) (serverUrls []string, err error) {
-	return vc.getStableVidMap().LookupVolumeServerUrl(vid)
+	return vc.vidMap.LookupVolumeServerUrl(vid)
 }

 // HasVolumeServer reports whether addr is currently a known volume server
 // (hosts at least one volume or EC shard) in the cached vid map. Used by
 // admission paths that must only contact peers learned from the master.
 func (vc *vidMapClient) HasVolumeServer(addr pb.ServerAddress) bool {
-	return vc.getStableVidMap().hasVolumeServer(addr)
+	return vc.vidMap.hasVolumeServer(addr)
 }

 // GetDataCenter safely retrieves the data center
 func (vc *vidMapClient) GetDataCenter() string {
-	return vc.getStableVidMap().DataCenter
+	return vc.vidMap.DataCenter
 }

 // Thread-safe helpers for vidMap operations

 // addLocation adds a volume location
 func (vc *vidMapClient) addLocation(vid uint32, location Location) {
-	vc.withCurrentVidMap(func(vm *vidMap) {
-		vm.addLocation(vid, location)
-	})
+	vc.vidMap.addLocation(vid, location)
 }

 // deleteLocation removes a volume location
 func (vc *vidMapClient) deleteLocation(vid uint32, location Location) {
-	vc.withCurrentVidMap(func(vm *vidMap) {
-		vm.deleteLocation(vid, location)
-	})
+	vc.vidMap.deleteLocation(vid, location)
 }

 // addEcLocation adds an EC volume location
 func (vc *vidMapClient) addEcLocation(vid uint32, location Location) {
-	vc.withCurrentVidMap(func(vm *vidMap) {
-		vm.addEcLocation(vid, location)
-	})
+	vc.vidMap.addEcLocation(vid, location)
 }

 // deleteEcLocation removes an EC volume location
 func (vc *vidMapClient) deleteEcLocation(vid uint32, location Location) {
-	vc.withCurrentVidMap(func(vm *vidMap) {
-		vm.deleteEcLocation(vid, location)
-	})
+	vc.vidMap.deleteEcLocation(vid, location)
 }

-// resetVidMap resets the volume ID map
+// resetVidMap starts a new generation, as when the master changes: what the
+// previous one told us stays readable until it is relearned or expires.
 func (vc *vidMapClient) resetVidMap() {
-	vc.vidMapLock.Lock()
-	defer vc.vidMapLock.Unlock()
-
-	// Preserve the existing vidMap in the cache chain
-	tail := vc.vidMap
-
-	nvm := newVidMap(tail.DataCenter)
-	nvm.cache.Store(tail)
-	vc.vidMap = nvm
-
-	// Trim cache chain to vidMapCacheSize
-	node := tail
-	for i := 0; i < vc.vidMapCacheSize-1; i++ {
-		if node.cache.Load() == nil {
-			return
-		}
-		node = node.cache.Load()
-	}
-	// node is guaranteed to be non-nil after the loop
-	node.cache.Store(nil)
+	vc.vidMap.reset()
 }

 // InvalidateCache removes all cached locations for a volume ID
@@ -365,7 +307,5 @@ func (vc *vidMapClient) InvalidateCache(fileId string) {
 	if err != nil {
 		return
 	}
-	vc.withCurrentVidMap(func(vm *vidMap) {
-		vm.deleteVid(uint32(vid))
-	})
+	vc.vidMap.deleteVid(uint32(vid))
 }
@@ -8,22 +8,14 @@ import (
 func TestInvalidateCacheValidFileId(t *testing.T) {
 	// Create a simple vidMapClient (can use nil provider for this test)
 	vc := &vidMapClient{
-		vidMap:          newVidMap(""),
-		vidMapCacheSize: 5,
+		vidMap: newVidMap("", DefaultVidMapCacheSize),
 	}

 	// Add some locations to the cache
 	vid := uint32(456)
-	vc.vidMap.Lock()
-	vc.vidMap.vid2Locations[vid] = []Location{{Url: "http://server1:8080"}}
-	vc.vidMap.Unlock()
+	vc.addLocation(vid, Location{Url: "http://server1:8080"})

-	// Verify location exists
-	vc.vidMap.RLock()
-	_, found := vc.vidMap.vid2Locations[vid]
-	vc.vidMap.RUnlock()
-
-	if !found {
+	if _, found := vc.GetLocations(vid); !found {
 		t.Fatal("Location should exist before invalidation")
 	}

@@ -32,11 +24,7 @@ func TestInvalidateCacheValidFileId(t *testing.T) {
 	vc.InvalidateCache(fileId)

 	// Verify the locations were removed
-	vc.vidMap.RLock()
-	_, foundAfter := vc.vidMap.vid2Locations[vid]
-	vc.vidMap.RUnlock()
-
-	if foundAfter {
+	if _, found := vc.GetLocations(vid); found {
 		t.Errorf("Expected locations for vid %d to be removed after InvalidateCache", vid)
 	}
 }
@@ -57,222 +45,128 @@ func TestInvalidateCacheInvalidFileId(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			vc := &vidMapClient{
-				vidMap:          newVidMap(""),
-				vidMapCacheSize: 5,
+				vidMap: newVidMap("", DefaultVidMapCacheSize),
 			}

 			// Add a location to ensure the cache isn't empty
-			vc.vidMap.Lock()
-			vc.vidMap.vid2Locations[1] = []Location{{Url: "http://server:8080"}}
-			vc.vidMap.Unlock()
+			vc.addLocation(1, Location{Url: "http://server:8080"})

 			// This should not panic or cause errors
 			vc.InvalidateCache(tc.fileId)

 			// Verify the existing location is still there (not affected)
-			vc.vidMap.RLock()
-			_, found := vc.vidMap.vid2Locations[1]
-			vc.vidMap.RUnlock()
-
-			if !found {
+			if _, found := vc.GetLocations(1); !found {
 				t.Errorf("InvalidateCache with invalid fileId '%s' should not affect other entries", tc.fileId)
 			}
 		})
 	}
 }

-// TestInvalidateCacheWithHistory tests that invalidation propagates through cache history
+// TestInvalidateCacheWithHistory tests that invalidation also drops locations
+// that were learned before the last reset and are still being retained.
 func TestInvalidateCacheWithHistory(t *testing.T) {
 	vid := uint32(789)

-	// Create first vidMap with the volume
-	vm1 := newVidMap("")
-	vm1.Lock()
-	vm1.vid2Locations[vid] = []Location{{Url: "http://server1:8080"}}
-	vm1.Unlock()
-
-	// Create second vidMap with the cached first one
-	vm2 := newVidMap("")
-	vm2.cache.Store(vm1) // vm1 becomes the cache/history
-	vm2.Lock()
-	vm2.vid2Locations[vid] = []Location{{Url: "http://server2:8080"}}
-	vm2.Unlock()
-
-	// Create vidMapClient with vm2 as current
 	vc := &vidMapClient{
-		vidMap:          vm2,
-		vidMapCacheSize: 5,
+		vidMap: newVidMap("", DefaultVidMapCacheSize),
 	}

-	// Verify both have the vid before invalidation
-	vm2.RLock()
-	_, foundInCurrent := vm2.vid2Locations[vid]
-	vm2.RUnlock()
+	// Learned from an earlier master, then kept across a reset
+	vc.addLocation(vid, Location{Url: "http://server1:8080"})
+	vc.resetVidMap()

-	vm1.RLock()
-	_, foundInCache := vm1.vid2Locations[vid]
-	vm1.RUnlock()
-
-	if !foundInCurrent || !foundInCache {
-		t.Fatal("Both maps should have the vid before invalidation")
+	if _, found := vc.GetLocations(vid); !found {
+		t.Fatal("Retained location should still be readable after a reset")
 	}

-	// Invalidate the cache
-	fileId := "789,xyz123"
-	vc.InvalidateCache(fileId)
+	vc.InvalidateCache("789,xyz123")

-	// Check that current map doesn't have the vid
-	vm2.RLock()
-	_, foundInCurrentAfter := vm2.vid2Locations[vid]
-	vm2.RUnlock()
-
-	if foundInCurrentAfter {
-		t.Error("Expected vid to be removed from current vidMap after InvalidateCache")
-	}
-
-	// Check that cache doesn't have the vid either (recursive deletion)
-	vm1.RLock()
-	_, foundInCacheAfter := vm1.vid2Locations[vid]
-	vm1.RUnlock()
-
-	if foundInCacheAfter {
-		t.Error("Expected vid to be removed from cached vidMap as well (recursive deletion)")
+	if _, found := vc.GetLocations(vid); found {
+		t.Error("Expected retained location to be dropped by InvalidateCache")
 	}
 }

-// TestDeleteVidRecursion tests the deleteVid method removes from history chain
-func TestDeleteVidRecursion(t *testing.T) {
+// TestDeleteVidDropsRetainedGenerations tests that deleteVid removes a volume
+// no matter which generation it was last refreshed in.
+func TestDeleteVidDropsRetainedGenerations(t *testing.T) {
 	vid := uint32(999)

-	// Create a chain: vm3 -> vm2 -> vm1
-	vm1 := newVidMap("")
-	vm1.Lock()
-	vm1.vid2Locations[vid] = []Location{{Url: "http://server1:8080"}}
-	vm1.Unlock()
+	vm := newVidMap("", DefaultVidMapCacheSize)
+	vm.addLocation(vid, Location{Url: "http://server1:8080"})
+	vm.addEcLocation(vid, Location{Url: "http://server2:8080"})
+	vm.reset()
+	vm.reset()

-	vm2 := newVidMap("")
-	vm2.cache.Store(vm1)
-	vm2.Lock()
-	vm2.vid2Locations[vid] = []Location{{Url: "http://server2:8080"}}
-	vm2.Unlock()
-
-	vm3 := newVidMap("")
-	vm3.cache.Store(vm2)
-	vm3.Lock()
-	vm3.vid2Locations[vid] = []Location{{Url: "http://server3:8080"}}
-	vm3.Unlock()
-
-	// Verify all have the vid
-	vm3.RLock()
-	_, found3 := vm3.vid2Locations[vid]
-	vm3.RUnlock()
-
-	vm2.RLock()
-	_, found2 := vm2.vid2Locations[vid]
-	vm2.RUnlock()
-
-	vm1.RLock()
-	_, found1 := vm1.vid2Locations[vid]
-	vm1.RUnlock()
-
-	if !found1 || !found2 || !found3 {
-		t.Fatal("All maps should have the vid before deletion")
+	if _, found := vm.GetLocations(vid); !found {
+		t.Fatal("Volume should still be readable before deletion")
 	}

-	// Delete from vm3 (should cascade)
-	vm3.deleteVid(vid)
+	vm.deleteVid(vid)

-	// Verify it's gone from all
-	vm3.RLock()
-	_, found3After := vm3.vid2Locations[vid]
-	vm3.RUnlock()
-
-	vm2.RLock()
-	_, found2After := vm2.vid2Locations[vid]
-	vm2.RUnlock()
-
-	vm1.RLock()
-	_, found1After := vm1.vid2Locations[vid]
-	vm1.RUnlock()
-
-	if found3After {
-		t.Error("Expected vid to be removed from vm3")
+	if _, found := vm.GetLocations(vid); found {
+		t.Error("Expected volume to be gone after deleteVid")
 	}
-	if found2After {
-		t.Error("Expected vid to be removed from vm2 (cascaded)")
-	}
-	if found1After {
-		t.Error("Expected vid to be removed from vm1 (cascaded)")
+	if vm.hasVolumeServer("http://server1:8080") || vm.hasVolumeServer("http://server2:8080") {
+		t.Error("Expected deleteVid to release the server references it held")
 	}
 }

-// TestGetLocationsEmptyArrayNoFallback tests that empty location arrays don't fall back to cache
-// This tests the fix for the bug where volume pods restart and vidMap has empty array [],
-// but GetLocations would fall back to stale cached locations from before restart.
+// TestGetLocationsEmptyArrayNoFallback tests that a volume known to have no
+// locations reports a miss instead of serving what an earlier generation held.
+// Covers the bug where a volume pod restarts, the vid map holds an empty array,
+// and lookups fall back to the stale locations from before the restart.
 func TestGetLocationsEmptyArrayNoFallback(t *testing.T) {
-	// Setup: Create vidMap with cache
-	currentMap := newVidMap("")
+	vm := newVidMap("", DefaultVidMapCacheSize)
 	vid := uint32(10)
 	oldLocation := Location{Url: "10.131.1.28:8081"}
 	newLocation := Location{Url: "10.131.1.65:8081"}

-	// Scenario: Volume initially has old location
-	currentMap.addLocation(vid, oldLocation)
-	locs, found := currentMap.GetLocations(vid)
+	// Volume initially has its old location, which is then retained across a reset
+	vm.addLocation(vid, oldLocation)
+	vm.reset()
+
+	locs, found := vm.GetLocations(vid)
 	if !found || len(locs) != 1 || locs[0].Url != oldLocation.Url {
 		t.Fatalf("Expected to find old location, got found=%v locs=%v", found, locs)
 	}

-	// Create cache chain with old location
-	cachedMap := newVidMap("")
-	cachedMap.addLocation(vid, oldLocation)
-	currentMap.cache.Store(cachedMap)
+	// Volume server restarts and the old location is deleted
+	vm.deleteLocation(vid, oldLocation)

-	// Simulate: Volume server restarts, old location is deleted
-	currentMap.deleteLocation(vid, oldLocation)
-
-	// BUG: At this point vid2Locations[vid] = [] (empty array, key exists)
-	// OLD BEHAVIOR: GetLocations would see found=true, len=0 and fall back to cache
-	// returning stale oldLocation
-	// NEW BEHAVIOR: GetLocations should return nil, false (no locations available)
-
-	locs, found = currentMap.GetLocations(vid)
+	locs, found = vm.GetLocations(vid)
 	if found {
 		t.Errorf("Expected found=false for empty location array, got found=true with locs=%v", locs)
 	}
 	if locs != nil {
-		t.Errorf("Expected nil locations for empty array, got %v (should not fall back to stale cache!)", locs)
+		t.Errorf("Expected nil locations for empty array, got %v (should not serve stale history!)", locs)
 	}

-	// Verify: When new location is added, it should be returned (not stale cache)
-	currentMap.addLocation(vid, newLocation)
-	locs, found = currentMap.GetLocations(vid)
+	// When the new location is added, it should be returned (not the stale one)
+	vm.addLocation(vid, newLocation)
+	locs, found = vm.GetLocations(vid)
 	if !found || len(locs) != 1 {
 		t.Fatalf("Expected to find new location, got found=%v locs=%v", found, locs)
 	}
 	if locs[0].Url != newLocation.Url {
-		t.Errorf("Expected new location %s, got %s (got stale cache!)", newLocation.Url, locs[0].Url)
+		t.Errorf("Expected new location %s, got %s (got stale history!)", newLocation.Url, locs[0].Url)
 	}
 }

-// TestGetLocationsUnknownVolumeUsesCache tests that truly unknown volumes still use cache
-func TestGetLocationsUnknownVolumeUsesCache(t *testing.T) {
-	// Setup: Current map doesn't know about volume, but cache does
-	currentMap := newVidMap("")
-	cachedMap := newVidMap("")
+// TestGetLocationsRetainedAcrossReset tests that a volume the new master has
+// not mentioned yet is still served from what the previous one told us.
+func TestGetLocationsRetainedAcrossReset(t *testing.T) {
+	vm := newVidMap("", DefaultVidMapCacheSize)
 	vid := uint32(99)
-	cachedLocation := Location{Url: "cache-server:8081"}
+	retained := Location{Url: "cache-server:8081"}

-	cachedMap.addLocation(vid, cachedLocation)
-	currentMap.cache.Store(cachedMap)
+	vm.addLocation(vid, retained)
+	vm.reset()

-	// Volume 99 is completely unknown to currentMap (not in vid2Locations)
-	// This should fall back to cache
-	locs, found := currentMap.GetLocations(vid)
+	locs, found := vm.GetLocations(vid)
 	if !found || len(locs) != 1 {
-		t.Fatalf("Expected to find cached location for unknown volume, got found=%v locs=%v", found, locs)
+		t.Fatalf("Expected to find retained location after reset, got found=%v locs=%v", found, locs)
 	}
-	if locs[0].Url != cachedLocation.Url {
-		t.Errorf("Expected cached location %s, got %s", cachedLocation.Url, locs[0].Url)
+	if locs[0].Url != retained.Url {
+		t.Errorf("Expected retained location %s, got %s", retained.Url, locs[0].Url)
 	}
 }