Fix stale cache fallback for empty volume locations in wdclient (#10081)

fix(wdclient): prevent stale cache fallback for empty volume locations

## Problem
During Kubernetes pod restarts, volume servers temporarily disconnect and their
locations are removed from vidMap. The deleteLocation function leaves an empty
array [] in vid2Locations map instead of removing the key entirely.

GetLocations() was checking 'if found && len(locations) > 0', which would fail
for empty arrays and fall back to the cache chain, returning STALE locations
from before the restart. This caused S3 gateway to try connecting to old pod
IPs that no longer exist, resulting in connection timeouts and hanging registry
sync jobs.

Example timeline:
1. Volume pod at 10.131.1.28:8081 registers volumes 10,12
2. S3 gateway caches: vid2Locations[10] = [10.131.1.28:8081]
3. Pod restarts, gets new IP 10.131.1.65:8081
4. Master sends delete → vid2Locations[10] = [] (empty, but key exists)
5. BUG: GetLocations(10) sees found=true, len=0 → falls back to cache
6. Returns stale 10.131.1.28:8081 instead of waiting for new location
7. S3 requests timeout trying to reach unreachable old IP

## Solution
Distinguish between two cases:
- found=true, locations=[] : Volume explicitly has no locations (e.g. restart)
  → Return nil, false (no fallback to cache)
- found=false : Volume never seen in current map
  → Check cache (preserve cache benefits for unknown volumes)

An empty array explicitly means 'this volume currently has no locations',
which is semantically different from 'volume unknown'. Don't fall back to
stale cache for explicitly empty volumes.

## Testing
Added comprehensive tests:
- TestGetLocationsEmptyArrayNoFallback: Verifies empty arrays don't use cache
- TestGetLocationsUnknownVolumeUsesCache: Verifies unknown volumes still use cache
- All existing tests pass

## Impact
Fixes registry sync job hangs during SeaweedFS upgrades/restarts. S3 gateway
will now correctly wait for updated volume locations instead of using stale
cached IPs.

Related: OutSystems.SeaWeedfs Helm chart, vega cluster incident 2026-06-24
This commit is contained in:
os-pradipbabar
2026-06-24 16:31:32 -07:00
committed by GitHub
parent 089acfbf36
commit d1b1338558
2 changed files with 82 additions and 2 deletions
+11 -2
View File
@@ -130,10 +130,19 @@ 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)
locations, found = vc.getLocations(vid)
if found && len(locations) > 0 {
return locations, found
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
}
// Volume exists but has no locations - return empty, don't check cache
return nil, false
}
// Volume not found in current map - check cache for unknown volumes
if cachedMap := vc.cache.Load(); cachedMap != nil {
return cachedMap.GetLocations(vid)
}
+71
View File
@@ -205,3 +205,74 @@ func TestDeleteVidRecursion(t *testing.T) {
t.Error("Expected vid to be removed from vm1 (cascaded)")
}
}
// 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.
func TestGetLocationsEmptyArrayNoFallback(t *testing.T) {
// Setup: Create vidMap with cache
currentMap := newVidMap("")
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)
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)
// 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)
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)
}
// Verify: When new location is added, it should be returned (not stale cache)
currentMap.addLocation(vid, newLocation)
locs, found = currentMap.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)
}
}
// 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("")
vid := uint32(99)
cachedLocation := Location{Url: "cache-server:8081"}
cachedMap.addLocation(vid, cachedLocation)
currentMap.cache.Store(cachedMap)
// Volume 99 is completely unknown to currentMap (not in vid2Locations)
// This should fall back to cache
locs, found := currentMap.GetLocations(vid)
if !found || len(locs) != 1 {
t.Fatalf("Expected to find cached location for unknown volume, 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)
}
}