mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-04 11:07:35 +01:00
Guard collection deletion against duplicate volume IDs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
1 parent
75a6a34528
commit
0ffb50887a
5 files changed
+204
No files matched your search
@@ -2,12 +2,14 @@ package weed_server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
|
||||
"github.com/seaweedfs/raft"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/operation"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/master_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/pb/volume_server_pb"
|
||||
"github.com/seaweedfs/seaweedfs/weed/topology"
|
||||
)
|
||||
|
||||
func (ms *MasterServer) CollectionList(ctx context.Context, req *master_pb.CollectionListRequest) (*master_pb.CollectionListResponse, error) {
|
||||
@@ -35,6 +37,10 @@ func (ms *MasterServer) CollectionDelete(ctx context.Context, req *master_pb.Col
|
||||
|
||||
resp := &master_pb.CollectionDeleteResponse{}
|
||||
|
||||
if err := ms.ensureCollectionDeleteSafe(req.Name); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
err := ms.doDeleteNormalCollection(req.Name)
|
||||
|
||||
if err != nil {
|
||||
@@ -50,6 +56,19 @@ func (ms *MasterServer) CollectionDelete(ctx context.Context, req *master_pb.Col
|
||||
return resp, nil
|
||||
}
|
||||
|
||||
func (ms *MasterServer) ensureCollectionDeleteSafe(collectionName string) error {
|
||||
duplicates := ms.Topo.FindDuplicateVolumeIds(collectionName)
|
||||
if len(duplicates) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
return fmt.Errorf(
|
||||
"refusing to delete collection %q: duplicate volume IDs exist across collections (%s). Run `volume.check.duplicates` to inspect and resolve them first",
|
||||
collectionName,
|
||||
topology.FormatDuplicateVolumeIds(duplicates),
|
||||
)
|
||||
}
|
||||
|
||||
func (ms *MasterServer) doDeleteNormalCollection(collectionName string) error {
|
||||
|
||||
collection, ok := ms.Topo.FindCollection(collectionName)
|
||||
|
||||
@@ -4,6 +4,11 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/seaweedfs/seaweedfs/weed/cluster"
|
||||
"github.com/seaweedfs/seaweedfs/weed/sequence"
|
||||
"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/topology"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
@@ -35,3 +40,41 @@ func TestInitialLockRingUpdateSkipsNonFilers(t *testing.T) {
|
||||
|
||||
assert.Nil(t, ms.initialLockRingUpdate(cluster.BrokerType, "group-a"))
|
||||
}
|
||||
|
||||
func TestEnsureCollectionDeleteSafeRejectsDuplicateVolumeIds(t *testing.T) {
|
||||
topo := topology.NewTopology("weedfs", sequence.NewMemorySequencer(), 32*1024, 5, false)
|
||||
dc := topo.GetOrCreateDataCenter("dc1")
|
||||
rack := dc.GetOrCreateRack("rack1")
|
||||
maxVolumeCounts := map[string]uint32{"": 25}
|
||||
|
||||
dn1 := rack.GetOrCreateDataNode("127.0.0.1", 34534, 0, "127.0.0.1", "", maxVolumeCounts)
|
||||
dn2 := rack.GetOrCreateDataNode("127.0.0.2", 34535, 0, "127.0.0.2", "", maxVolumeCounts)
|
||||
|
||||
replicaPlacement := &super_block.ReplicaPlacement{}
|
||||
collectionA := storage.VolumeInfo{
|
||||
Id: needle.VolumeId(100),
|
||||
Collection: "collection-a",
|
||||
Version: needle.GetCurrentVersion(),
|
||||
ReplicaPlacement: replicaPlacement,
|
||||
Ttl: needle.EMPTY_TTL,
|
||||
}
|
||||
collectionB := storage.VolumeInfo{
|
||||
Id: needle.VolumeId(100),
|
||||
Collection: "collection-b",
|
||||
Version: needle.GetCurrentVersion(),
|
||||
ReplicaPlacement: replicaPlacement,
|
||||
Ttl: needle.EMPTY_TTL,
|
||||
}
|
||||
|
||||
dn1.UpdateVolumes([]storage.VolumeInfo{collectionA})
|
||||
dn2.UpdateVolumes([]storage.VolumeInfo{collectionB})
|
||||
topo.RegisterVolumeLayout(collectionA, dn1)
|
||||
topo.RegisterVolumeLayout(collectionB, dn2)
|
||||
|
||||
ms := &MasterServer{Topo: topo}
|
||||
|
||||
err := ms.ensureCollectionDeleteSafe("collection-a")
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "refusing to delete collection")
|
||||
assert.Contains(t, err.Error(), "100:[collection-a, collection-b]")
|
||||
}
|
||||
@@ -30,6 +30,10 @@ func (ms *MasterServer) collectionDeleteHandler(w http.ResponseWriter, r *http.R
|
||||
writeJsonError(w, r, http.StatusBadRequest, fmt.Errorf("collection %s does not exist", collectionName))
|
||||
return
|
||||
}
|
||||
if err := ms.ensureCollectionDeleteSafe(collectionName); err != nil {
|
||||
writeJsonError(w, r, http.StatusConflict, err)
|
||||
return
|
||||
}
|
||||
for _, server := range collection.ListVolumeServers() {
|
||||
err := operation.WithVolumeServerClient(false, server.ServerAddress(), ms.grpcDialOption, func(client volume_server_pb.VolumeServerClient) error {
|
||||
_, deleteErr := client.DeleteCollection(context.Background(), &volume_server_pb.DeleteCollectionRequest{
|
||||
|
||||
Reference in new issue
Block a user