fix(ec): bring ec.encode worker and EC/volume helpers to parity with shell (#9599)

* refactor(volume): extract replica sync/select into shared volume_replica package

Move the volume replica reconciliation helpers (status, union builder,
SyncAndSelectBestReplica, ReadNeedleMeta) out of the shell into a new
weed/storage/volume_replica package so both the shell (ec.encode, volume.tier.move,
volume.check.disk) and the EC encode worker can reuse them. No behavior change.

* fix(ec): bring ec.encode worker to parity with the shell

- Sync replicas and encode the most-complete one (via the shared
  volume_replica.SyncAndSelectBestReplica) instead of a possibly-stale replica,
  marking all replicas readonly first. Prevents silent data loss when a stale
  replica is encoded and the originals deleted.
- Skip remote/tiered volumes in detection (shell ec.encode excludes them).
- Min-node safety gate: refuse to encode when cluster nodes < parity shards.
- Align default thresholds with the shell (fullness 0.95, quiet 1h).

* fix(vacuum): plugin path honors min_volume_age_seconds override

deriveVacuumConfig hard-coded MinVolumeAgeSeconds=0, dropping any configured
value. Read it from worker config (default 0, matching the shell/master vacuum
which has no age gate) so an explicit override is honored.

* address review feedback

- config.go: align GetConfigSpec schema defaults (quiet_for_seconds=3600,
  fullness_ratio=0.95) with the runtime defaults so UI/bootstrap flows match the
  shell (coderabbitai).
- ec_task.go: roll back readonly when markReplicasReadonly fails partway, so
  already-marked replicas don't stay readonly (coderabbitai).
- volume_replica: pass the caller's replica statuses into buildUnionReplica instead
  of re-fetching them, and skip the per-needle ReadNeedleMeta RPC when the source
  replica is read-only (gemini-code-assist).

* test(plugin_workers/ec): make fixtures eligible under the new defaults

The default EC encode thresholds were raised to match the shell (fullness 0.95,
quiet 1h), but the plugin-worker integration fixtures still used 90%-full /
10-minute-old volumes, so detection found no eligible volumes and the tests failed
in CI. Bump the eligible fixtures to 96% full and 2h old.
This commit is contained in:
Chris Lu authored and GitHub committed 2026-05-21 02:16:28 -07:00
1 parent 3f6410fdc3
commit cd15ae1395
15 files changed
+238 -134

No files matched your search

+5 -5
View File
@@ -27,8 +27,8 @@ func NewDefaultConfig() *Config {
ScanIntervalSeconds: 60 * 60, // 1 hour
MaxConcurrent: 1,
},
QuietForSeconds: 300, // 5 minutes
FullnessRatio: 0.8, // 80%
QuietForSeconds: 3600, // 1 hour, matching the shell ec.encode -quietFor default
FullnessRatio: 0.95, // 95%, matching the shell ec.encode -fullPercent default
CollectionFilter: "",
MinSizeMB: 30, // 30MB (more reasonable than 100MB)
PreferredTags: nil,
@@ -87,14 +87,14 @@ func GetConfigSpec() base.ConfigSpec {
Name: "quiet_for_seconds",
JSONName: "quiet_for_seconds",
Type: config.FieldTypeInterval,
DefaultValue: 300,
DefaultValue: 3600,
MinValue: 1,
MaxValue: 3600,
Required: true,
DisplayName: "Quiet Period",
Description: "Minimum time volume must be quiet before erasure coding",
HelpText: "Volume must not be modified for this duration before erasure coding",
Placeholder: "5",
Placeholder: "60",
Unit: config.UnitMinutes,
InputType: "interval",
CSSClasses: "form-control",
@@ -103,7 +103,7 @@ func GetConfigSpec() base.ConfigSpec {
Name: "fullness_ratio",
JSONName: "fullness_ratio",
Type: config.FieldTypeFloat,
DefaultValue: 0.8,
DefaultValue: 0.95,
MinValue: 0.0001,
MaxValue: 1.0,
Required: true,
+43 -2
View File
@@ -52,12 +52,19 @@ func Detection(ctx context.Context, metrics []*types.VolumeHealthMetrics, cluste
skippedCollectionFilter := 0
skippedQuietTime := 0
skippedFullness := 0
skippedRemote := 0
skippedTooFewNodes := 0
consecutivePlanningFailures := 0
var planner *ecPlacementPlanner
allowedCollections := wildcard.CompileWildcardMatchers(ecConfig.CollectionFilter)
// Cluster node count for the min-node safety gate (mirrors the shell ec.encode
// guard that refuses to encode when nodes < parity shards, so shards cannot be
// spread for fault tolerance).
clusterNodeCount := countTopologyNodes(clusterInfo.ActiveTopology)
// Group metrics by VolumeID to handle replicas and select canonical server
volumeGroups := make(map[uint32][]*types.VolumeHealthMetrics)
for _, metric := range metrics {
@@ -163,6 +170,21 @@ func Detection(ctx context.Context, metrics []*types.VolumeHealthMetrics, cluste
continue
}
// Skip remote/tiered volumes: encoding them would lose the tiering. The
// shell ec.encode excludes remote volumes for the same reason.
if metric.HasRemoteCopy {
skippedRemote++
continue
}
// Min-node safety gate: don't encode when the cluster has fewer nodes than
// this collection's parity shards — shards could not be spread to tolerate
// failures. Mirrors the shell ec.encode guard (node count < parity shards).
if clusterNodeCount > 0 && clusterNodeCount < erasure_coding.ParityShardsCount {
skippedTooFewNodes++
continue
}
// Check quiet duration and fullness criteria
if metric.Age >= quietThreshold && metric.FullnessRatio >= ecConfig.FullnessRatio {
if ctx != nil {
@@ -373,8 +395,8 @@ func Detection(ctx context.Context, metrics []*types.VolumeHealthMetrics, cluste
// Log debug summary if no tasks were created
if len(results) == 0 && len(metrics) > 0 && !stoppedEarly {
totalVolumes := len(metrics)
glog.V(1).Infof("EC detection: No tasks created for %d volumes (skipped: %d already EC, %d too small, %d filtered, %d not quiet, %d not full)",
totalVolumes, skippedAlreadyEC, skippedTooSmall, skippedCollectionFilter, skippedQuietTime, skippedFullness)
glog.V(1).Infof("EC detection: No tasks created for %d volumes (skipped: %d already EC, %d too small, %d filtered, %d not quiet, %d not full, %d remote, %d too few nodes)",
totalVolumes, skippedAlreadyEC, skippedTooSmall, skippedCollectionFilter, skippedQuietTime, skippedFullness, skippedRemote, skippedTooFewNodes)
// Show details for first few volumes
for i, metric := range metrics {
@@ -658,6 +680,25 @@ func (p *ecPlacementPlanner) buildCandidateSets(shardsNeeded int) [][]*placement
// planECDestinations plans the destinations for erasure coding operation.
// dataShards/parityShards are parameters so callers can drive non-10+4 ratios.
// countTopologyNodes counts volume-server nodes in the active topology, used by
// the min-node safety gate.
func countTopologyNodes(at *topology.ActiveTopology) int {
if at == nil {
return 0
}
topo := at.GetTopologyInfo()
if topo == nil {
return 0
}
n := 0
for _, dc := range topo.DataCenterInfos {
for _, rack := range dc.RackInfos {
n += len(rack.DataNodeInfos)
}
}
return n
}
func planECDestinations(planner *ecPlacementPlanner, metric *types.VolumeHealthMetrics, ecConfig *Config, dataShards, parityShards int) (*topology.MultiDestinationPlan, error) {
if planner == nil || planner.activeTopology == nil {
return nil, fmt.Errorf("active topology not available for EC placement")
@@ -233,13 +233,13 @@ func buildStuckSourceTopology(t *testing.T, volumeID uint32, presentShardCount i
// criteria (Age, FullnessRatio, Size), with `Age` derived from `LastModified`
// so the two fields stay consistent for any reader.
func buildStuckSourceMetrics(volumeID uint32, server string) []*types.VolumeHealthMetrics {
lastModified := time.Now().Add(-time.Hour)
lastModified := time.Now().Add(-2 * time.Hour)
return []*types.VolumeHealthMetrics{{
VolumeID: volumeID,
Server: server,
Size: 200 * 1024 * 1024,
Collection: "",
FullnessRatio: 0.9,
FullnessRatio: 0.96,
LastModified: lastModified,
Age: time.Since(lastModified),
}}
@@ -482,9 +482,9 @@ func buildVolumeMetricsForIDs(count int) []*types.VolumeHealthMetrics {
Server: "10.0.0.1:8080",
Size: 200 * 1024 * 1024,
Collection: "",
FullnessRatio: 0.9,
LastModified: now.Add(-time.Hour),
Age: 10 * time.Minute,
FullnessRatio: 0.96,
LastModified: now.Add(-2 * time.Hour),
Age: 2 * time.Hour,
})
}
return metrics
+96 -34
View File
@@ -1,6 +1,7 @@
package erasure_coding
import (
"bytes"
"context"
"fmt"
"io"
@@ -19,6 +20,8 @@ import (
"github.com/seaweedfs/seaweedfs/weed/storage/needle"
storagetypes "github.com/seaweedfs/seaweedfs/weed/storage/types"
"github.com/seaweedfs/seaweedfs/weed/storage/volume_info"
"github.com/seaweedfs/seaweedfs/weed/storage/volume_replica"
"github.com/seaweedfs/seaweedfs/weed/wdclient"
"github.com/seaweedfs/seaweedfs/weed/worker/types"
"github.com/seaweedfs/seaweedfs/weed/worker/types/base"
"google.golang.org/grpc"
@@ -35,12 +38,13 @@ type ErasureCodingTask struct {
grpcDialOption grpc.DialOption
// EC parameters
dataShards int32
parityShards int32
sourceDiskType string // source volume's disk type, forwarded to Mount RPC (#9423)
targets []*worker_pb.TaskTarget // Unified targets for EC shards
sources []*worker_pb.TaskSource // Unified sources for cleanup
shardAssignment map[string][]string // destination -> assigned shard types
dataShards int32
parityShards int32
sourceDiskType string // source volume's disk type, forwarded to Mount RPC (#9423)
targets []*worker_pb.TaskTarget // Unified targets for EC shards
sources []*worker_pb.TaskSource // Unified sources for cleanup
shardAssignment map[string][]string // destination -> assigned shard types
readonlyReplicas []pb.ServerAddress // replicas marked readonly, for rollback
}
// NewErasureCodingTask creates a new unified EC task instance
@@ -146,12 +150,22 @@ func (t *ErasureCodingTask) Execute(ctx context.Context, params *worker_pb.TaskP
}
}()
// Step 1: Mark volume readonly
// Step 1: Mark all replicas readonly, then reconcile them and select the most
// complete replica as the encode source. Encoding a stale replica and then
// deleting the originals would silently lose entries that exist only on another
// replica; SyncAndSelectBestReplica builds the union onto the best replica first
// (mirrors the shell ec.encode best-replica selection).
t.ReportProgressWithStage(10.0, "Marking volume readonly")
t.GetLogger().Info("Marking volume readonly")
if err := t.markVolumeReadonly(ctx); err != nil {
if err := t.markReplicasReadonly(ctx); err != nil {
// Marking can fail partway; restore the replicas already marked readonly.
t.rollbackReadonly(ctx)
return fmt.Errorf("failed to mark volume readonly: %v", err)
}
if err := t.syncAndSelectSourceReplica(); err != nil {
t.rollbackReadonly(ctx)
return fmt.Errorf("failed to sync and select source replica: %v", err)
}
// Step 2: Copy volume files to worker
// The .idx and .dat are copied as separate network transfers, with .idx
@@ -277,39 +291,87 @@ func (t *ErasureCodingTask) GetProgress() float64 {
// Helper methods for actual EC operations
// markVolumeReadonly marks the volume as readonly on the source server
func (t *ErasureCodingTask) markVolumeReadonly(ctx context.Context) error {
return operation.WithVolumeServerClient(false, pb.ServerAddress(t.server), t.grpcDialOption,
func(client volume_server_pb.VolumeServerClient) error {
_, err := client.VolumeMarkReadonly(ctx, &volume_server_pb.VolumeMarkReadonlyRequest{
VolumeId: t.volumeID,
})
return err
})
// replicaLocations returns the regular (non-EC) volume replica locations from the
// task sources. EC-shard sources carry shard ids; regular replicas do not. Falls
// back to the assigned source server when no replica sources are present.
func (t *ErasureCodingTask) replicaLocations() []wdclient.Location {
var locs []wdclient.Location
for _, s := range t.sources {
if s == nil || len(s.ShardIds) > 0 || s.Node == "" {
continue
}
locs = append(locs, wdclient.Location{Url: s.Node, DataCenter: s.DataCenter})
}
if len(locs) == 0 {
locs = append(locs, wdclient.Location{Url: t.server})
}
return locs
}
// rollbackReadonly is a best-effort rollback of markVolumeReadonly, used when the
// EC task fails before any shards are distributed. Logs but does not return errors.
// Uses a fresh context with timeout since the caller's ctx may already be cancelled.
// markReplicasReadonly marks every regular replica readonly so no writes land
// during encoding, recording them so rollbackReadonly can restore them all.
func (t *ErasureCodingTask) markReplicasReadonly(ctx context.Context) error {
t.readonlyReplicas = t.readonlyReplicas[:0]
for _, loc := range t.replicaLocations() {
addr := loc.ServerAddress()
err := operation.WithVolumeServerClient(false, addr, t.grpcDialOption,
func(client volume_server_pb.VolumeServerClient) error {
_, e := client.VolumeMarkReadonly(ctx, &volume_server_pb.VolumeMarkReadonlyRequest{VolumeId: t.volumeID})
return e
})
if err != nil {
return fmt.Errorf("mark volume %d readonly on %s: %w", t.volumeID, addr, err)
}
t.readonlyReplicas = append(t.readonlyReplicas, addr)
}
return nil
}
// syncAndSelectSourceReplica reconciles the volume's replicas (building the union
// of all live entries onto the most complete one) and switches the encode source
// to that replica, so a stale replica is never the basis of the encode.
func (t *ErasureCodingTask) syncAndSelectSourceReplica() error {
locs := t.replicaLocations()
if len(locs) <= 1 {
return nil // single replica: nothing to reconcile
}
var buf bytes.Buffer
best, err := volume_replica.SyncAndSelectBestReplica(t.grpcDialOption, needle.VolumeId(t.volumeID), t.collection, locs, "", &buf)
if out := strings.TrimSpace(buf.String()); out != "" {
glog.Infof("EC encode replica sync for volume %d:\n%s", t.volumeID, out)
}
if err != nil {
return err
}
if best.Url != "" && best.Url != t.server {
glog.Infof("EC encode: using best replica %s as source for volume %d (was %s)", best.Url, t.volumeID, t.server)
t.server = best.Url
}
return nil
}
// rollbackReadonly is a best-effort restore of every replica markReplicasReadonly
// touched, used when the EC task fails before the originals are deleted. Logs but
// does not return errors; uses a fresh context since the caller's may be cancelled.
func (t *ErasureCodingTask) rollbackReadonly(_ context.Context) {
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
if err := t.markVolumeWritable(ctx); err != nil {
glog.Warningf("failed to restore volume %d to writable after EC task failure: %v", t.volumeID, err)
} else {
glog.V(0).Infof("restored volume %d to writable after EC task failure", t.volumeID)
servers := t.readonlyReplicas
if len(servers) == 0 {
servers = []pb.ServerAddress{pb.ServerAddress(t.server)}
}
}
// markVolumeWritable restores the volume to writable on the source server.
func (t *ErasureCodingTask) markVolumeWritable(ctx context.Context) error {
return operation.WithVolumeServerClient(false, pb.ServerAddress(t.server), t.grpcDialOption,
func(client volume_server_pb.VolumeServerClient) error {
_, err := client.VolumeMarkWritable(ctx, &volume_server_pb.VolumeMarkWritableRequest{
VolumeId: t.volumeID,
for _, addr := range servers {
err := operation.WithVolumeServerClient(false, addr, t.grpcDialOption,
func(client volume_server_pb.VolumeServerClient) error {
_, e := client.VolumeMarkWritable(ctx, &volume_server_pb.VolumeMarkWritableRequest{VolumeId: t.volumeID})
return e
})
return err
})
if err != nil {
glog.Warningf("failed to restore volume %d to writable on %s after EC task failure: %v", t.volumeID, addr, err)
} else {
glog.V(0).Infof("restored volume %d to writable on %s after EC task failure", t.volumeID, addr)
}
}
}
// copyVolumeFilesToWorker copies .idx and .dat files from source server to local worker.
@@ -59,7 +59,7 @@ func TestCopyVolumeFilesToWorkerUsesCurrentCompactionRevision(t *testing.T) {
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
defer cancel()
require.NoError(t, task.markVolumeReadonly(ctx))
require.NoError(t, task.markReplicasReadonly(ctx))
fileStatus, err := task.readSourceVolumeFileStatus(ctx)
require.NoError(t, err)
@@ -143,10 +143,10 @@ func (h *ErasureCodingHandler) Descriptor() *plugin_pb.JobTypeDescriptor {
},
DefaultValues: map[string]*plugin_pb.ConfigValue{
"quiet_for_seconds": {
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 300},
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 3600},
},
"fullness_ratio": {
Kind: &plugin_pb.ConfigValue_DoubleValue{DoubleValue: 0.8},
Kind: &plugin_pb.ConfigValue_DoubleValue{DoubleValue: 0.95},
},
"min_size_mb": {
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 30},
@@ -170,10 +170,10 @@ func (h *ErasureCodingHandler) Descriptor() *plugin_pb.JobTypeDescriptor {
},
WorkerDefaultValues: map[string]*plugin_pb.ConfigValue{
"quiet_for_seconds": {
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 300},
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 3600},
},
"fullness_ratio": {
Kind: &plugin_pb.ConfigValue_DoubleValue{DoubleValue: 0.8},
Kind: &plugin_pb.ConfigValue_DoubleValue{DoubleValue: 0.95},
},
"min_size_mb": {
Kind: &plugin_pb.ConfigValue_Int64Value{Int64Value: 30},