diff --git a/weed/admin/dash/admin_server.go b/weed/admin/dash/admin_server.go index ba70b5e09..f22490392 100644 --- a/weed/admin/dash/admin_server.go +++ b/weed/admin/dash/admin_server.go @@ -261,23 +261,10 @@ func NewAdminServer(masters string, filerGroup string, templateFS http.FileSyste maintenanceConfig = maintenance.DefaultMaintenanceConfig() } - // Apply new defaults to handle schema changes (like enabling by default) - schema := maintenance.GetMaintenanceConfigSchema() - if err := schema.ApplyDefaultsToProtobuf(maintenanceConfig); err != nil { - glog.Warningf("Failed to apply schema defaults to loaded config: %v", err) - } - - // Force enable maintenance system for new default behavior - // This handles the case where old configs had Enabled=false as default - if !maintenanceConfig.Enabled { - glog.V(1).Infof("Enabling maintenance system (new default behavior)") - maintenanceConfig.Enabled = true - } - - glog.V(1).Infof("Maintenance system initialized with persistent configuration (enabled: %v)", maintenanceConfig.Enabled) + glog.V(1).Infof("Maintenance system initialized with persistent configuration (enabled: %v)", maintenanceConfig.GetEnabled()) } else { maintenanceConfig = maintenance.DefaultMaintenanceConfig() - glog.V(1).Infof("No data directory configured, maintenance system will run in memory-only mode (enabled: %v)", maintenanceConfig.Enabled) + glog.V(1).Infof("No data directory configured, maintenance system will run in memory-only mode (enabled: %v)", maintenanceConfig.GetEnabled()) } // Load saved task configurations from persistence. This has to run before the maintenance @@ -292,7 +279,7 @@ func NewAdminServer(masters string, filerGroup string, templateFS http.FileSyste server.InitMaintenanceManager(maintenanceConfig) // Start maintenance manager if enabled - if maintenanceConfig.Enabled { + if maintenanceConfig.GetEnabled() { go func() { // Give master client a bit of time to connect before starting scans time.Sleep(2 * time.Second) @@ -300,6 +287,8 @@ func NewAdminServer(masters string, filerGroup string, templateFS http.FileSyste glog.Errorf("Failed to start maintenance manager: %v", err) } }() + } else { + glog.V(0).Infof("Maintenance system is disabled by configuration, not starting the maintenance manager") } pluginOpts := adminplugin.Options{ @@ -1872,7 +1861,7 @@ func (s *AdminServer) InitMaintenanceManager(config *maintenance.MaintenanceConf } } - glog.V(1).Infof("Maintenance manager initialized (enabled: %v)", config.Enabled) + glog.V(1).Infof("Maintenance manager initialized (enabled: %v)", config.GetEnabled()) } // GetMaintenanceManager returns the maintenance manager diff --git a/weed/admin/dash/config_persistence.go b/weed/admin/dash/config_persistence.go index fe52abcab..a4ef90c38 100644 --- a/weed/admin/dash/config_persistence.go +++ b/weed/admin/dash/config_persistence.go @@ -159,6 +159,17 @@ func (cp *ConfigPersistence) LoadMaintenanceConfig() (*MaintenanceConfig, error) if configData, err := os.ReadFile(configPath); err == nil { var config MaintenanceConfig if err := proto.Unmarshal(configData, &config); err == nil { + // Fill in fields added to the schema after this file was written. The + // enabled flag tracks presence, so an explicitly persisted false survives + // this, while a file from before presence tracking (where an operator's + // explicit false and the field's absence look the same on the wire) keeps + // the enabled default rather than silently switching maintenance off. + if err := maintenance.GetMaintenanceConfigSchema().ApplyDefaultsToProtobuf(&config); err != nil { + glog.Warningf("Failed to apply schema defaults to loaded maintenance config: %v", err) + } + if config.Enabled == nil { + config.Enabled = proto.Bool(true) + } // Always populate policy from separate task configuration files config.Policy = cp.buildPolicyFromTaskConfigs() return &config, nil diff --git a/weed/admin/dash/config_toml.go b/weed/admin/dash/config_toml.go index 3872d5b9e..c4c12a853 100644 --- a/weed/admin/dash/config_toml.go +++ b/weed/admin/dash/config_toml.go @@ -12,6 +12,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/worker/tasks/base" "github.com/seaweedfs/seaweedfs/weed/worker/tasks/erasure_coding" "github.com/seaweedfs/seaweedfs/weed/worker/tasks/vacuum" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/timestamppb" ) @@ -30,6 +31,20 @@ type TomlConfig interface { // directory loss and override admin UI edits on restart. Absent keys keep // their persisted values. func (cp *ConfigPersistence) ApplyMaintenanceConfigFromToml(v TomlConfig) error { + var maintenanceConf *MaintenanceConfig + maintenanceChanged := false + if k := "maintenance.enabled"; v.IsSet(k) { + conf, err := cp.LoadMaintenanceConfig() + if err != nil { + return fmt.Errorf("load maintenance config: %w", err) + } + conf.Enabled = proto.Bool(v.GetBool(k)) + // the policy lives in the per-task config files; don't snapshot it here + conf.Policy = nil + maintenanceConf = conf + maintenanceChanged = true + } + vacuumConf := vacuum.LoadConfigFromPersistence(cp) vacuumChanged := applyBaseConfigFromToml(v, "maintenance.vacuum.", &vacuumConf.BaseConfig) if k := "maintenance.vacuum.garbage_threshold"; v.IsSet(k) { @@ -84,13 +99,19 @@ func (cp *ConfigPersistence) ApplyMaintenanceConfigFromToml(v TomlConfig) error ecChanged = true } - if !vacuumChanged && !balanceChanged && !ecChanged { + if !maintenanceChanged && !vacuumChanged && !balanceChanged && !ecChanged { return nil } if !cp.IsConfigured() { return fmt.Errorf("admin.toml maintenance settings require -dataDir to persist") } + if maintenanceChanged { + if err := cp.SaveMaintenanceConfig(maintenanceConf); err != nil { + return fmt.Errorf("save maintenance config: %w", err) + } + glog.V(0).Infof("Applied [maintenance] settings from admin.toml (enabled: %v)", maintenanceConf.GetEnabled()) + } if vacuumChanged { if err := cp.SaveVacuumTaskPolicy(vacuumConf.ToTaskPolicy()); err != nil { return fmt.Errorf("save vacuum task config: %w", err) diff --git a/weed/admin/dash/config_toml_test.go b/weed/admin/dash/config_toml_test.go index c68e31a68..111c3ccff 100644 --- a/weed/admin/dash/config_toml_test.go +++ b/weed/admin/dash/config_toml_test.go @@ -90,6 +90,36 @@ preferred_tags = "Fast, ssd" } } +func TestApplyMaintenanceConfigFromTomlEnabledToggle(t *testing.T) { + dir := t.TempDir() + cp := NewConfigPersistence(dir) + + if err := cp.ApplyMaintenanceConfigFromToml(tomlConfig(t, "[maintenance]\nenabled = false\n")); err != nil { + t.Fatalf("apply: %v", err) + } + conf, err := cp.LoadMaintenanceConfig() + if err != nil { + t.Fatalf("load maintenance config: %v", err) + } + if conf.GetEnabled() { + t.Errorf("maintenance still enabled after [maintenance] enabled = false") + } + if conf.ScanIntervalSeconds != 30*60 { + t.Errorf("scan interval = %d, want default 1800 kept alongside the toggle", conf.ScanIntervalSeconds) + } + + if err := cp.ApplyMaintenanceConfigFromToml(tomlConfig(t, "[maintenance]\nenabled = true\n")); err != nil { + t.Fatalf("apply: %v", err) + } + conf, err = cp.LoadMaintenanceConfig() + if err != nil { + t.Fatalf("load maintenance config: %v", err) + } + if !conf.GetEnabled() { + t.Errorf("maintenance still disabled after [maintenance] enabled = true") + } +} + func TestApplyMaintenanceConfigFromTomlNoKeys(t *testing.T) { dir := t.TempDir() cp := NewConfigPersistence(dir) diff --git a/weed/admin/dash/maintenance_startup_test.go b/weed/admin/dash/maintenance_startup_test.go index fd16fb99b..b0f300e02 100644 --- a/weed/admin/dash/maintenance_startup_test.go +++ b/weed/admin/dash/maintenance_startup_test.go @@ -8,6 +8,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/worker/tasks/balance" "github.com/seaweedfs/seaweedfs/weed/worker/tasks/vacuum" "github.com/seaweedfs/seaweedfs/weed/worker/types" + "google.golang.org/protobuf/proto" ) // The task definitions these tests configure are process-global, so put them back the way a @@ -95,6 +96,69 @@ func TestDisabledTaskIsNotScannedAfterStartup(t *testing.T) { } } +// TestLoadMaintenanceConfigKeepsPersistedEnabledFalse covers the top of issue-shaped startup: +// an operator persisted enabled=false, and the load path used to lose it twice over — once to +// ApplyDefaultsToProtobuf treating the bool zero value as unset, and once to an explicit +// force-enable "migration" block. The persisted flag must come back as saved, while fields the +// old file never carried still pick up their schema defaults. +func TestLoadMaintenanceConfigKeepsPersistedEnabledFalse(t *testing.T) { + dir := t.TempDir() + cp := NewConfigPersistence(dir) + + // A file that only knows about the enabled flag: everything else zero. + if err := cp.SaveMaintenanceConfig(&MaintenanceConfig{Enabled: proto.Bool(false)}); err != nil { + t.Fatalf("save maintenance config: %v", err) + } + + loaded, err := cp.LoadMaintenanceConfig() + if err != nil { + t.Fatalf("load maintenance config: %v", err) + } + if loaded.GetEnabled() { + t.Error("persisted enabled=false came back true; the maintenance system cannot be disabled") + } + if loaded.ScanIntervalSeconds != 30*60 { + t.Errorf("scan interval = %d, want schema default 1800 filled in", loaded.ScanIntervalSeconds) + } + + // And with no file at all, the default is enabled. + fresh, err := NewConfigPersistence(t.TempDir()).LoadMaintenanceConfig() + if err != nil { + t.Fatalf("load maintenance config: %v", err) + } + if !fresh.GetEnabled() { + t.Error("maintenance not enabled by default when nothing is persisted") + } +} + +// TestLoadMaintenanceConfigTreatsLegacyAbsentEnabledAsOn: a maintenance.pb written before +// enabled tracked presence carries no enabled field on the wire whether the old default left +// it false or an operator unchecked it — the two are indistinguishable. Such files must keep +// the enabled default rather than silently switching maintenance off on upgrade; only a file +// that explicitly persists the toggle may disable it. +func TestLoadMaintenanceConfigTreatsLegacyAbsentEnabledAsOn(t *testing.T) { + dir := t.TempDir() + cp := NewConfigPersistence(dir) + + // What an old writer produced for enabled=false plus a tuned scan interval: the bool is + // simply absent from the wire. + if err := cp.SaveMaintenanceConfig(&MaintenanceConfig{ScanIntervalSeconds: 15 * 60}); err != nil { + t.Fatalf("save maintenance config: %v", err) + } + + loaded, err := cp.LoadMaintenanceConfig() + if err != nil { + t.Fatalf("load maintenance config: %v", err) + } + if !loaded.GetEnabled() { + t.Error("legacy config without an enabled field loads as disabled; " + + "upgrading would silently switch the maintenance system off") + } + if loaded.ScanIntervalSeconds != 15*60 { + t.Errorf("scan interval = %d, want persisted 900 kept", loaded.ScanIntervalSeconds) + } +} + // TestPolicyMirrorsWhatTheDetectorsReport checks that the maintenance policy the queue and // the scanner run on agrees with the detectors. A disagreement means one of the two paths // into the task configs has gone stale again. diff --git a/weed/admin/maintenance/maintenance_config_proto.go b/weed/admin/maintenance/maintenance_config_proto.go index 4295a706f..64163f9ba 100644 --- a/weed/admin/maintenance/maintenance_config_proto.go +++ b/weed/admin/maintenance/maintenance_config_proto.go @@ -2,12 +2,13 @@ package maintenance import ( "github.com/seaweedfs/seaweedfs/weed/pb/worker_pb" + "google.golang.org/protobuf/proto" ) // DefaultMaintenanceConfigProto returns default configuration as protobuf func DefaultMaintenanceConfigProto() *worker_pb.MaintenanceConfig { return &worker_pb.MaintenanceConfig{ - Enabled: true, + Enabled: proto.Bool(true), ScanIntervalSeconds: 30 * 60, // 30 minutes WorkerTimeoutSeconds: 5 * 60, // 5 minutes TaskTimeoutSeconds: 2 * 60 * 60, // 2 hours diff --git a/weed/admin/maintenance/maintenance_manager.go b/weed/admin/maintenance/maintenance_manager.go index 6189fc687..32618fb1b 100644 --- a/weed/admin/maintenance/maintenance_manager.go +++ b/weed/admin/maintenance/maintenance_manager.go @@ -109,7 +109,7 @@ func NewMaintenanceManager(adminClient AdminClient, config *MaintenanceConfig, c // Start begins the maintenance manager func (mm *MaintenanceManager) Start() error { - if !mm.config.Enabled { + if !mm.config.GetEnabled() { glog.V(1).Infof("Maintenance system is disabled") return nil } diff --git a/weed/command/admin.go b/weed/command/admin.go index 098b97d5b..41bf74d91 100644 --- a/weed/command/admin.go +++ b/weed/command/admin.go @@ -177,8 +177,9 @@ var cmdAdmin = &Command{ - Example: weed admin -metricsPort=9327 -master="localhost:9333" Maintenance Configuration: - - An optional admin.toml declares maintenance task settings - ([maintenance.vacuum], [maintenance.balance], [maintenance.erasure_coding]) + - An optional admin.toml declares maintenance settings ([maintenance] + to toggle the whole system, plus per-task [maintenance.vacuum], + [maintenance.balance], [maintenance.erasure_coding]) - Settings in admin.toml are applied at every startup, overriding values saved from the admin UI, so they can be managed declaratively - Requires -dataDir; values can also be set via WEED_* environment diff --git a/weed/command/scaffold/admin.toml b/weed/command/scaffold/admin.toml index 3ea6f554b..d7dab0808 100644 --- a/weed/command/scaffold/admin.toml +++ b/weed/command/scaffold/admin.toml @@ -11,6 +11,10 @@ # Each value can also be set via environment variable, e.g. # export WEED_MAINTENANCE_VACUUM_GARBAGE_THRESHOLD=0.3 +[maintenance] +# toggle the entire maintenance system (task detection and execution) +# enabled = true + [maintenance.vacuum] # enabled = true # vacuum volumes with more deleted content than this ratio diff --git a/weed/pb/worker.proto b/weed/pb/worker.proto index d32f2efb3..3ab5ce960 100644 --- a/weed/pb/worker.proto +++ b/weed/pb/worker.proto @@ -324,7 +324,10 @@ message TaskLogEntry { // MaintenanceConfig holds configuration for the maintenance system message MaintenanceConfig { - bool enabled = 1; + // optional so a file that predates presence tracking (where an operator's + // explicit false and the field's absence are indistinguishable on the wire) + // can be told apart from one that explicitly persists the toggle + optional bool enabled = 1; int32 scan_interval_seconds = 2; // How often to scan for maintenance needs int32 worker_timeout_seconds = 3; // Worker heartbeat timeout int32 task_timeout_seconds = 4; // Individual task timeout diff --git a/weed/pb/worker_pb/worker.pb.go b/weed/pb/worker_pb/worker.pb.go index 501e9bd7d..46f4f088e 100644 --- a/weed/pb/worker_pb/worker.pb.go +++ b/weed/pb/worker_pb/worker.pb.go @@ -2566,16 +2566,19 @@ func (x *TaskLogEntry) GetStatus() string { // MaintenanceConfig holds configuration for the maintenance system type MaintenanceConfig struct { - state protoimpl.MessageState `protogen:"open.v1"` - Enabled bool `protobuf:"varint,1,opt,name=enabled,proto3" json:"enabled,omitempty"` - ScanIntervalSeconds int32 `protobuf:"varint,2,opt,name=scan_interval_seconds,json=scanIntervalSeconds,proto3" json:"scan_interval_seconds,omitempty"` // How often to scan for maintenance needs - WorkerTimeoutSeconds int32 `protobuf:"varint,3,opt,name=worker_timeout_seconds,json=workerTimeoutSeconds,proto3" json:"worker_timeout_seconds,omitempty"` // Worker heartbeat timeout - TaskTimeoutSeconds int32 `protobuf:"varint,4,opt,name=task_timeout_seconds,json=taskTimeoutSeconds,proto3" json:"task_timeout_seconds,omitempty"` // Individual task timeout - RetryDelaySeconds int32 `protobuf:"varint,5,opt,name=retry_delay_seconds,json=retryDelaySeconds,proto3" json:"retry_delay_seconds,omitempty"` // Delay between retries - MaxRetries int32 `protobuf:"varint,6,opt,name=max_retries,json=maxRetries,proto3" json:"max_retries,omitempty"` // Default max retries for tasks - CleanupIntervalSeconds int32 `protobuf:"varint,7,opt,name=cleanup_interval_seconds,json=cleanupIntervalSeconds,proto3" json:"cleanup_interval_seconds,omitempty"` // How often to clean up old tasks - TaskRetentionSeconds int32 `protobuf:"varint,8,opt,name=task_retention_seconds,json=taskRetentionSeconds,proto3" json:"task_retention_seconds,omitempty"` // How long to keep completed/failed tasks - Policy *MaintenancePolicy `protobuf:"bytes,9,opt,name=policy,proto3" json:"policy,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + // optional so a file that predates presence tracking (where an operator's + // explicit false and the field's absence are indistinguishable on the wire) + // can be told apart from one that explicitly persists the toggle + Enabled *bool `protobuf:"varint,1,opt,name=enabled,proto3,oneof" json:"enabled,omitempty"` + ScanIntervalSeconds int32 `protobuf:"varint,2,opt,name=scan_interval_seconds,json=scanIntervalSeconds,proto3" json:"scan_interval_seconds,omitempty"` // How often to scan for maintenance needs + WorkerTimeoutSeconds int32 `protobuf:"varint,3,opt,name=worker_timeout_seconds,json=workerTimeoutSeconds,proto3" json:"worker_timeout_seconds,omitempty"` // Worker heartbeat timeout + TaskTimeoutSeconds int32 `protobuf:"varint,4,opt,name=task_timeout_seconds,json=taskTimeoutSeconds,proto3" json:"task_timeout_seconds,omitempty"` // Individual task timeout + RetryDelaySeconds int32 `protobuf:"varint,5,opt,name=retry_delay_seconds,json=retryDelaySeconds,proto3" json:"retry_delay_seconds,omitempty"` // Delay between retries + MaxRetries int32 `protobuf:"varint,6,opt,name=max_retries,json=maxRetries,proto3" json:"max_retries,omitempty"` // Default max retries for tasks + CleanupIntervalSeconds int32 `protobuf:"varint,7,opt,name=cleanup_interval_seconds,json=cleanupIntervalSeconds,proto3" json:"cleanup_interval_seconds,omitempty"` // How often to clean up old tasks + TaskRetentionSeconds int32 `protobuf:"varint,8,opt,name=task_retention_seconds,json=taskRetentionSeconds,proto3" json:"task_retention_seconds,omitempty"` // How long to keep completed/failed tasks + Policy *MaintenancePolicy `protobuf:"bytes,9,opt,name=policy,proto3" json:"policy,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -2611,8 +2614,8 @@ func (*MaintenanceConfig) Descriptor() ([]byte, []int) { } func (x *MaintenanceConfig) GetEnabled() bool { - if x != nil { - return x.Enabled + if x != nil && x.Enabled != nil { + return *x.Enabled } return false } @@ -4245,9 +4248,9 @@ const file_worker_proto_rawDesc = "" + "\x06status\x18\x06 \x01(\tR\x06status\x1a9\n" + "\vFieldsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xc0\x03\n" + - "\x11MaintenanceConfig\x12\x18\n" + - "\aenabled\x18\x01 \x01(\bR\aenabled\x122\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xd1\x03\n" + + "\x11MaintenanceConfig\x12\x1d\n" + + "\aenabled\x18\x01 \x01(\bH\x00R\aenabled\x88\x01\x01\x122\n" + "\x15scan_interval_seconds\x18\x02 \x01(\x05R\x13scanIntervalSeconds\x124\n" + "\x16worker_timeout_seconds\x18\x03 \x01(\x05R\x14workerTimeoutSeconds\x120\n" + "\x14task_timeout_seconds\x18\x04 \x01(\x05R\x12taskTimeoutSeconds\x12.\n" + @@ -4256,7 +4259,9 @@ const file_worker_proto_rawDesc = "" + "maxRetries\x128\n" + "\x18cleanup_interval_seconds\x18\a \x01(\x05R\x16cleanupIntervalSeconds\x124\n" + "\x16task_retention_seconds\x18\b \x01(\x05R\x14taskRetentionSeconds\x124\n" + - "\x06policy\x18\t \x01(\v2\x1c.worker_pb.MaintenancePolicyR\x06policy\"\x80\x03\n" + + "\x06policy\x18\t \x01(\v2\x1c.worker_pb.MaintenancePolicyR\x06policyB\n" + + "\n" + + "\b_enabled\"\x80\x03\n" + "\x11MaintenancePolicy\x12S\n" + "\rtask_policies\x18\x01 \x03(\v2..worker_pb.MaintenancePolicy.TaskPoliciesEntryR\ftaskPolicies\x122\n" + "\x15global_max_concurrent\x18\x02 \x01(\x05R\x13globalMaxConcurrent\x12E\n" + @@ -4554,6 +4559,7 @@ func file_worker_proto_init() { (*TaskParams_EcBalanceParams)(nil), (*TaskParams_S3LifecycleParams)(nil), } + file_worker_proto_msgTypes[27].OneofWrappers = []any{} file_worker_proto_msgTypes[29].OneofWrappers = []any{ (*TaskPolicy_VacuumConfig)(nil), (*TaskPolicy_ErasureCodingConfig)(nil),