From 7065b32e5dc228a62a87a36eb556182fe7531e23 Mon Sep 17 00:00:00 2001 From: bruce-zzz Date: Wed, 2 Sep 2026 18:17:12 +0800 Subject: [PATCH] wdclient: prefer local volume replicas over remote-tier replicas on lookup LookupFileIdWithFallback (and the publicUrl variant in FilerClient) didn't honor the DataInRemote flag when shuffling URLs, so the DataInRemote patch only took effect in LookupVolumeServerUrl. Apply the same ReorderToFront(localUrls) to sameDcUrls/otherDcUrls so non-remote replicas stay at the front, matching the existing vidMap convention. --- weed/wdclient/filer_client.go | 11 ++++ weed/wdclient/vidmap_client.go | 13 ++++ .../wdclient/vidmap_client_localfirst_test.go | 65 +++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 weed/wdclient/vidmap_client_localfirst_test.go diff --git a/weed/wdclient/filer_client.go b/weed/wdclient/filer_client.go index 65237fb3c..08418a99f 100644 --- a/weed/wdclient/filer_client.go +++ b/weed/wdclient/filer_client.go @@ -468,6 +468,7 @@ func (fc *FilerClient) GetLookupFileIdFunction() LookupFileIdFunctionType { // Build URLs with publicUrl preference, and also prefer same DC var sameDcUrls, otherDcUrls []string + localUrls := make(map[string]bool) dataCenter := fc.GetDataCenter() for _, loc := range locations { url := loc.PublicUrl @@ -475,6 +476,10 @@ func (fc *FilerClient) GetLookupFileIdFunction() LookupFileIdFunctionType { url = loc.Url } httpUrl := "http://" + url + "/" + fileId + glog.V(4).Infof("lookup %s => %s, data in remote storage tier: %v", fileId, url, loc.DataInRemote) + if !loc.DataInRemote { + localUrls[httpUrl] = true + } if dataCenter != "" && dataCenter == loc.DataCenter { sameDcUrls = append(sameDcUrls, httpUrl) } else { @@ -484,6 +489,12 @@ func (fc *FilerClient) GetLookupFileIdFunction() LookupFileIdFunctionType { // Shuffle to distribute load across volume servers rand.Shuffle(len(sameDcUrls), func(i, j int) { sameDcUrls[i], sameDcUrls[j] = sameDcUrls[j], sameDcUrls[i] }) rand.Shuffle(len(otherDcUrls), func(i, j int) { otherDcUrls[i], otherDcUrls[j] = otherDcUrls[j], otherDcUrls[i] }) + // Keep local volumes in front so remote-tier replicas are only used as fallback, + // mirroring vidMap.LookupVolumeServerUrl + if len(localUrls) > 0 { + sameDcUrls = util.ReorderToFront(localUrls, sameDcUrls) + otherDcUrls = util.ReorderToFront(localUrls, otherDcUrls) + } // Prefer same data center fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil diff --git a/weed/wdclient/vidmap_client.go b/weed/wdclient/vidmap_client.go index f2fc9add9..94c8c082b 100644 --- a/weed/wdclient/vidmap_client.go +++ b/weed/wdclient/vidmap_client.go @@ -13,6 +13,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb" + "github.com/seaweedfs/seaweedfs/weed/util" ) // VolumeLocationProvider is the interface for looking up volume locations @@ -90,8 +91,13 @@ func (vc *vidMapClient) LookupFileIdWithFallback(ctx context.Context, fileId str // Build HTTP URLs from locations, preferring same data center var sameDcUrls, otherDcUrls []string + localUrls := make(map[string]bool) for _, loc := range locations { httpUrl := "http://" + loc.Url + "/" + fileId + glog.V(4).Infof("lookup %s => %s, data in remote storage tier: %v", fileId, loc.Url, loc.DataInRemote) + if !loc.DataInRemote { + localUrls[httpUrl] = true + } if dataCenter != "" && dataCenter == loc.DataCenter { sameDcUrls = append(sameDcUrls, httpUrl) } else { @@ -103,6 +109,13 @@ func (vc *vidMapClient) LookupFileIdWithFallback(ctx context.Context, fileId str rand.Shuffle(len(sameDcUrls), func(i, j int) { sameDcUrls[i], sameDcUrls[j] = sameDcUrls[j], sameDcUrls[i] }) rand.Shuffle(len(otherDcUrls), func(i, j int) { otherDcUrls[i], otherDcUrls[j] = otherDcUrls[j], otherDcUrls[i] }) + // Keep local volumes in front so remote-tier replicas are only used as fallback, + // mirroring vidMap.LookupVolumeServerUrl + if len(localUrls) > 0 { + sameDcUrls = util.ReorderToFront(localUrls, sameDcUrls) + otherDcUrls = util.ReorderToFront(localUrls, otherDcUrls) + } + // Prefer same data center fullUrls = append(sameDcUrls, otherDcUrls...) return fullUrls, nil diff --git a/weed/wdclient/vidmap_client_localfirst_test.go b/weed/wdclient/vidmap_client_localfirst_test.go new file mode 100644 index 000000000..976f9f449 --- /dev/null +++ b/weed/wdclient/vidmap_client_localfirst_test.go @@ -0,0 +1,65 @@ +package wdclient + +import ( + "context" + "strings" + "testing" +) + +type testLocationProvider struct { + locations map[string][]Location +} + +func (p *testLocationProvider) LookupVolumeIds(ctx context.Context, volumeIds []string) (map[string][]Location, error) { + result := make(map[string][]Location) + for _, vid := range volumeIds { + if locs, found := p.locations[vid]; found { + result[vid] = locs + } + } + return result, nil +} + +// TestLookupFileIdWithFallbackLocalFirst ensures volumes whose data is still +// local are tried before remote-tier replicas, on the provider (cache miss) path. +func TestLookupFileIdWithFallbackLocalFirst(t *testing.T) { + vc := newVidMapClient(&testLocationProvider{ + locations: map[string][]Location{ + "5": { + {Url: "10.0.0.1:8080", DataInRemote: true}, + {Url: "10.0.0.2:8080", DataInRemote: false}, + }, + }, + }, "", 5) + + urls, err := vc.LookupFileIdWithFallback(context.Background(), "5,abcdef0123456789") + if err != nil { + t.Fatalf("lookup failed: %v", err) + } + if len(urls) != 2 { + t.Fatalf("expected 2 urls, got %v", urls) + } + if !strings.Contains(urls[0], "10.0.0.2:8080") { + t.Errorf("expected local replica first, got %v", urls) + } +} + +// TestLookupFileIdWithFallbackAllRemote keeps shuffled order when every replica is remote. +func TestLookupFileIdWithFallbackAllRemote(t *testing.T) { + vc := newVidMapClient(&testLocationProvider{ + locations: map[string][]Location{ + "6": { + {Url: "10.0.0.1:8080", DataInRemote: true}, + {Url: "10.0.0.2:8080", DataInRemote: true}, + }, + }, + }, "", 5) + + urls, err := vc.LookupFileIdWithFallback(context.Background(), "6,abcdef0123456789") + if err != nil { + t.Fatalf("lookup failed: %v", err) + } + if len(urls) != 2 { + t.Fatalf("expected 2 urls, got %v", urls) + } +}