From c4c4d227a17ebd9d7dec879ee1a693df5559d9cb Mon Sep 17 00:00:00 2001
From: chrislu <chris.lu@gmail.com>
Date: Thu, 30 Oct 2025 20:44:33 -0700
Subject: [PATCH] Further reduce lock contention in LookupVolumeIdsWithFallback

Optimized two loops that were holding RLock for extended periods:

Before:
- Held RLock during entire loop iteration
- Included string parsing and cache lookups
- Could block resetVidMap for significant time with large batches

After:
- Grab vidMap pointer with brief RLock
- Release lock immediately
- Perform all loop operations on local pointer

Impact:
- First loop: Cache check on initial volumeIds
- Second loop: Double-check after singleflight wait

Benefits:
- Minimal lock hold time (just pointer copy)
- resetVidMap no longer blocked by long loops
- Better concurrent performance with large volume ID lists
- Still thread-safe (vidMap methods have internal locks)

Verified with: go test -race ./weed/wdclient/... (passes)
---
 weed/wdclient/masterclient.go | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/weed/wdclient/masterclient.go b/weed/wdclient/masterclient.go
index 74828eb2a..c46910935 100644
--- a/weed/wdclient/masterclient.go
+++ b/weed/wdclient/masterclient.go
@@ -128,23 +128,26 @@ func (mc *MasterClient) 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
 	mc.vidMapLock.RLock()
+	vm := mc.vidMap
+	mc.vidMapLock.RUnlock()
+	
 	for _, vidString := range volumeIds {
 		vid, err := strconv.ParseUint(vidString, 10, 32)
 		if err != nil {
-			mc.vidMapLock.RUnlock()
 			return nil, fmt.Errorf("invalid volume id %s: %v", vidString, err)
 		}
 		vidStringToUint[vidString] = uint32(vid)
 
-		locations, found := mc.vidMap.GetLocations(uint32(vid))
+		locations, found := vm.GetLocations(uint32(vid))
 		if found && len(locations) > 0 {
 			result[vidString] = locations
 		} else {
 			needsLookup = append(needsLookup, vidString)
 		}
 	}
-	mc.vidMapLock.RUnlock()
 
 	if len(needsLookup) == 0 {
 		return result, nil
@@ -160,16 +163,19 @@ func (mc *MasterClient) LookupVolumeIdsWithFallback(ctx context.Context, volumeI
 		stillNeedLookup := make([]string, 0, len(needsLookup))
 		batchResult := make(map[string][]Location)
 
+		// Get stable pointer with minimal lock hold time
 		mc.vidMapLock.RLock()
+		vm := mc.vidMap
+		mc.vidMapLock.RUnlock()
+		
 		for _, vidString := range needsLookup {
 			vid := vidStringToUint[vidString] // Use pre-parsed value
-			if locations, found := mc.vidMap.GetLocations(vid); found && len(locations) > 0 {
+			if locations, found := vm.GetLocations(vid); found && len(locations) > 0 {
 				batchResult[vidString] = locations
 			} else {
 				stillNeedLookup = append(stillNeedLookup, vidString)
 			}
 		}
-		mc.vidMapLock.RUnlock()
 
 		if len(stillNeedLookup) == 0 {
 			return batchResult, nil