master: repair the lookup index after a vacuum that raced a disconnect (#10226)

* master: re-create a vacuum-committed volume the index has lost

SetVolumeAvailable dereferenced vid2location[vid] with no nil check. A
disconnect during a long vacuum can drop a single-replica volume from the
lookup index while it stays on the node; the commit then panics the master
instead of re-registering it. Re-create the entry, and seed its size tracking
so assigns are counted right away rather than after the next heartbeat.

* master: re-register a vacuumed volume the index lost on mark-writable

The maintenance worker re-enables a volume by marking it writable after the
vacuum commit, but VolumeMarkReadonly only updated nodes the lookup index
already knew. If a disconnect race dropped the volume from the index during the
vacuum, that path was a no-op and the volume stayed "not found" until the next
full heartbeat healed it. Re-register it from the node that still holds it.

weed/server/master_grpc_server_volume.go

@@ -15,6 +15,7 @@ import (
 	"github.com/seaweedfs/raft"
 
 	"github.com/seaweedfs/seaweedfs/weed/glog"
+	"github.com/seaweedfs/seaweedfs/weed/pb"
 	"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
 	"github.com/seaweedfs/seaweedfs/weed/security"
 	"github.com/seaweedfs/seaweedfs/weed/storage/needle"
@@ -323,18 +324,33 @@ func (ms *MasterServer) VolumeMarkReadonly(ctx context.Context, req *master_pb.V
 
 	replicaPlacement, _ := super_block.NewReplicaPlacementFromByte(byte(req.ReplicaPlacement))
 	vl := ms.Topo.GetVolumeLayout(req.Collection, replicaPlacement, needle.LoadTTLFromUint32(req.Ttl), types.ToDiskType(req.DiskType))
-	dataNodes := ms.Topo.Lookup(req.Collection, needle.VolumeId(req.VolumeId))
+	vid := needle.VolumeId(req.VolumeId)
+	dataNodes := ms.Topo.Lookup(req.Collection, vid)
 
+	found := false
 	for _, dn := range dataNodes {
 		if dn.Ip == req.Ip && dn.Port == int(req.Port) {
+			found = true
 			if req.IsReadonly {
-				vid := needle.VolumeId(req.VolumeId)
 				vl.SetVolumeReadOnly(dn, vid)
 				if pending := vl.GetPendingSize(vid); pending > 0 {
 					glog.V(0).Infof("volume %d marked readonly with %d pending bytes", vid, pending)
 				}
 			} else {
-				vl.SetVolumeWritable(dn, needle.VolumeId(req.VolumeId))
+				vl.SetVolumeWritable(dn, vid)
+			}
+		}
+	}
+
+	// A vacuum worker marks the volume writable after a successful commit. If the
+	// index lost it meanwhile (e.g. a disconnect race during the vacuum), the loop
+	// found nothing to update; re-register it from the node that still holds it so
+	// LookupVolume stops returning "not found".
+	if !found && !req.IsReadonly {
+		if dn := ms.Topo.LookupDataNodeByAddress(pb.NewServerAddress(req.Ip, int(req.Port), 0)); dn != nil {
+			if vi, err := dn.GetVolumesById(vid); err == nil {
+				ms.Topo.RegisterVolumeLayout(vi, dn)
+				glog.V(0).Infof("volume %d re-registered from %s:%d, was missing from the lookup index at mark-writable", vid, req.Ip, req.Port)
 			}
 		}
 	}

weed/topology/topology_test.go

@@ -626,3 +626,54 @@ func TestSyncDataNodeRegistrationReRegistersMissingVolume(t *testing.T) {
 		t.Fatalf("after self-heal: lookup %d got %v, want 1 location", vid, got)
 	}
 }
+
+// TestSetVolumeAvailableRepairsMissingVolume covers the vacuum-commit variant of
+// the same divergence. A disconnect during a long vacuum can drop a
+// single-replica volume from the lookup index while it stays on the node; the
+// commit then calls SetVolumeAvailable on it. That used to dereference a nil
+// location and panic; it now re-creates the entry and repairs the split.
+func TestSetVolumeAvailableRepairsMissingVolume(t *testing.T) {
+	topo := NewTopology("weedfs", sequence.NewMemorySequencer(), 32*1024, 5, false)
+
+	dc := topo.GetOrCreateDataCenter("dc1")
+	rack := dc.GetOrCreateRack("rack1")
+	dn := rack.GetOrCreateDataNode("127.0.0.1", 34534, 0, "127.0.0.1", "", map[string]uint32{"": 25})
+
+	vid := needle.VolumeId(2640)
+	volumeMessage := &master_pb.VolumeInformationMessage{
+		Id:               uint32(vid),
+		Size:             100,
+		Collection:       "drr",
+		ReplicaPlacement: uint32(0),
+		Version:          uint32(needle.GetCurrentVersion()),
+		Ttl:              0,
+	}
+
+	topo.SyncDataNodeRegistration([]*master_pb.VolumeInformationMessage{volumeMessage}, dn)
+
+	rp, _ := super_block.NewReplicaPlacementFromString("000")
+	vl := topo.GetVolumeLayout("drr", rp, needle.EMPTY_TTL, types.HardDriveType)
+
+	// Disconnect drops the volume from the index but leaves it on the node.
+	vl.SetVolumeUnavailable(dn, vid)
+	if got := topo.Lookup("", vid); got != nil {
+		t.Fatalf("after SetVolumeUnavailable: expected lookup miss, got %v", got)
+	}
+	if _, err := dn.GetVolumesById(vid); err != nil {
+		t.Fatalf("volume %d should still be in the data node disk map: %v", vid, err)
+	}
+
+	// The vacuum commit re-marks the volume available; it must re-register it.
+	vl.SetVolumeAvailable(dn, vid, false, false)
+	if got := topo.Lookup("", vid); len(got) != 1 {
+		t.Fatalf("after SetVolumeAvailable: lookup %d got %v, want 1 location", vid, got)
+	}
+	// Size tracking must be seeded too, or assigns go uncounted until the next
+	// heartbeat and the volume can overfill.
+	vl.accessLock.RLock()
+	_, tracked := vl.sizeTracking[vid]
+	vl.accessLock.RUnlock()
+	if !tracked {
+		t.Fatalf("after SetVolumeAvailable: size tracking for %d not seeded", vid)
+	}
+}

weed/topology/volume_layout.go

@@ -164,24 +164,37 @@ func (vl *VolumeLayout) String() string {
 	return fmt.Sprintf("rp:%v, ttl:%v, writables:%v, volumeSizeLimit:%v", vl.rp, vl.ttl, vl.writables, vl.volumeSizeLimit)
 }
 
+// getOrCreateLocationList returns the vid's location list, creating an empty one
+// if the index has none. Callers hold accessLock.
+func (vl *VolumeLayout) getOrCreateLocationList(vid needle.VolumeId) *VolumeLocationList {
+	location, ok := vl.vid2location[vid]
+	if !ok {
+		location = NewVolumeLocationList()
+		vl.vid2location[vid] = location
+	}
+	return location
+}
+
+// initSizeTracking seeds a vid's size tracking from a reported size if it has
+// none yet. Callers hold accessLock.
+func (vl *VolumeLayout) initSizeTracking(vid needle.VolumeId, size uint64, compactRevision uint32) {
+	if _, exists := vl.sizeTracking[vid]; !exists {
+		vl.sizeTracking[vid] = &volumeSizeTracking{
+			effectiveSize:   size,
+			reportedSize:    size,
+			compactRevision: compactRevision,
+		}
+	}
+}
+
 func (vl *VolumeLayout) RegisterVolume(v *storage.VolumeInfo, dn *DataNode) {
 	vl.accessLock.Lock()
 	defer vl.accessLock.Unlock()
 
 	defer vl.rememberOversizedVolume(v, dn)
 
-	if _, ok := vl.vid2location[v.Id]; !ok {
-		vl.vid2location[v.Id] = NewVolumeLocationList()
-	}
-	vl.vid2location[v.Id].Set(dn)
-	// For new volumes, initialize size tracking from reported size.
-	if _, exists := vl.sizeTracking[v.Id]; !exists {
-		vl.sizeTracking[v.Id] = &volumeSizeTracking{
-			effectiveSize:   v.Size,
-			reportedSize:    v.Size,
-			compactRevision: v.CompactRevision,
-		}
-	}
+	vl.getOrCreateLocationList(v.Id).Set(dn)
+	vl.initSizeTracking(v.Id, v.Size, v.CompactRevision)
 	// glog.V(4).Infof("volume %d added to %s len %d copy %d", v.Id, dn.Id(), vl.vid2location[v.Id].Length(), v.ReplicaPlacement.GetCopyCount())
 	for _, dn := range vl.vid2location[v.Id].list {
 		if vInfo, err := dn.GetVolumesById(v.Id); err == nil {
@@ -886,7 +899,11 @@ func (vl *VolumeLayout) SetVolumeAvailable(dn *DataNode, vid needle.VolumeId, is
 		return false
 	}
 
-	vl.vid2location[vid].Set(dn)
+	// A disconnect during a long vacuum can drop the entry while the volume is
+	// still on the node; re-create it (and seed size tracking) instead of
+	// dereferencing a nil location, so the commit also repairs the split.
+	vl.getOrCreateLocationList(vid).Set(dn)
+	vl.initSizeTracking(vid, vInfo.Size, vInfo.CompactRevision)
 
 	if vInfo.ReadOnly || isReadOnly || isFullCapacity {
 		return false