mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-15 18:48:32 +02:00
fix(ec): blanket-clean every destination over the full shard range (#9512)
* fix(ec): blanket-clean every destination over the full shard range The previous cleanup pass walked t.sources only, with the shard ids the topology had reported at detection time. In the wild, a destination can end up with EC shards mounted that the topology snapshot didn't list — shards on a sibling disk that hadn't heartbeated, or shards left over from a concurrent attempt's mount step. FindEcVolume still returns true, so the next ReceiveFile trips the mounted-volume guard. Cleanup now unions t.sources (with ShardIds) and t.targets and issues unmount + delete over [0..totalShards-1] on each. Both RPCs are idempotent on missing shards, so the wider sweep is free. Two new tests cover the gap: shards mounted beyond what t.sources lists, and a target-only destination with no source row. * log(ec): include disk_id in EC unmount/delete/refusal log lines The current logs identify the volume and shard but leave disk_id off, which makes the cross-server cleanup story hard to follow when multiple disks of one server hold pieces of the same volume: UnmountEcShards 4121.1 -> add disk_id ec volume video-recordings_4121 shard delete [1 5] -> add per-loc disk_id volume server X:Y deletes ec shards from 4121 [...] -> add disk_id ReceiveFile: ec volume 4121 is mounted; refusing... -> add disk_ids ReceiveFile's refusal now names the disk_ids actually holding the mount so operators can see whether the next cleanup pass needs to target a sibling disk. Added Store.FindEcVolumeDiskIds / Store::find_ec_volume_disk_ids as the supporting primitive. Mirrored in seaweed-volume/src/ (unmount log in Store::unmount_ec_shard, heartbeat delete log in diff_ec_shard_delta_messages, refusal in the ReceiveFile handler). * test(ec): stub VolumeEcShardsUnmount/Delete on the fake volume server The plugin-worker EC tests boot a fake volume server that embeds UnimplementedVolumeServerServer. After the worker started calling VolumeEcShardsUnmount + VolumeEcShardsDelete pre-distribute, the default Unimplemented response surfaced as fourteen "method not implemented" errors and TestErasureCodingExecutionEncodesShards failed. Both RPCs are no-ops here — nothing on the fake server has mounted state or persisted shard files to remove.
This commit is contained in:
1 parent
bf9110ebd3
commit
2a41e76101
10 files changed
+255
-42
No files matched your search
@@ -691,54 +691,49 @@ func (t *ErasureCodingTask) getReplicas() []string {
|
||||
return replicas
|
||||
}
|
||||
|
||||
// cleanupStaleEcShards unmounts and deletes partial EC shards still mounted
|
||||
// on destinations from a previous failed encode. Safe by ordering: runs
|
||||
// after the source .dat is in the worker's workdir and a full local shard
|
||||
// set is generated. Per-destination errors are aggregated, not short-circuited.
|
||||
// cleanupStaleEcShards unmounts and deletes any EC shards still mounted on
|
||||
// destinations from a previous failed encode of this volume. Targets every
|
||||
// node we plan to write to (t.targets) plus every node detection saw EC
|
||||
// shards on (t.sources with ShardIds set), and issues the cleanup over the
|
||||
// full shard range so a stale topology snapshot — or shards landed by a
|
||||
// prior attempt that haven't heartbeated yet — cannot leave the
|
||||
// mounted-volume guard tripped during distributeEcShards. Safe by ordering:
|
||||
// runs after the source .dat is in the worker's workdir and a full local
|
||||
// shard set is generated. Per-destination errors are aggregated, not
|
||||
// short-circuited.
|
||||
func (t *ErasureCodingTask) cleanupStaleEcShards(ctx context.Context) error {
|
||||
if len(t.sources) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Union shard ids per destination node — volume-server cleanup walks
|
||||
// every DiskLocation, so per-disk source rows collapse to one RPC.
|
||||
perNode := make(map[string]map[uint32]struct{})
|
||||
nodes := make(map[string]struct{})
|
||||
for _, source := range t.sources {
|
||||
if source == nil || len(source.ShardIds) == 0 {
|
||||
if source == nil || source.Node == "" || len(source.ShardIds) == 0 {
|
||||
continue
|
||||
}
|
||||
shardSet, ok := perNode[source.Node]
|
||||
if !ok {
|
||||
shardSet = make(map[uint32]struct{})
|
||||
perNode[source.Node] = shardSet
|
||||
}
|
||||
for _, shardID := range source.ShardIds {
|
||||
shardSet[shardID] = struct{}{}
|
||||
}
|
||||
nodes[source.Node] = struct{}{}
|
||||
}
|
||||
if len(perNode) == 0 {
|
||||
for _, target := range t.targets {
|
||||
if target == nil || target.Node == "" {
|
||||
continue
|
||||
}
|
||||
nodes[target.Node] = struct{}{}
|
||||
}
|
||||
if len(nodes) == 0 {
|
||||
return nil
|
||||
}
|
||||
|
||||
var cleanupErrors []string
|
||||
for node, shardSet := range perNode {
|
||||
shardIds := make([]uint32, 0, len(shardSet))
|
||||
for id := range shardSet {
|
||||
shardIds = append(shardIds, id)
|
||||
}
|
||||
allShards := fullShardIdRange(t.dataShards, t.parityShards)
|
||||
|
||||
var cleanupErrors []string
|
||||
for node := range nodes {
|
||||
t.GetLogger().WithFields(map[string]interface{}{
|
||||
"volume_id": t.volumeID,
|
||||
"destination": node,
|
||||
"shard_ids": shardIds,
|
||||
"shard_ids": allShards,
|
||||
}).Info("Clearing stale EC shards on destination before re-distribute")
|
||||
|
||||
if err := unmountAndDeleteEcShards(ctx, t.grpcDialOption, node, t.volumeID, t.collection, shardIds); err != nil {
|
||||
if err := unmountAndDeleteEcShards(ctx, t.grpcDialOption, node, t.volumeID, t.collection, allShards); err != nil {
|
||||
cleanupErrors = append(cleanupErrors, fmt.Sprintf("%s: %v", node, err))
|
||||
t.GetLogger().WithFields(map[string]interface{}{
|
||||
"volume_id": t.volumeID,
|
||||
"destination": node,
|
||||
"shard_ids": shardIds,
|
||||
"error": err.Error(),
|
||||
}).Error("Failed to clear stale EC shards on destination")
|
||||
}
|
||||
@@ -751,6 +746,24 @@ func (t *ErasureCodingTask) cleanupStaleEcShards(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// fullShardIdRange builds [0..total-1] for unmount/delete RPCs. Falls back
|
||||
// to erasure_coding.TotalShardsCount when the task's ratio is unset (early
|
||||
// callers, tests); the helper never returns an empty slice.
|
||||
func fullShardIdRange(dataShards, parityShards int32) []uint32 {
|
||||
total := int(dataShards + parityShards)
|
||||
if total <= 0 {
|
||||
total = erasure_coding.TotalShardsCount
|
||||
}
|
||||
if total > erasure_coding.MaxShardCount {
|
||||
total = erasure_coding.MaxShardCount
|
||||
}
|
||||
ids := make([]uint32, total)
|
||||
for i := range ids {
|
||||
ids[i] = uint32(i)
|
||||
}
|
||||
return ids
|
||||
}
|
||||
|
||||
// unmountAndDeleteEcShards unmounts then deletes the named shards on one
|
||||
// destination. Unmount must precede delete (delete requires the shard be
|
||||
// unmounted); both RPCs are idempotent against missing shards.
|
||||
|
||||
@@ -106,6 +106,144 @@ func TestCleanupStaleEcShardsBeforeDistribute(t *testing.T) {
|
||||
"EC-shard sources must not appear in replica delete list")
|
||||
}
|
||||
|
||||
// The destination has more shards mounted than t.sources lists — simulates
|
||||
// detection-time topology missing shards (e.g., a prior attempt's mount
|
||||
// hadn't heartbeated yet, or different shards live on a sibling disk). The
|
||||
// cleanup must clear FindEcVolume regardless, by issuing unmount/delete
|
||||
// over the full shard range, so the next ReceiveFile lands.
|
||||
func TestCleanupStaleEcShardsClearsShardsBeyondSources(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test in short mode")
|
||||
}
|
||||
|
||||
clusterHarness := framework.StartVolumeCluster(t, matrix.P1())
|
||||
conn, grpcClient := framework.DialVolumeServer(t, clusterHarness.VolumeGRPCAddress())
|
||||
defer conn.Close()
|
||||
|
||||
const (
|
||||
volumeID = uint32(94780)
|
||||
collection = "ec-9478-beyond"
|
||||
)
|
||||
|
||||
framework.AllocateVolume(t, grpcClient, volumeID, collection)
|
||||
|
||||
httpClient := framework.NewHTTPClient()
|
||||
fid := framework.NewFileID(volumeID, 9478001, 0x9478FACE)
|
||||
upResp := framework.UploadBytes(t, httpClient, clusterHarness.VolumeAdminURL(), fid,
|
||||
[]byte("payload-for-shards-beyond-sources"))
|
||||
_ = framework.ReadAllAndClose(t, upResp)
|
||||
require.Equal(t, http.StatusCreated, upResp.StatusCode)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
|
||||
defer cancel()
|
||||
|
||||
_, err := grpcClient.VolumeEcShardsGenerate(ctx, &volume_server_pb.VolumeEcShardsGenerateRequest{
|
||||
VolumeId: volumeID, Collection: collection,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Five shards mounted on the destination.
|
||||
mountedShards := []uint32{0, 1, 2, 3, 4}
|
||||
_, err = grpcClient.VolumeEcShardsMount(ctx, &volume_server_pb.VolumeEcShardsMountRequest{
|
||||
VolumeId: volumeID, Collection: collection,
|
||||
ShardIds: mountedShards,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
// Detection only "saw" two of them — the rest must still get cleared.
|
||||
knownToDetection := []uint32{0, 1}
|
||||
|
||||
task := NewErasureCodingTask(
|
||||
"stale-ec-beyond-sources",
|
||||
clusterHarness.VolumeServerAddress(),
|
||||
volumeID,
|
||||
collection,
|
||||
grpc.WithTransportCredentials(insecure.NewCredentials()),
|
||||
)
|
||||
task.dataShards = erasure_coding.DataShardsCount
|
||||
task.parityShards = erasure_coding.ParityShardsCount
|
||||
task.sources = []*worker_pb.TaskSource{
|
||||
{
|
||||
Node: clusterHarness.VolumeServerAddress(),
|
||||
VolumeId: volumeID,
|
||||
ShardIds: knownToDetection,
|
||||
},
|
||||
}
|
||||
|
||||
require.NoError(t, task.cleanupStaleEcShards(ctx))
|
||||
|
||||
_, infoErr := grpcClient.VolumeEcShardsInfo(ctx, &volume_server_pb.VolumeEcShardsInfoRequest{VolumeId: volumeID})
|
||||
require.Error(t, infoErr,
|
||||
"all mounted shards must be cleared even though detection only listed a subset")
|
||||
|
||||
shardPath := makeTinyEcShardFile(t)
|
||||
require.NoError(t,
|
||||
sendShardViaReceiveFile(ctx, grpcClient, volumeID, collection, 4, shardPath),
|
||||
"ReceiveFile for a shard outside detection's snapshot must succeed after cleanup")
|
||||
}
|
||||
|
||||
// Cleanup also targets fresh destinations from t.targets even when no
|
||||
// EC-shard source row exists for them. This catches concurrent-attempt
|
||||
// fallout where a previous worker mounted shards on a node we're now
|
||||
// writing to but the topology snapshot is stale.
|
||||
func TestCleanupStaleEcShardsCoversTargetsWithoutSources(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping integration test in short mode")
|
||||
}
|
||||
|
||||
clusterHarness := framework.StartVolumeCluster(t, matrix.P1())
|
||||
conn, grpcClient := framework.DialVolumeServer(t, clusterHarness.VolumeGRPCAddress())
|
||||
defer conn.Close()
|
||||
|
||||
const (
|
||||
volumeID = uint32(94781)
|
||||
collection = "ec-9478-targets-only"
|
||||
)
|
||||
|
||||
framework.AllocateVolume(t, grpcClient, volumeID, collection)
|
||||
|
||||
httpClient := framework.NewHTTPClient()
|
||||
fid := framework.NewFileID(volumeID, 9478101, 0x947811CE)
|
||||
upResp := framework.UploadBytes(t, httpClient, clusterHarness.VolumeAdminURL(), fid,
|
||||
[]byte("payload-for-targets-only-cleanup"))
|
||||
_ = framework.ReadAllAndClose(t, upResp)
|
||||
require.Equal(t, http.StatusCreated, upResp.StatusCode)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
|
||||
defer cancel()
|
||||
|
||||
_, err := grpcClient.VolumeEcShardsGenerate(ctx, &volume_server_pb.VolumeEcShardsGenerateRequest{
|
||||
VolumeId: volumeID, Collection: collection,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
_, err = grpcClient.VolumeEcShardsMount(ctx, &volume_server_pb.VolumeEcShardsMountRequest{
|
||||
VolumeId: volumeID, Collection: collection,
|
||||
ShardIds: []uint32{0, 1},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
task := NewErasureCodingTask(
|
||||
"stale-ec-targets-only",
|
||||
clusterHarness.VolumeServerAddress(),
|
||||
volumeID,
|
||||
collection,
|
||||
grpc.WithTransportCredentials(insecure.NewCredentials()),
|
||||
)
|
||||
task.dataShards = erasure_coding.DataShardsCount
|
||||
task.parityShards = erasure_coding.ParityShardsCount
|
||||
// No sources. The destination is named only as a target — the cleanup
|
||||
// must still reach it.
|
||||
task.targets = []*worker_pb.TaskTarget{
|
||||
{Node: clusterHarness.VolumeServerAddress(), VolumeId: volumeID, ShardIds: []uint32{0}},
|
||||
}
|
||||
|
||||
require.NoError(t, task.cleanupStaleEcShards(ctx))
|
||||
|
||||
_, infoErr := grpcClient.VolumeEcShardsInfo(ctx, &volume_server_pb.VolumeEcShardsInfoRequest{VolumeId: volumeID})
|
||||
require.Error(t, infoErr,
|
||||
"target-only destinations must also be cleaned even without a source row")
|
||||
}
|
||||
|
||||
// Cleanup is a no-op when sources carry only the regular .dat replica.
|
||||
func TestCleanupStaleEcShardsSkipsRegularReplicas(t *testing.T) {
|
||||
if testing.Short() {
|
||||
|
||||
Reference in new issue
Block a user