Always clear_bucket_priority_on_schedule.

8 files changed, 7 insertions, 73 deletions

diff --git a/storage/src/tests/distributor/distributor_stripe_test.cpp b/storage/src/tests/distributor/distributor_stripe_test.cpp
index 302ed6d2d9b..5ae19088bfb 100644
--- a/storage/src/tests/distributor/distributor_stripe_test.cpp
+++ b/storage/src/tests/distributor/distributor_stripe_test.cpp
@@ -172,12 +172,6 @@ struct DistributorStripeTest : Test, DistributorStripeTestUtil {
         });
     }
 
-    void configure_implicitly_clear_priority_on_schedule(bool implicitly_clear) {
-        configure_stripe_with([&](auto& builder) {
-            builder.implicitlyClearBucketPriorityOnSchedule = implicitly_clear;
-        });
-    }
-
     void configure_use_unordered_merge_chaining(bool use_unordered) {
         configure_stripe_with([&](auto& builder) {
             builder.useUnorderedMergeChaining = use_unordered;
@@ -202,10 +196,6 @@ struct DistributorStripeTest : Test, DistributorStripeTestUtil {
         });
     }
 
-    [[nodiscard]] bool scheduler_has_implicitly_clear_priority_on_schedule_set() const noexcept {
-        return _stripe->_scheduler->implicitly_clear_priority_on_schedule();
-    }
-
     [[nodiscard]] bool distributor_owns_bucket_in_current_and_pending_states(document::BucketId bucket_id) const {
         return (getDistributorBucketSpace().get_bucket_ownership_flags(bucket_id).owned_in_pending_state() &&
                 getDistributorBucketSpace().check_ownership_in_pending_and_current_state(bucket_id).isOwned());
@@ -903,17 +893,6 @@ TEST_F(DistributorStripeTest, max_activation_inhibited_out_of_sync_groups_config
     EXPECT_EQ(getConfig().max_activation_inhibited_out_of_sync_groups(), 0);
 }
 
-TEST_F(DistributorStripeTest, implicitly_clear_priority_on_schedule_config_is_propagated_to_scheduler)
-{
-    setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
-
-    configure_implicitly_clear_priority_on_schedule(true);
-    EXPECT_TRUE(scheduler_has_implicitly_clear_priority_on_schedule_set());
-
-    configure_implicitly_clear_priority_on_schedule(false);
-    EXPECT_FALSE(scheduler_has_implicitly_clear_priority_on_schedule_set());
-}
-
 TEST_F(DistributorStripeTest, wanted_split_bit_count_is_lower_bounded)
 {
     setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
diff --git a/storage/src/tests/distributor/maintenanceschedulertest.cpp b/storage/src/tests/distributor/maintenanceschedulertest.cpp
index 461ad725574..f4bd9989869 100644
--- a/storage/src/tests/distributor/maintenanceschedulertest.cpp
+++ b/storage/src/tests/distributor/maintenanceschedulertest.cpp
@@ -4,9 +4,6 @@
 #include <vespa/storage/distributor/maintenance/maintenancescheduler.h>
 #include <tests/distributor/maintenancemocks.h>
 #include <vespa/vespalib/gtest/gtest.h>
-#include <memory>
-#include <string>
-#include <sstream>
 
 using document::test::makeDocumentBucket;
 using namespace ::testing;
@@ -31,10 +28,6 @@ struct MaintenanceSchedulerTest : TestWithParam<bool> {
           _pending_window_checker(),
           _scheduler(_operation_generator, _priority_db, _pending_window_checker, _operation_starter)
     {}
-
-    void SetUp() override {
-        _scheduler.set_implicitly_clear_priority_on_schedule(GetParam());
-    }
 };
 
 TEST_P(MaintenanceSchedulerTest, priority_cleared_after_scheduled) {
@@ -80,17 +73,6 @@ TEST_P(MaintenanceSchedulerTest, suppress_low_priorities_in_emergency_mode) {
               _priority_db.toString());
 }
 
-TEST_P(MaintenanceSchedulerTest, priority_not_cleared_if_operation_not_started) {
-    if (GetParam()) {
-        return; // Only works when implicit clearing is NOT enabled
-    }
-    _priority_db.setPriority(PrioritizedBucket(makeDocumentBucket(BucketId(16, 1)), Priority::HIGH));
-    _operation_starter.setShouldStartOperations(false);
-    WaitTimeMs waitMs(_scheduler.tick(MaintenanceScheduler::NORMAL_SCHEDULING_MODE));
-    EXPECT_EQ(WaitTimeMs(1), waitMs);
-    EXPECT_EQ("PrioritizedBucket(Bucket(BucketSpace(0x0000000000000001), BucketId(0x4000000000000001)), pri HIGH)\n",
-              _priority_db.toString());
-}
 
 TEST_P(MaintenanceSchedulerTest, priority_cleared_if_operation_not_started_inside_pending_window) {
     if (!GetParam()) {
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp
index 6949882a2f7..443dcc32174 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.cpp
+++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp
@@ -43,7 +43,6 @@ DistributorConfiguration::DistributorConfiguration(StorageComponent& component)
       _merge_operations_disabled(false),
       _use_weak_internal_read_consistency_for_client_gets(false),
       _enable_metadata_only_fetch_phase_for_inconsistent_updates(false),
-      _implicitly_clear_priority_on_schedule(false),
       _use_unordered_merge_chaining(false),
       _inhibit_default_merges_when_global_merges_pending(false),
       _enable_two_phase_garbage_collection(false),
@@ -141,7 +140,6 @@ DistributorConfiguration::configure(const vespa::config::content::core::StorDist
     _use_weak_internal_read_consistency_for_client_gets = config.useWeakInternalReadConsistencyForClientGets;
     _enable_metadata_only_fetch_phase_for_inconsistent_updates = config.enableMetadataOnlyFetchPhaseForInconsistentUpdates;
     _max_activation_inhibited_out_of_sync_groups = config.maxActivationInhibitedOutOfSyncGroups;
-    _implicitly_clear_priority_on_schedule = config.implicitlyClearBucketPriorityOnSchedule;
     _use_unordered_merge_chaining = config.useUnorderedMergeChaining;
     _inhibit_default_merges_when_global_merges_pending = config.inhibitDefaultMergesWhenGlobalMergesPending;
     _enable_two_phase_garbage_collection = config.enableTwoPhaseGarbageCollection;
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h
index 178a966a2a7..5b1e1c0af4c 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.h
+++ b/storage/src/vespa/storage/config/distributorconfiguration.h
@@ -230,9 +230,6 @@ public:
         return _max_activation_inhibited_out_of_sync_groups;
     }
 
-    [[nodiscard]] bool implicitly_clear_priority_on_schedule() const noexcept {
-        return _implicitly_clear_priority_on_schedule;
-    }
     void set_use_unordered_merge_chaining(bool unordered) noexcept {
         _use_unordered_merge_chaining = unordered;
     }
@@ -301,7 +298,6 @@ private:
     bool _merge_operations_disabled;
     bool _use_weak_internal_read_consistency_for_client_gets;
     bool _enable_metadata_only_fetch_phase_for_inconsistent_updates;
-    bool _implicitly_clear_priority_on_schedule;
     bool _use_unordered_merge_chaining;
     bool _inhibit_default_merges_when_global_merges_pending;
     bool _enable_two_phase_garbage_collection;
diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def
index 2ff058e2a1b..cf6e65c13bb 100644
--- a/storage/src/vespa/storage/config/stor-distributormanager.def
+++ b/storage/src/vespa/storage/config/stor-distributormanager.def
@@ -177,12 +177,6 @@ max_activation_inhibited_out_of_sync_groups int default=0
 ## Stripe counts must be a power of two.
 num_distributor_stripes int default=0 restart
 
-## If set, the maintenance scheduler will implicitly clear entries from its internal
-## bucket maintenance priority database even when no operation can be started for the
-## bucket due to being blocked by concurrent operations. This avoids potential head-of-line
-## blocking of later buckets in the priority database.
-implicitly_clear_bucket_priority_on_schedule bool default=true
-
 ## Enables sending merges that are forwarded between content nodes in ideal state node key
 ## order, instead of strictly increasing node key order (which is the default).
 ## Even if this config is set to true, unordered merges will only be sent if _all_ nodes
diff --git a/storage/src/vespa/storage/distributor/distributor_stripe.cpp b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
index 4f84785ccae..4ddb4056232 100644
--- a/storage/src/vespa/storage/distributor/distributor_stripe.cpp
+++ b/storage/src/vespa/storage/distributor/distributor_stripe.cpp
@@ -885,7 +885,6 @@ DistributorStripe::propagate_config_snapshot_to_internal_components()
             getConfig().allowStaleReadsDuringClusterStateTransitions());
     _externalOperationHandler.set_use_weak_internal_read_consistency_for_gets(
             getConfig().use_weak_internal_read_consistency_for_client_gets());
-    _scheduler->set_implicitly_clear_priority_on_schedule(getConfig().implicitly_clear_priority_on_schedule());
 }
 
 void
diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
index 7bcb887d869..7c85d3d8e46 100644
--- a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
+++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.cpp
@@ -19,8 +19,7 @@ MaintenanceScheduler::MaintenanceScheduler(
     : _operationGenerator(operationGenerator),
       _priorityDb(priorityDb),
       _pending_window_checker(pending_window_checker),
-      _operationStarter(operationStarter),
-      _implicitly_clear_priority_on_schedule(false)
+      _operationStarter(operationStarter)
 {
 }
 
@@ -46,7 +45,7 @@ MaintenanceScheduler::tick(SchedulingMode currentMode)
     // maintenance scheduling until we're able to schedule the next possible bucket.
     // The inverse is the case for other maintenance operations.
     const bool is_activation = has_bucket_activation_priority(mostImportant);
-    if (_implicitly_clear_priority_on_schedule && !is_activation) {
+    if (!is_activation) {
         // If we can't start the operation, move on to the next bucket. Bucket will be
         // re-prioritized when the distributor stripe next scans it.
         clearPriority(mostImportant);
@@ -54,7 +53,7 @@ MaintenanceScheduler::tick(SchedulingMode currentMode)
     if (!startOperation(mostImportant)) {
         return WaitTimeMs(1);
     }
-    if (!_implicitly_clear_priority_on_schedule || is_activation) {
+    if (is_activation) {
         clearPriority(mostImportant);
     }
     return WaitTimeMs(0);
@@ -68,9 +67,7 @@ MaintenanceScheduler::possibleToSchedule(const PrioritizedBucket& bucket,
         return false;
     }
     // If pending window is full nothing of equal or lower priority can be scheduled, so no point in trying.
-    if (_implicitly_clear_priority_on_schedule &&
-        !_pending_window_checker.may_allow_operation_with_priority(convertToOperationPriority(bucket.getPriority())))
-    {
+    if (!_pending_window_checker.may_allow_operation_with_priority(convertToOperationPriority(bucket.getPriority()))) {
         return false;
     }
     if (currentMode == RECOVERY_SCHEDULING_MODE) {
@@ -81,8 +78,7 @@ MaintenanceScheduler::possibleToSchedule(const PrioritizedBucket& bucket,
 }
 
 bool
-MaintenanceScheduler::possibleToScheduleInEmergency(
-        const PrioritizedBucket& bucket) const
+MaintenanceScheduler::possibleToScheduleInEmergency(const PrioritizedBucket& bucket) const
 {
     return bucket.moreImportantThan(MaintenancePriority::VERY_HIGH);
 }
@@ -90,8 +86,7 @@ MaintenanceScheduler::possibleToScheduleInEmergency(
 void
 MaintenanceScheduler::clearPriority(const PrioritizedBucket& bucket)
 {
-    _priorityDb.setPriority(PrioritizedBucket(bucket.getBucket(),
-                                              MaintenancePriority::NO_MAINTENANCE_NEEDED));
+    _priorityDb.setPriority(PrioritizedBucket(bucket.getBucket(), MaintenancePriority::NO_MAINTENANCE_NEEDED));
 }
 
 OperationStarter::Priority
@@ -122,8 +117,7 @@ MaintenanceScheduler::startOperation(const PrioritizedBucket& bucket)
     if (!operation) {
         return true;
     }
-    OperationStarter::Priority operationPriority(
-            convertToOperationPriority(bucket.getPriority()));
+    OperationStarter::Priority operationPriority(convertToOperationPriority(bucket.getPriority()));
     return _operationStarter.start(operation, operationPriority);
 }
 
diff --git a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.h b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.h
index 18e40b156e0..105f23e7066 100644
--- a/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.h
+++ b/storage/src/vespa/storage/distributor/maintenance/maintenancescheduler.h
@@ -28,13 +28,6 @@ public:
 
     WaitTimeMs tick(SchedulingMode currentMode);
 
-    void set_implicitly_clear_priority_on_schedule(bool implicitly_clear) noexcept {
-        _implicitly_clear_priority_on_schedule = implicitly_clear;
-    }
-    [[nodiscard]] bool implicitly_clear_priority_on_schedule() const noexcept {
-        return _implicitly_clear_priority_on_schedule;
-    }
-
 private:
     MaintenanceScheduler(const MaintenanceScheduler&);
     MaintenanceScheduler& operator=(const MaintenanceScheduler&);
@@ -51,7 +44,6 @@ private:
     BucketPriorityDatabase&        _priorityDb;
     const PendingWindowChecker&    _pending_window_checker;
     OperationStarter&              _operationStarter;
-    bool                           _implicitly_clear_priority_on_schedule;
 };
 
 }