GC priority control in config. Correct priority is essential to content layer, and should not be reconfigured.

5 files changed, 7 insertions, 128 deletions

diff --git a/storage/src/tests/distributor/distributor_stripe_test.cpp b/storage/src/tests/distributor/distributor_stripe_test.cpp
index 92cd3898886..a52b110b435 100644
--- a/storage/src/tests/distributor/distributor_stripe_test.cpp
+++ b/storage/src/tests/distributor/distributor_stripe_test.cpp
@@ -471,43 +471,6 @@ TEST_F(DistributorStripeTest, update_bucket_database)
               updateBucketDB("0:456", "2:333", ResetTrusted(true)));
 }
 
-TEST_F(DistributorStripeTest, priority_config_is_propagated_to_distributor_configuration)
-{
-    using namespace vespa::config::content::core;
-
-    setup_stripe(Redundancy(2), NodeCount(2), "storage:2 distributor:1");
-
-    ConfigBuilder builder;
-    builder.priorityMergeMoveToIdealNode = 1;
-    builder.priorityMergeOutOfSyncCopies = 2;
-    builder.priorityMergeTooFewCopies = 3;
-    builder.priorityActivateNoExistingActive = 4;
-    builder.priorityActivateWithExistingActive = 5;
-    builder.priorityDeleteBucketCopy = 6;
-    builder.priorityJoinBuckets = 7;
-    builder.prioritySplitDistributionBits = 8;
-    builder.prioritySplitLargeBucket = 9;
-    builder.prioritySplitInconsistentBucket = 10;
-    builder.priorityGarbageCollection = 11;
-    builder.priorityMergeGlobalBuckets = 12;
-
-    configure_stripe(builder);
-
-    const auto& mp = getConfig().getMaintenancePriorities();
-    EXPECT_EQ(1, static_cast<int>(mp.mergeMoveToIdealNode));
-    EXPECT_EQ(2, static_cast<int>(mp.mergeOutOfSyncCopies));
-    EXPECT_EQ(3, static_cast<int>(mp.mergeTooFewCopies));
-    EXPECT_EQ(4, static_cast<int>(mp.activateNoExistingActive));
-    EXPECT_EQ(5, static_cast<int>(mp.activateWithExistingActive));
-    EXPECT_EQ(6, static_cast<int>(mp.deleteBucketCopy));
-    EXPECT_EQ(7, static_cast<int>(mp.joinBuckets));
-    EXPECT_EQ(8, static_cast<int>(mp.splitDistributionBits));
-    EXPECT_EQ(9, static_cast<int>(mp.splitLargeBucket));
-    EXPECT_EQ(10, static_cast<int>(mp.splitInconsistentBucket));
-    EXPECT_EQ(11, static_cast<int>(mp.garbageCollection));
-    EXPECT_EQ(12, static_cast<int>(mp.mergeGlobalBuckets));
-}
-
 TEST_F(DistributorStripeTest, no_db_resurrection_for_bucket_not_owned_in_pending_state) {
     setup_stripe(Redundancy(1), NodeCount(10), "storage:2 distributor:2");
     // Force new state into being the pending state. According to the initial
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp
index 83be5d71b23..82f80911cf3 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.cpp
+++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp
@@ -96,25 +96,6 @@ DistributorConfiguration::containsTimeStatement(const std::string& documentSelec
     return visitor.hasCurrentTime;
 }
 
-void
-DistributorConfiguration::configureMaintenancePriorities(
-        const vespa::config::content::core::StorDistributormanagerConfig& cfg)
-{
-    MaintenancePriorities& mp(_maintenancePriorities);
-    mp.mergeMoveToIdealNode       = cfg.priorityMergeMoveToIdealNode;
-    mp.mergeOutOfSyncCopies       = cfg.priorityMergeOutOfSyncCopies;
-    mp.mergeTooFewCopies          = cfg.priorityMergeTooFewCopies;
-    mp.mergeGlobalBuckets         = cfg.priorityMergeGlobalBuckets;
-    mp.activateNoExistingActive   = cfg.priorityActivateNoExistingActive;
-    mp.activateWithExistingActive = cfg.priorityActivateWithExistingActive;
-    mp.deleteBucketCopy           = cfg.priorityDeleteBucketCopy;
-    mp.joinBuckets                = cfg.priorityJoinBuckets;
-    mp.splitDistributionBits      = cfg.prioritySplitDistributionBits;
-    mp.splitLargeBucket           = cfg.prioritySplitLargeBucket;
-    mp.splitInconsistentBucket    = cfg.prioritySplitInconsistentBucket;
-    mp.garbageCollection          = cfg.priorityGarbageCollection;
-}
-
 void 
 DistributorConfiguration::configure(const vespa::config::content::core::StorDistributormanagerConfig& config) 
 {
@@ -179,11 +160,8 @@ DistributorConfiguration::configure(const vespa::config::content::core::StorDist
     _enable_two_phase_garbage_collection = config.enableTwoPhaseGarbageCollection;
     _enable_condition_probing = config.enableConditionProbing;
     _enable_operation_cancellation = config.enableOperationCancellation;
-
     _minimumReplicaCountingMode = config.minimumReplicaCountingMode;
 
-    configureMaintenancePriorities(config);
-
     if (config.maxClusterClockSkewSec >= 0) {
         _maxClusterClockSkew = std::chrono::seconds(config.maxClusterClockSkewSec);
     }
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h
index 9d879fa62d5..8332a2b30db 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.h
+++ b/storage/src/vespa/storage/config/distributorconfiguration.h
@@ -12,21 +12,20 @@ namespace storage {
 namespace distributor { struct LegacyDistributorTest; }
 
 class DistributorConfiguration {
-public: 
+public:
+    DistributorConfiguration(const DistributorConfiguration& other) = delete;
+    DistributorConfiguration& operator=(const DistributorConfiguration& other) = delete;
     explicit DistributorConfiguration(StorageComponent& component);
     ~DistributorConfiguration();
 
-    struct MaintenancePriorities
-    {
-        // Defaults for these are chosen as those used as the current (non-
-        // configurable) values at the time of implementation.
-        uint8_t mergeMoveToIdealNode {120};
+    struct MaintenancePriorities {
+        uint8_t mergeMoveToIdealNode {165};
         uint8_t mergeOutOfSyncCopies {120};
         uint8_t mergeTooFewCopies {120};
         uint8_t mergeGlobalBuckets {115};
         uint8_t activateNoExistingActive {100};
         uint8_t activateWithExistingActive {100};
-        uint8_t deleteBucketCopy {100};
+        uint8_t deleteBucketCopy {120};
         uint8_t joinBuckets {155};
         uint8_t splitDistributionBits {200};
         uint8_t splitLargeBucket {175};
@@ -302,9 +301,6 @@ public:
     bool containsTimeStatement(const std::string& documentSelection) const;
     
 private:
-    DistributorConfiguration(const DistributorConfiguration& other);
-    DistributorConfiguration& operator=(const DistributorConfiguration& other);
-    
     StorageComponent& _component;
     
     uint32_t _byteCountSplitLimit;
@@ -359,10 +355,6 @@ private:
     bool _enable_operation_cancellation;
 
     DistrConfig::MinimumReplicaCountingMode _minimumReplicaCountingMode;
-
-    friend struct distributor::LegacyDistributorTest;
-    void configureMaintenancePriorities(
-            const vespa::config::content::core::StorDistributormanagerConfig&);
 };
 
 }
diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def
index 8c4f1d5bb17..dc779cd1c4b 100644
--- a/storage/src/vespa/storage/config/stor-distributormanager.def
+++ b/storage/src/vespa/storage/config/stor-distributormanager.def
@@ -60,60 +60,6 @@ maximum_nodes_per_merge int default=16
 ## distributor thread to be able to call tick() manually and run single threaded
 start_distributor_thread bool default=true restart
 
-## The number of ticks calls done before a wait is done.  This can be
-## set higher than 10 for the distributor to improve speed of bucket iterations
-## while still keep CPU load low/moderate.
-ticks_before_wait int default=10
-
-## Allow overriding default priorities of certain maintenance operations.
-## This is an advanced feature, do not touch this unless you have a very good
-## reason to do so! Configuring these values wrongly may cause starvation of
-## important operations, leading to unpredictable behavior and/or data loss.
-##
-## Merge used to move data to ideal location
-priority_merge_move_to_ideal_node int default=165
-
-## Merge for copies that have gotten out of sync with each other
-priority_merge_out_of_sync_copies int default=120
-
-## Merge for restoring redundancy of copies
-priority_merge_too_few_copies int default=120
-
-## Merge that involves a global bucket. There are generally significantly fewer such
-## buckets than default-space buckets, and searches to documents in the default space
-## may depend on the presence of (all) global documents. Consequently, this priority
-## should be higher (i.e. numerically smaller) than that of regular merges.
-priority_merge_global_buckets int default=115
-
-## Copy activation when there are no other active copies (likely causing
-## lack of search coverage for that bucket)
-priority_activate_no_existing_active int default=100
-
-## Copy activation when there is already an active copy for the bucket.
-priority_activate_with_existing_active int default=100
-
-## Deletion of bucket copy.
-priority_delete_bucket_copy int default=120
-
-## Joining caused by bucket siblings getting sufficiently small to fit into a
-## single bucket.
-priority_join_buckets int default=155
-
-## Splitting caused by system increasing its minimum distribution bit count.
-priority_split_distribution_bits int default=200
-
-## Splitting due to bucket exceeding max document count or byte size (see
-## splitcount and splitsize config values)
-priority_split_large_bucket int default=175
-
-## Splitting due to buckets being inconsistently split. Should be higher
-## priority than the vast majority of external load.
-priority_split_inconsistent_bucket int default=110
-
-## Background garbage collection. Should be lower priority than external load
-## and other ideal state operations (aside from perhaps minimum bit splitting).
-priority_garbage_collection int default=200
-
 ## The distributor can send joins that "lift" a bucket without any siblings
 ## higher up in the bucket tree hierarchy. The assumption is that if this
 ## is done for all sibling-less buckets, they will all eventually reach a
diff --git a/storage/src/vespa/storage/distributor/statecheckers.cpp b/storage/src/vespa/storage/distributor/statecheckers.cpp
index 97641ae86a6..3472628ec02 100644
--- a/storage/src/vespa/storage/distributor/statecheckers.cpp
+++ b/storage/src/vespa/storage/distributor/statecheckers.cpp
@@ -721,7 +721,7 @@ checkForNodesMissingFromIdealState(const StateChecker::Context& c)
                 if (c.idealState().size() > c.entry()->getNodeCount()) {
                     ret.markMissingReplica(node, mp.mergeTooFewCopies);
                 } else {
-                    ret.markMoveToIdealLocation(node,mp.mergeMoveToIdealNode);
+                    ret.markMoveToIdealLocation(node, mp.mergeMoveToIdealNode);
                 }
                 c.stats.incCopyingIn(node, c.getBucketSpace());
                 hasMissingReplica = true;