From 498922dbb2f7f2ac02bd695367b459fe1fe57a8e Mon Sep 17 00:00:00 2001 From: chrislu Date: Thu, 30 Oct 2025 21:03:13 -0700 Subject: [PATCH] Optimize locking: Reduce lock acquisitions and use helper methods Two optimizations to further reduce lock contention and improve code consistency: 1. LookupFileIdWithFallback: Eliminated redundant lock acquisition - Before: Two separate locks to get vidMap and dataCenter - After: Single lock gets both values together - Benefit: 50% reduction in lock/unlock overhead for this hot path 2. KeepConnected: Use GetDataCenter() helper for consistency - Before: Manual lock/unlock to access DataCenter field - After: Use existing GetDataCenter() helper method - Benefit: Better encapsulation and code consistency Impact: - Reduced lock contention in high-traffic lookup path - More consistent use of accessor methods throughout codebase - Cleaner, more maintainable code Verified with: go test -race ./weed/wdclient/... (passes) --- weed/wdclient/masterclient.go | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/weed/wdclient/masterclient.go b/weed/wdclient/masterclient.go index c883576f1..dd9b8b8c7 100644 --- a/weed/wdclient/masterclient.go +++ b/weed/wdclient/masterclient.go @@ -71,9 +71,10 @@ func (mc *MasterClient) GetLookupFileIdFunction() LookupFileIdFunctionType { } func (mc *MasterClient) LookupFileIdWithFallback(ctx context.Context, fileId string) (fullUrls []string, err error) { - // Try cache first using the fast path - need to protect vidMap pointer access + // Try cache first using the fast path - grab both vidMap and dataCenter in one lock mc.vidMapLock.RLock() vm := mc.vidMap + dataCenter := vm.DataCenter mc.vidMapLock.RUnlock() fullUrls, err = vm.LookupFileId(ctx, fileId) @@ -100,10 +101,6 @@ func (mc *MasterClient) LookupFileIdWithFallback(ctx context.Context, fileId str } // Build HTTP URLs from locations, preferring same data center - mc.vidMapLock.RLock() - dataCenter := mc.vidMap.DataCenter - mc.vidMapLock.RUnlock() - var sameDcUrls, otherDcUrls []string for _, loc := range locations { httpUrl := "http://" + loc.Url + "/" + fileId @@ -368,13 +365,9 @@ func (mc *MasterClient) tryConnectToMaster(ctx context.Context, master pb.Server return err } - mc.vidMapLock.RLock() - dataCenter := mc.vidMap.DataCenter - mc.vidMapLock.RUnlock() - if err = stream.Send(&master_pb.KeepConnectedRequest{ FilerGroup: mc.FilerGroup, - DataCenter: dataCenter, + DataCenter: mc.GetDataCenter(), Rack: mc.rack, ClientType: mc.clientType, ClientAddress: string(mc.clientHost),