Merge pull request #30165 from vespa-engine/balder/gc-unused-distribution-config

Balder/gc unused distribution config

5 files changed, 4 insertions, 30 deletions

diff --git a/storage/src/tests/bucketdb/bucketmanagertest.cpp b/storage/src/tests/bucketdb/bucketmanagertest.cpp
index 45d8fab7061..6173a43e25e 100644
--- a/storage/src/tests/bucketdb/bucketmanagertest.cpp
+++ b/storage/src/tests/bucketdb/bucketmanagertest.cpp
@@ -689,7 +689,7 @@ public:
 
     static std::unique_ptr<lib::Distribution> default_grouped_distribution() {
         return std::make_unique<lib::Distribution>(
-                GlobalBucketSpaceDistributionConverter::string_to_config(vespalib::string(
+                lib::Distribution::ConfigWrapper(GlobalBucketSpaceDistributionConverter::string_to_config(vespalib::string(
 R"(redundancy 2
 group[3]
 group[0].name "invalid"
@@ -708,7 +708,7 @@ group[2].nodes[3]
 group[2].nodes[0].index 3
 group[2].nodes[1].index 4
 group[2].nodes[2].index 5
-)")));
+)"))));
     }
 
     static std::shared_ptr<lib::Distribution> derived_global_grouped_distribution() {
diff --git a/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp b/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
index 5631ec71e4d..774f90821fa 100644
--- a/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
+++ b/storage/src/tests/common/global_bucket_space_distribution_converter_test.cpp
@@ -37,7 +37,6 @@ initial_redundancy 0
 ensure_primary_persisted true
 ready_copies 3
 active_per_leaf_group true
-distributor_auto_ownership_transfer_on_whole_group_down true
 group[0].index "invalid"
 group[0].name "invalid"
 group[0].capacity 1
@@ -48,7 +47,6 @@ group[0].nodes[1].index 1
 group[0].nodes[1].retired false
 group[0].nodes[2].index 2
 group[0].nodes[2].retired false
-disk_distribution MODULO_BID
 )");
 
 }
@@ -92,7 +90,6 @@ initial_redundancy 0
 ensure_primary_persisted true
 ready_copies 6
 active_per_leaf_group true
-distributor_auto_ownership_transfer_on_whole_group_down true
 group[0].index "invalid"
 group[0].name "invalid"
 group[0].capacity 1
@@ -117,7 +114,6 @@ group[2].nodes[1].index 4
 group[2].nodes[1].retired false
 group[2].nodes[2].index 5
 group[2].nodes[2].retired false
-disk_distribution MODULO_BID
 )");
     EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
@@ -163,7 +159,6 @@ initial_redundancy 0
 ensure_primary_persisted true
 ready_copies 4
 active_per_leaf_group true
-distributor_auto_ownership_transfer_on_whole_group_down true
 group[0].index "invalid"
 group[0].name "invalid"
 group[0].capacity 1
@@ -200,7 +195,6 @@ group[6].capacity 1
 group[6].partitions ""
 group[6].nodes[0].index 3
 group[6].nodes[0].retired false
-disk_distribution MODULO_BID
 )");
     EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
@@ -234,7 +228,6 @@ initial_redundancy 0
 ensure_primary_persisted true
 ready_copies 3
 active_per_leaf_group true
-distributor_auto_ownership_transfer_on_whole_group_down true
 group[0].index "invalid"
 group[0].name "invalid"
 group[0].capacity 1
@@ -253,7 +246,6 @@ group[2].capacity 1
 group[2].partitions ""
 group[2].nodes[0].index 2
 group[2].nodes[0].retired false
-disk_distribution MODULO_BID
 )");
     EXPECT_EQ(expected_global_config, default_to_global_config(default_config));
 }
diff --git a/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp b/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp
index d21ecc814a5..51c0a75e45d 100644
--- a/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp
+++ b/storage/src/tests/distributor/top_level_bucket_db_updater_test.cpp
@@ -1580,18 +1580,6 @@ TEST_F(TopLevelBucketDBUpdaterTest, pending_cluster_state_with_group_down) {
                                  "distributor:6 .2.s:d storage:6"));
 }
 
-TEST_F(TopLevelBucketDBUpdaterTest, pending_cluster_state_with_group_down_and_no_handover) {
-    std::string config = dist_config_6_nodes_across_4_groups();
-    config += "distributor_auto_ownership_transfer_on_whole_group_down false\n";
-    set_distribution(config);
-
-    // Group is down, but config says to not do anything about it.
-    EXPECT_EQ(get_node_list({0, 1, 2, 3, 4, 5}, _bucket_spaces.size() - 1),
-              get_sent_nodes("distributor:6 storage:6",
-                             "distributor:6 .2.s:d .3.s:d storage:6"));
-}
-
-
 namespace {
 
 void
diff --git a/storage/src/vespa/storage/common/global_bucket_space_distribution_converter.cpp b/storage/src/vespa/storage/common/global_bucket_space_distribution_converter.cpp
index ec606af0690..eb42f19a5e8 100644
--- a/storage/src/vespa/storage/common/global_bucket_space_distribution_converter.cpp
+++ b/storage/src/vespa/storage/common/global_bucket_space_distribution_converter.cpp
@@ -4,7 +4,6 @@
 #include <vespa/vdslib/distribution/distribution.h>
 #include <vespa/config/print/asciiconfigwriter.h>
 #include <vespa/config/print/asciiconfigreader.hpp>
-#include <vespa/vespalib/util/stringfmt.h>
 #include <vespa/vdslib/distribution/distribution_config_util.h>
 #include <vespa/vespalib/stllike/asciistream.h>
 #include <cassert>
@@ -22,9 +21,7 @@ struct Group {
     std::map<uint16_t, std::unique_ptr<Group>> sub_groups;
 };
 
-void set_distribution_invariant_config_fields(DistributionConfigBuilder& builder, const DistributionConfig& source) {
-    builder.diskDistribution = source.diskDistribution;
-    builder.distributorAutoOwnershipTransferOnWholeGroupDown = true;
+void set_distribution_invariant_config_fields(DistributionConfigBuilder& builder) {
     builder.activePerLeafGroup = true;
     // TODO consider how to best support n-of-m replication for global docs
     builder.ensurePrimaryPersisted = true;
@@ -155,7 +152,7 @@ void build_global_groups(DistributionConfigBuilder& builder, const DistributionC
 std::shared_ptr<DistributionConfig>
 GlobalBucketSpaceDistributionConverter::convert_to_global(const DistributionConfig& source) {
     DistributionConfigBuilder builder;
-    set_distribution_invariant_config_fields(builder, source);
+    set_distribution_invariant_config_fields(builder);
     build_global_groups(builder, source);
     return std::make_shared<DistributionConfig>(builder);
 }
diff --git a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp
index aac16f8b618..19d66f629c5 100644
--- a/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp
+++ b/storage/src/vespa/storage/distributor/pending_bucket_space_db_transition.cpp
@@ -303,9 +303,6 @@ PendingBucketSpaceDbTransition::nodeNeedsOwnershipTransferFromGroupDown(
         const lib::ClusterState& state) const
 {
     const auto &dist(_bucket_space_state.get_distribution());
-    if (!dist.distributorAutoOwnershipTransferOnWholeGroupDown()) {
-        return false; // Not doing anything for downed groups.
-    }
     const lib::Group* group(dist.getNodeGraph().getGroupForNode(nodeIndex));
     // If there is no group information associated with the node (because the
     // group has changed or the node has been removed from config), we must