mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-22 07:06:51 +00:00
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
279 lines
7.7 KiB
Go
279 lines
7.7 KiB
Go
package wdclient
|
|
|
|
import (
|
|
"testing"
|
|
)
|
|
|
|
// TestInvalidateCacheValidFileId tests cache invalidation with a valid file ID
|
|
func TestInvalidateCacheValidFileId(t *testing.T) {
|
|
// Create a simple vidMapClient (can use nil provider for this test)
|
|
vc := &vidMapClient{
|
|
vidMap: newVidMap(""),
|
|
vidMapCacheSize: 5,
|
|
}
|
|
|
|
// Add some locations to the cache
|
|
vid := uint32(456)
|
|
vc.vidMap.Lock()
|
|
vc.vidMap.vid2Locations[vid] = []Location{{Url: "http://server1:8080"}}
|
|
vc.vidMap.Unlock()
|
|
|
|
// Verify location exists
|
|
vc.vidMap.RLock()
|
|
_, found := vc.vidMap.vid2Locations[vid]
|
|
vc.vidMap.RUnlock()
|
|
|
|
if !found {
|
|
t.Fatal("Location should exist before invalidation")
|
|
}
|
|
|
|
// Call InvalidateCache with a properly formatted file ID
|
|
fileId := "456,abcdef123456"
|
|
vc.InvalidateCache(fileId)
|
|
|
|
// Verify the locations were removed
|
|
vc.vidMap.RLock()
|
|
_, foundAfter := vc.vidMap.vid2Locations[vid]
|
|
vc.vidMap.RUnlock()
|
|
|
|
if foundAfter {
|
|
t.Errorf("Expected locations for vid %d to be removed after InvalidateCache", vid)
|
|
}
|
|
}
|
|
|
|
// TestInvalidateCacheInvalidFileId tests cache invalidation with invalid file IDs
|
|
func TestInvalidateCacheInvalidFileId(t *testing.T) {
|
|
testCases := []struct {
|
|
name string
|
|
fileId string
|
|
}{
|
|
{"empty file ID", ""},
|
|
{"no comma separator", "12345"},
|
|
{"non-numeric vid", "abc,defg"},
|
|
{"negative vid", "-1,abcd"},
|
|
{"oversized vid", "999999999999999999999,abcd"},
|
|
}
|
|
|
|
for _, tc := range testCases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
vc := &vidMapClient{
|
|
vidMap: newVidMap(""),
|
|
vidMapCacheSize: 5,
|
|
}
|
|
|
|
// 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()
|
|
|
|
// 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 {
|
|
t.Errorf("InvalidateCache with invalid fileId '%s' should not affect other entries", tc.fileId)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// TestInvalidateCacheWithHistory tests that invalidation propagates through cache history
|
|
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,
|
|
}
|
|
|
|
// Verify both have the vid before invalidation
|
|
vm2.RLock()
|
|
_, foundInCurrent := vm2.vid2Locations[vid]
|
|
vm2.RUnlock()
|
|
|
|
vm1.RLock()
|
|
_, foundInCache := vm1.vid2Locations[vid]
|
|
vm1.RUnlock()
|
|
|
|
if !foundInCurrent || !foundInCache {
|
|
t.Fatal("Both maps should have the vid before invalidation")
|
|
}
|
|
|
|
// Invalidate the cache
|
|
fileId := "789,xyz123"
|
|
vc.InvalidateCache(fileId)
|
|
|
|
// 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)")
|
|
}
|
|
}
|
|
|
|
// TestDeleteVidRecursion tests the deleteVid method removes from history chain
|
|
func TestDeleteVidRecursion(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()
|
|
|
|
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")
|
|
}
|
|
|
|
// Delete from vm3 (should cascade)
|
|
vm3.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 found2After {
|
|
t.Error("Expected vid to be removed from vm2 (cascaded)")
|
|
}
|
|
if found1After {
|
|
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)
|
|
}
|
|
}
|