two_phase_garbage_collection is always enabled

6 files changed, 1 insertions, 56 deletions

diff --git a/storage/src/tests/distributor/distributor_stripe_test.cpp b/storage/src/tests/distributor/distributor_stripe_test.cpp
index 4568447a28d..2fb65c9d46d 100644
--- a/storage/src/tests/distributor/distributor_stripe_test.cpp
+++ b/storage/src/tests/distributor/distributor_stripe_test.cpp
@@ -174,12 +174,6 @@ struct DistributorStripeTest : Test, DistributorStripeTestUtil {
         });
     }
 
-    void configure_enable_two_phase_garbage_collection(bool use_two_phase) {
-        configure_stripe_with([&](auto& builder) {
-            builder.enableTwoPhaseGarbageCollection = use_two_phase;
-        });
-    }
-
     void configure_enable_condition_probing(bool enable_probing) {
         configure_stripe_with([&](auto& builder) {
             builder.enableConditionProbing = enable_probing;
@@ -949,19 +943,6 @@ TEST_F(DistributorStripeTest, closing_aborts_gets_started_outside_stripe_thread)
     EXPECT_EQ(api::ReturnCode::ABORTED, _sender.reply(0)->getResult().getResult());
 }
 
-TEST_F(DistributorStripeTest, enable_two_phase_gc_config_is_propagated_to_internal_config)
-{
-    setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
-
-    EXPECT_TRUE(getConfig().enable_two_phase_garbage_collection());
-
-    configure_enable_two_phase_garbage_collection(true);
-    EXPECT_TRUE(getConfig().enable_two_phase_garbage_collection());
-
-    configure_enable_two_phase_garbage_collection(false);
-    EXPECT_FALSE(getConfig().enable_two_phase_garbage_collection());
-}
-
 TEST_F(DistributorStripeTest, enable_condition_probing_config_is_propagated_to_internal_config)
 {
     setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
diff --git a/storage/src/tests/distributor/garbagecollectiontest.cpp b/storage/src/tests/distributor/garbagecollectiontest.cpp
index bd80b9b447a..b28a2ab3d33 100644
--- a/storage/src/tests/distributor/garbagecollectiontest.cpp
+++ b/storage/src/tests/distributor/garbagecollectiontest.cpp
@@ -60,14 +60,6 @@ struct GarbageCollectionOperationTest : Test, DistributorStripeTestUtil {
         with_two_phase.two_phase_remove_location = true;
         set_node_supported_features(0, with_two_phase);
         set_node_supported_features(1, with_two_phase);
-
-        config_enable_two_phase_gc(true);
-    }
-
-    void config_enable_two_phase_gc(bool enabled) {
-        auto config = make_config();
-        config->set_enable_two_phase_garbage_collection(enabled);
-        configure_stripe(std::move(config));
     }
 
     std::shared_ptr<GarbageCollectionOperation> create_op() {
@@ -235,8 +227,6 @@ TEST_F(GarbageCollectionOperationTest, two_phase_gc_requires_config_enabling_and
     with_two_phase.two_phase_remove_location = true;
     set_node_supported_features(1, with_two_phase);
 
-    config_enable_two_phase_gc(true);
-
     // Config enabled, but only 1 node says it supports two-phase RemoveLocation
     auto op = create_op();
     op->start(_sender);
@@ -247,13 +237,6 @@ TEST_F(GarbageCollectionOperationTest, two_phase_gc_requires_config_enabling_and
     op = create_op();
     op->start(_sender);
     EXPECT_TRUE(op->is_two_phase());
-
-    // But doesn't matter if two-phase GC is config-disabled
-    config_enable_two_phase_gc(false);
-
-    op = create_op();
-    op->start(_sender);
-    EXPECT_FALSE(op->is_two_phase());
 }
 
 TEST_F(GarbageCollectionOperationTest, first_phase_sends_enumerate_only_remove_locations_with_provided_gc_pri) {
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp
index ff4f4a7e7bd..7e22d2f7dba 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.cpp
+++ b/storage/src/vespa/storage/config/distributorconfiguration.cpp
@@ -45,7 +45,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),
-      _enable_two_phase_garbage_collection(false),
       _enable_condition_probing(false),
       _enable_operation_cancellation(false),
       _minimumReplicaCountingMode(ReplicaCountingMode::TRUSTED)
@@ -152,7 +151,6 @@ DistributorConfiguration::configure(const DistributorManagerConfig & config)
     _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;
-    _enable_two_phase_garbage_collection = config.enableTwoPhaseGarbageCollection;
     _enable_condition_probing = config.enableConditionProbing;
     _enable_operation_cancellation = config.enableOperationCancellation;
     _minimumReplicaCountingMode = deriveReplicaCountingMode(config.minimumReplicaCountingMode);
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.h b/storage/src/vespa/storage/config/distributorconfiguration.h
index 8b161103747..bbd0ed8ea45 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.h
+++ b/storage/src/vespa/storage/config/distributorconfiguration.h
@@ -231,12 +231,6 @@ public:
         return _max_activation_inhibited_out_of_sync_groups;
     }
 
-    void set_enable_two_phase_garbage_collection(bool enable) noexcept {
-        _enable_two_phase_garbage_collection = enable;
-    }
-    [[nodiscard]] bool enable_two_phase_garbage_collection() const noexcept {
-        return _enable_two_phase_garbage_collection;
-    }
     void set_enable_condition_probing(bool enable) noexcept {
         _enable_condition_probing = enable;
     }
@@ -288,7 +282,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 _enable_two_phase_garbage_collection;
     bool _enable_condition_probing;
     bool _enable_operation_cancellation;
 
diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def
index 48316c0187f..c77d1b3eac3 100644
--- a/storage/src/vespa/storage/config/stor-distributormanager.def
+++ b/storage/src/vespa/storage/config/stor-distributormanager.def
@@ -174,14 +174,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 true, garbage collection is performed in two phases (metadata gathering and deletion)
-## instead of just a single phase. Two-phase GC allows for ensuring the same set of documents
-## is deleted across all nodes and explicitly takes write locks on the distributor to prevent
-## concurrent feed ops to GC'd documents from potentially creating inconsistencies.
-## Two-phase GC is only used iff all replica content nodes support the feature AND it's enabled
-## by this config.
-enable_two_phase_garbage_collection bool default=true
-
 ## If true, a conditional Put or Remove operation received for a bucket with inconsistent
 ## replicas will trigger an implicit distributed condition probe to resolve the outcome of
 ## the condition across all divergent replicas.
diff --git a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp
index cf23067748f..3fc0da5f2ca 100644
--- a/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp
+++ b/storage/src/vespa/storage/distributor/operations/idealstate/garbagecollectionoperation.cpp
@@ -95,9 +95,7 @@ void GarbageCollectionOperation::send_current_phase_remove_locations(Distributor
 }
 
 void GarbageCollectionOperation::onStart(DistributorStripeMessageSender& sender) {
-    if (_manager->operation_context().distributor_config().enable_two_phase_garbage_collection() &&
-        all_involved_nodes_support_two_phase_gc())
-    {
+    if (all_involved_nodes_support_two_phase_gc()) {
         _cluster_state_version_at_phase1_start_time = _bucketSpace->getClusterState().getVersion();
         LOG(debug, "Starting first phase of two-phase GC for %s at cluster state version %u",
             getBucket().toString().c_str(), _cluster_state_version_at_phase1_start_time);