diff --git a/weed/topology/volume_layout.go b/weed/topology/volume_layout.go index 7c6fd71f3..3200879e5 100644 --- a/weed/topology/volume_layout.go +++ b/weed/topology/volume_layout.go @@ -206,6 +206,7 @@ func (vl *VolumeLayout) UnRegisterVolume(v *storage.VolumeInfo, dn *DataNode) { if location.Length() == 0 { delete(vl.vid2location, v.Id) + vl.removeFromCrowded(v.Id) } } @@ -400,7 +401,6 @@ func (vl *VolumeLayout) removeFromWritable(vid needle.VolumeId) bool { break } } - vl.removeFromCrowded(vid) if toDeleteIndex >= 0 { glog.V(0).Infoln("Volume", vid, "becomes unwritable") vl.writables = append(vl.writables[0:toDeleteIndex], vl.writables[toDeleteIndex+1:]...) diff --git a/weed/topology/volume_layout_test.go b/weed/topology/volume_layout_test.go index b5646fb13..999c8de8e 100644 --- a/weed/topology/volume_layout_test.go +++ b/weed/topology/volume_layout_test.go @@ -3,8 +3,10 @@ package topology import ( "testing" + "github.com/seaweedfs/seaweedfs/weed/storage" "github.com/seaweedfs/seaweedfs/weed/storage/needle" "github.com/seaweedfs/seaweedfs/weed/storage/super_block" + "github.com/seaweedfs/seaweedfs/weed/storage/types" ) func TestVolumesBinaryState(t *testing.T) { @@ -114,3 +116,75 @@ func TestVolumesBinaryState(t *testing.T) { }) } } + +func TestVolumeLayoutCrowdedState(t *testing.T) { + rp, _ := super_block.NewReplicaPlacementFromString("000") + ttl, _ := needle.ReadTTL("") + diskType := types.HardDriveType + + vl := NewVolumeLayout(rp, ttl, diskType, 1024*1024*1024, false) + + vid := needle.VolumeId(1) + dn := &DataNode{ + NodeImpl: NodeImpl{ + id: "test-node", + }, + Ip: "127.0.0.1", + Port: 8080, + } + + // Create a volume info + volumeInfo := &storage.VolumeInfo{ + Id: vid, + ReplicaPlacement: rp, + Ttl: ttl, + DiskType: string(diskType), + } + + // Register the volume + vl.RegisterVolume(volumeInfo, dn) + + // Add the volume to writables + vl.accessLock.Lock() + vl.setVolumeWritable(vid) + vl.accessLock.Unlock() + + // Mark the volume as crowded + vl.SetVolumeCrowded(vid) + + t.Run("should be crowded after being marked", func(t *testing.T) { + vl.accessLock.RLock() + _, isCrowded := vl.crowded[vid] + vl.accessLock.RUnlock() + if !isCrowded { + t.Fatal("Volume should be marked as crowded after SetVolumeCrowded") + } + }) + + // Remove from writable (simulating temporary unwritable state) + vl.accessLock.Lock() + vl.removeFromWritable(vid) + vl.accessLock.Unlock() + + t.Run("should remain crowded after becoming unwritable", func(t *testing.T) { + // This is the fix for issue #6712 - crowded state should persist + vl.accessLock.RLock() + _, stillCrowded := vl.crowded[vid] + vl.accessLock.RUnlock() + if !stillCrowded { + t.Fatal("Volume should remain crowded after becoming unwritable (fix for issue #6712)") + } + }) + + // Now unregister the volume completely + vl.UnRegisterVolume(volumeInfo, dn) + + t.Run("should not be crowded after unregistering", func(t *testing.T) { + vl.accessLock.RLock() + _, stillCrowdedAfterUnregister := vl.crowded[vid] + vl.accessLock.RUnlock() + if stillCrowdedAfterUnregister { + t.Fatal("Volume should be removed from crowded map after full unregistration") + } + }) +}