diff options
| author | chrislu <chris.lu@gmail.com> | 2025-10-30 20:44:33 -0700 |
|---|---|---|
| committer | chrislu <chris.lu@gmail.com> | 2025-10-30 20:44:33 -0700 |
| commit | c4c4d227a17ebd9d7dec879ee1a693df5559d9cb (patch) | |
| tree | 241fc77902d960b8b0cb0a7d788c23b2ff164193 | |
| parent | 60f9dea60a8b6d892d35ee94a77bfbc19c0fe402 (diff) | |
| download | seaweedfs-c4c4d227a17ebd9d7dec879ee1a693df5559d9cb.tar.xz seaweedfs-c4c4d227a17ebd9d7dec879ee1a693df5559d9cb.zip | |
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)
| -rw-r--r-- | weed/wdclient/masterclient.go | 16 |
1 files 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 |
