diff options
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); |