admin: honor a persisted or admin.toml maintenance enabled=false (#10909)

* admin: honor a persisted or admin.toml maintenance enabled=false

The startup path discarded an operator's enabled=false twice over:
ApplyDefaultsToProtobuf treated the bool zero value as unset and applied
the schema default of true, and a force-enable migration block flipped
any survivor. With the legacy /maintenance UI routes gone, nothing could
write the config either, so the maintenance system ran unconditionally.

Keep the persisted enabled flag across schema-default application in
LoadMaintenanceConfig, drop the force-enable block, and add a top-level
[maintenance] enabled key to admin.toml as the config surface, persisted
through SaveMaintenanceConfig like the per-task settings. Absent config
still defaults to enabled.

* admin: track presence on the maintenance enabled flag

A plain proto3 bool cannot distinguish an operator's persisted false
from a legacy file that simply omits the field, so honoring false would
have silently switched maintenance off for configs written before the
toggle could be persisted. Make the field optional: files that predate
presence tracking keep the enabled default, while a file that explicitly
persists the toggle is honored either way.
This commit is contained in:
Chris Lu
2026-08-24 00:01:48 -07:00
committed by GitHub
parent e931cccc7b
commit 68ec8ca655
11 changed files with 169 additions and 39 deletions
+6 -17
View File
@@ -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
+11
View File
@@ -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
+22 -1
View File
@@ -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)
+30
View File
@@ -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)
@@ -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.
@@ -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
@@ -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
}
+3 -2
View File
@@ -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
+4
View File
@@ -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
+4 -1
View File
@@ -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
+22 -16
View File
@@ -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),