diff --git a/weed/admin/dash/maintenance_policy_persistence_test.go b/weed/admin/dash/maintenance_policy_persistence_test.go index 77e650fe5..1f9e1b23a 100644 --- a/weed/admin/dash/maintenance_policy_persistence_test.go +++ b/weed/admin/dash/maintenance_policy_persistence_test.go @@ -7,8 +7,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/worker/tasks/vacuum" ) -// TestLoadMaintenanceConfigHonoursPersistedTaskConfigs guards against the regression in -// https://github.com/seaweedfs/seaweedfs/issues/10874: buildPolicyFromTaskConfigs used to call +// TestLoadMaintenanceConfigHonoursPersistedTaskConfigs guards against a regression: buildPolicyFromTaskConfigs used to call // LoadConfigFromPersistence(nil), which can never satisfy the loaders' type assertion, so every // task silently fell back to its compiled-in defaults (Enabled: true) and a task disabled in the // admin UI kept being scheduled. diff --git a/weed/admin/dash/maintenance_startup_test.go b/weed/admin/dash/maintenance_startup_test.go index 7bd9502c5..fd16fb99b 100644 --- a/weed/admin/dash/maintenance_startup_test.go +++ b/weed/admin/dash/maintenance_startup_test.go @@ -25,7 +25,7 @@ func restoreGlobalTaskState(t *testing.T) { // data directory that has a disabled balance task saved in it, and checks the end state that // actually matters: the balance detector reports disabled, so ScanWithTaskDetectors skips it. // -// This is the whole of issue #10874 in one test. The reporter disabled balance, and the +// This is the whole reported bug in one test. The reporter disabled balance, and the // scanner kept detecting balance tasks, cancelling them and re-detecting them. Two separate // defects had to line up for the disabled flag to survive to here: the policy had to be built // from the persisted configs rather than from a nil store, and the policy had to reach diff --git a/weed/admin/dash/task_policy_defaults_test.go b/weed/admin/dash/task_policy_defaults_test.go index 29da93352..3335232bb 100644 --- a/weed/admin/dash/task_policy_defaults_test.go +++ b/weed/admin/dash/task_policy_defaults_test.go @@ -129,9 +129,15 @@ func TestEcBalanceTaskPolicyRoundTrip(t *testing.T) { t.Errorf("ec_balance collection filter = %q, want the persisted %q", loaded.CollectionFilter, "pictures") } - // The generic dispatcher the maintenance manager uses has to know the type too. - if err := cp.SaveTaskPolicy("ec_balance", saved.ToTaskPolicy()); err != nil { - t.Errorf("SaveTaskPolicy(ec_balance): %v", err) + // The generic dispatcher the maintenance manager uses has to know the type too, and + // has to write the same file the dedicated loader reads. + dispatched := NewConfigPersistence(t.TempDir()) + if err := dispatched.SaveTaskPolicy("ec_balance", saved.ToTaskPolicy()); err != nil { + t.Fatalf("SaveTaskPolicy(ec_balance): %v", err) + } + roundTripped := ec_balance.LoadConfigFromPersistence(dispatched) + if roundTripped == nil || roundTripped.MinServerCount != 9 { + t.Errorf("SaveTaskPolicy(ec_balance) did not round-trip through the ec_balance store: %+v", roundTripped) } } @@ -164,8 +170,8 @@ func TestBuildPolicyKeepsTaskSpecificFields(t *testing.T) { if got := ecPolicy.GetReplicaPlacement(); got != "020" { t.Errorf("erasure coding replica placement = %q, want the persisted %q", got, "020") } - if got := ecPolicy.GetPreferredTags(); len(got) != 2 { - t.Errorf("erasure coding preferred tags = %v, want the persisted 2 entries", got) + if got := ecPolicy.GetPreferredTags(); len(got) != 2 || got[0] != "ssd" || got[1] != "archive" { + t.Errorf("erasure coding preferred tags = %v, want the persisted [ssd archive]", got) } balancePolicy := policy.TaskPolicies["balance"].GetBalanceConfig() diff --git a/weed/admin/maintenance/maintenance_manager.go b/weed/admin/maintenance/maintenance_manager.go index 26c60091c..6189fc687 100644 --- a/weed/admin/maintenance/maintenance_manager.go +++ b/weed/admin/maintenance/maintenance_manager.go @@ -201,8 +201,7 @@ func (mm *MaintenanceManager) scanLoop() { // configured interval instead never restores the normal cadence: once the errors stop, // getScanInterval returns scanInterval again, the comparison comes out false, and the // ticker is left at the backoff delay. A single transient scan failure therefore pinned - // the scanner to one scan per second forever - see issue #10874, where that produced a - // ~658 KB/s log flood and 193k orphaned task files. + // the scanner to one scan per second forever. activeInterval := scanInterval ticker := time.NewTicker(activeInterval) // Wrapped in a closure so the replacement ticker is stopped, not the one that happened diff --git a/weed/admin/maintenance/maintenance_policy_test.go b/weed/admin/maintenance/maintenance_policy_test.go index adc606c92..4ae3c94d6 100644 --- a/weed/admin/maintenance/maintenance_policy_test.go +++ b/weed/admin/maintenance/maintenance_policy_test.go @@ -70,8 +70,7 @@ func disabledStub() *stubConfigPersistence { } } -// TestBuildPolicyFromTaskConfigsUsesPersistence covers the bug reported in -// https://github.com/seaweedfs/seaweedfs/issues/10874: the persistence argument used to be a +// TestBuildPolicyFromTaskConfigsUsesPersistence covers a reported bug: the persistence argument used to be a // literal nil, which no type assertion can satisfy, so a task disabled on disk came back enabled. func TestBuildPolicyFromTaskConfigsUsesPersistence(t *testing.T) { policy := BuildPolicyFromTaskConfigs(disabledStub()) diff --git a/weed/admin/maintenance/maintenance_policy_wiring_test.go b/weed/admin/maintenance/maintenance_policy_wiring_test.go index c80744b18..fbbf4ab2f 100644 --- a/weed/admin/maintenance/maintenance_policy_wiring_test.go +++ b/weed/admin/maintenance/maintenance_policy_wiring_test.go @@ -38,7 +38,7 @@ func snapshotDetectorState(t *testing.T) { }) } -// TestPolicyReachesRegisteredDetectors is the regression test for the half of issue #10874 +// TestPolicyReachesRegisteredDetectors is the regression test for the half of the scan-flood bug // that a corrected policy alone did not fix: MaintenanceIntegration pushes the policy into // detectors and schedulers through interface{ SetEnabled(bool) } and // interface{ SetMaxConcurrent(int) } type assertions, but every task is backed by diff --git a/weed/admin/maintenance/maintenance_scan_cadence_test.go b/weed/admin/maintenance/maintenance_scan_cadence_test.go index 7dc195d78..e8db97234 100644 --- a/weed/admin/maintenance/maintenance_scan_cadence_test.go +++ b/weed/admin/maintenance/maintenance_scan_cadence_test.go @@ -39,8 +39,7 @@ func (c *scanCadenceClient) callCount() int { return c.calls } -// TestScanLoopRestoresIntervalAfterBackoff is the regression test for the scan flood in -// https://github.com/seaweedfs/seaweedfs/issues/10874. The loop shortens its ticker to the +// TestScanLoopRestoresIntervalAfterBackoff is the regression test for the scan flood. The loop shortens its ticker to the // error backoff delay after a failed scan. It used to compare the target interval against // the *configured* scan interval rather than against the interval the ticker was actually // running at, so once the errors stopped the comparison came out false and the ticker was @@ -104,15 +103,16 @@ func TestScanLoopRestoresIntervalAfterBackoff(t *testing.T) { // Observe a window that a 1s cadence would fill with scans and a 3s cadence would not. const window = 5 * time.Second time.Sleep(window) - scansInWindow := client.callCount() - before + callsInWindow := client.callCount() - before - // 3s cadence: at most 2 scans in 5s. 1s cadence: about 5. - if scansInWindow > 2 { - t.Errorf("scan loop ran %d scans in %v after recovering from a failed scan; "+ + // One scan makes two master-client calls. 3s cadence: at most 2 scans (4 calls) in 5s, + // even if a stall shifts the window. 1s cadence: about 5 scans (10 calls). + if callsInWindow > 4 { + t.Errorf("scan loop made %d master-client calls in %v after recovering from a failed scan; "+ "the ticker was left at the %v backoff instead of returning to the configured %v", - scansInWindow, window, time.Second, baseInterval) + callsInWindow, window, time.Second, baseInterval) } - if scansInWindow == 0 { + if callsInWindow == 0 { t.Errorf("scan loop ran no scans in %v, expected the %v cadence to fire at least once", window, baseInterval) } } diff --git a/weed/worker/tasks/base/generic_components.go b/weed/worker/tasks/base/generic_components.go index 259a5f777..3a2f9c703 100644 --- a/weed/worker/tasks/base/generic_components.go +++ b/weed/worker/tasks/base/generic_components.go @@ -49,7 +49,7 @@ func (d *GenericDetector) IsEnabled() bool { // MaintenanceIntegration.configureDetectorFromPolicy). Every registered task is // backed by this generic detector, so without this method that assertion failed // for every task and the policy never reached the flag that -// ScanWithTaskDetectors actually gates on. See issue #10874. +// ScanWithTaskDetectors actually gates on. func (d *GenericDetector) SetEnabled(enabled bool) { d.taskDef.Config.SetEnabled(enabled) } @@ -143,7 +143,7 @@ func (s *GenericScheduler) IsEnabled() bool { // SetEnabled turns scheduling for this task type on or off. Detector and scheduler // share one TaskDefinition, so this is the same flag GenericDetector.SetEnabled sets; // both setters exist because the maintenance integration configures the two -// independently. See GenericDetector.SetEnabled and issue #10874. +// independently. See GenericDetector.SetEnabled. func (s *GenericScheduler) SetEnabled(enabled bool) { s.taskDef.Config.SetEnabled(enabled) } diff --git a/weed/worker/tasks/vacuum/config_persistence_test.go b/weed/worker/tasks/vacuum/config_persistence_test.go index c05021109..e37e35bc7 100644 --- a/weed/worker/tasks/vacuum/config_persistence_test.go +++ b/weed/worker/tasks/vacuum/config_persistence_test.go @@ -18,8 +18,8 @@ func (s *stubVacuumStore) LoadVacuumTaskPolicy() (*worker_pb.TaskPolicy, error) // wrongShapedStore is what the maintenance manager used to be handed: a non-nil value that // does not satisfy the accessor the loader asserts on. It has to keep falling back to the -// defaults, but no longer silently - the reporter of issue #10874 had to read the source to -// find out why their disabled task kept running. +// defaults, but no longer silently: a fallback the operator cannot see reads as a disabled task +// that keeps running. type wrongShapedStore struct{} func (wrongShapedStore) SomethingElse() {}