From 5c910ad54634ebd44b6e34e791ed86eeceef0f53 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Sun, 8 Nov 2026 15:05:41 -0800 Subject: [PATCH] topology: list collections without nesting the collection map read lock (#12495) * topology: list collections without nesting the collection map read lock Items() already takes the read lock and copies the values. Holding it around the call made ListCollections take the read lock twice, so a DeleteCollection waiting for the write lock in between blocked the second read lock and wedged the master. Co-Authored-By: Claude Opus 5.5 * topology: test ListCollections against a concurrent collection delete Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- weed/topology/topology.go | 2 -- weed/topology/topology_test.go | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/weed/topology/topology.go b/weed/topology/topology.go index 0d393bdfd9..81db2ff59a 100644 --- a/weed/topology/topology.go +++ b/weed/topology/topology.go @@ -813,11 +813,9 @@ func (t *Topology) ListCollections(includeNormalVolumes, includeEcVolumes bool) found := make(map[string]bool) if includeNormalVolumes { - t.collectionMap.RLock() for _, c := range t.collectionMap.Items() { found[c.(*Collection).Name] = true } - t.collectionMap.RUnlock() } if includeEcVolumes { diff --git a/weed/topology/topology_test.go b/weed/topology/topology_test.go index 681d9f282d..4c3e34b62e 100644 --- a/weed/topology/topology_test.go +++ b/weed/topology/topology_test.go @@ -641,6 +641,40 @@ func TestListCollections(t *testing.T) { } } +// A collection delete that queues for the write lock while ListCollections +// reads the collections must not wedge both. +func TestListCollectionsWithConcurrentCollectionDelete(t *testing.T) { + rp, _ := super_block.NewReplicaPlacementFromString("000") + topo := NewTopology("weedfs", sequence.NewMemorySequencer(), 32*1024, 5, false) + + stop := make(chan struct{}) + defer close(stop) + go func() { + for { + select { + case <-stop: + return + default: + } + topo.GetVolumeLayout("churn", rp, needle.EMPTY_TTL, types.HardDriveType) + topo.DeleteCollection("churn") + } + }() + + listed := make(chan struct{}) + go func() { + defer close(listed) + for i := 0; i < 100000; i++ { + topo.ListCollections(true, false) + } + }() + select { + case <-listed: + case <-time.After(10 * time.Second): + t.Fatal("ListCollections deadlocked with a concurrent DeleteCollection") + } +} + func TestDataNodeIdBasedIdentification(t *testing.T) { topo := NewTopology("weedfs", sequence.NewMemorySequencer(), 32*1024, 5, false) dc := topo.GetOrCreateDataCenter("dc1")