Condition probing has long been default

9 files changed, 13 insertions, 88 deletions

diff --git a/storage/src/tests/distributor/check_condition_test.cpp b/storage/src/tests/distributor/check_condition_test.cpp
index a47505a2ee6..5308d9a7c34 100644
--- a/storage/src/tests/distributor/check_condition_test.cpp
+++ b/storage/src/tests/distributor/check_condition_test.cpp
@@ -39,7 +39,6 @@ public:
         // By default, set up 2 nodes {0, 1} with mutually out of sync replica state
         // and with both reporting that they support condition probing.
         setup_stripe(2, 2, "version:1 storage:2 distributor:1");
-        config_enable_condition_probing(true);
         tag_content_node_supports_condition_probing(0, true);
         tag_content_node_supports_condition_probing(1, true);
         addNodesToBucketDB(_bucket_id, "0=10/20/30/t,1=40/50/60");
@@ -119,12 +118,6 @@ public:
 CheckConditionTest::CheckConditionTest() = default;
 CheckConditionTest::~CheckConditionTest() = default;
 
-TEST_F(CheckConditionTest, no_checker_returned_when_config_disabled) {
-    config_enable_condition_probing(false);
-    auto cond = create_check_condition();
-    EXPECT_FALSE(cond);
-}
-
 TEST_F(CheckConditionTest, no_checker_returned_when_probing_not_supported_on_at_least_one_node) {
     tag_content_node_supports_condition_probing(1, false);
     auto cond = create_check_condition();
diff --git a/storage/src/tests/distributor/distributor_stripe_test.cpp b/storage/src/tests/distributor/distributor_stripe_test.cpp
index 2fb65c9d46d..b3efe8e4bad 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_condition_probing(bool enable_probing) {
-        configure_stripe_with([&](auto& builder) {
-            builder.enableConditionProbing = enable_probing;
-        });
-    }
-
     void configure_enable_operation_cancellation(bool enable_cancellation) {
         configure_stripe_with([&](auto& builder) {
             builder.enableOperationCancellation = enable_cancellation;
@@ -943,19 +937,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_condition_probing_config_is_propagated_to_internal_config)
-{
-    setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
-
-    EXPECT_TRUE(getConfig().enable_condition_probing());
-
-    configure_enable_condition_probing(false);
-    EXPECT_FALSE(getConfig().enable_condition_probing());
-
-    configure_enable_condition_probing(true);
-    EXPECT_TRUE(getConfig().enable_condition_probing());
-}
-
 TEST_F(DistributorStripeTest, enable_operation_cancellation_config_is_propagated_to_internal_config) {
     setup_stripe(Redundancy(1), NodeCount(1), "distributor:1 storage:1");
 
@@ -1023,7 +1004,6 @@ TEST_F(DistributorStripeTest, distribution_config_change_edge_cancels_pending_op
 void DistributorStripeTest::set_up_for_bucket_ownership_cancellation(uint32_t superbucket_idx) {
     setup_stripe(Redundancy(1), NodeCount(10), "version:1 distributor:2 storage:2");
     configure_stripe_with([](auto& builder) {
-        builder.enableConditionProbing = true;
         builder.enableOperationCancellation = true;
     });
 
diff --git a/storage/src/tests/distributor/distributor_stripe_test_util.cpp b/storage/src/tests/distributor/distributor_stripe_test_util.cpp
index 65bc6d1fe3e..923c7d1730b 100644
--- a/storage/src/tests/distributor/distributor_stripe_test_util.cpp
+++ b/storage/src/tests/distributor/distributor_stripe_test_util.cpp
@@ -554,13 +554,6 @@ DistributorStripeTestUtil::setSystemState(const lib::ClusterState& systemState)
 }
 
 void
-DistributorStripeTestUtil::config_enable_condition_probing(bool enable) {
-    auto cfg = make_config();
-    cfg->set_enable_condition_probing(enable);
-    configure_stripe(cfg);
-}
-
-void
 DistributorStripeTestUtil::tag_content_node_supports_condition_probing(uint16_t index, bool supported) {
     NodeSupportedFeatures features;
     features.document_condition_probe = supported;
diff --git a/storage/src/tests/distributor/putoperationtest.cpp b/storage/src/tests/distributor/putoperationtest.cpp
index 5c6510ab1ec..f048472f3a9 100644
--- a/storage/src/tests/distributor/putoperationtest.cpp
+++ b/storage/src/tests/distributor/putoperationtest.cpp
@@ -724,7 +724,6 @@ TEST_F(PutOperationTest, minority_failure_override_not_in_effect_for_non_tas_err
 
 void PutOperationTest::set_up_tas_put_with_2_inconsistent_replica_nodes(bool create) {
     setup_stripe(Redundancy(2), NodeCount(2), "version:1 storage:2 distributor:1");
-    config_enable_condition_probing(true);
     tag_content_node_supports_condition_probing(0, true);
     tag_content_node_supports_condition_probing(1, true);
 
@@ -875,7 +874,6 @@ TEST_F(PutOperationTest, not_found_condition_probe_with_create_set_acts_as_if_ma
 
 TEST_F(PutOperationTest, conditional_put_no_replicas_case_with_create_set_acts_as_if_matched) {
     setup_stripe(Redundancy(2), NodeCount(2), "version:1 storage:2 distributor:1");
-    config_enable_condition_probing(true);
     tag_content_node_supports_condition_probing(0, true);
     tag_content_node_supports_condition_probing(1, true);
 
diff --git a/storage/src/tests/distributor/removeoperationtest.cpp b/storage/src/tests/distributor/removeoperationtest.cpp
index 3da36312e18..f58634bfbae 100644
--- a/storage/src/tests/distributor/removeoperationtest.cpp
+++ b/storage/src/tests/distributor/removeoperationtest.cpp
@@ -105,7 +105,6 @@ struct ExtRemoveOperationTest : public RemoveOperationTest {
 
 void ExtRemoveOperationTest::set_up_tas_remove_with_2_nodes(ReplicaState replica_state) {
     setup_stripe(Redundancy(2), NodeCount(2), "version:1 storage:2 distributor:1");
-    config_enable_condition_probing(true);
     tag_content_node_supports_condition_probing(0, true);
     tag_content_node_supports_condition_probing(1, true);
 
@@ -134,8 +133,7 @@ TEST_F(RemoveOperationTest, simple) {
 
     sendRemove();
 
-    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 1",
+    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 1",
             _sender.getLastCommand());
 
     replyToMessage(*op, -1, 34);
@@ -150,8 +148,7 @@ TEST_F(RemoveOperationTest, not_found) {
 
     sendRemove();
 
-    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 1",
+    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 1",
               _sender.getLastCommand());
 
     replyToMessage(*op, -1, 0);
@@ -166,8 +163,7 @@ TEST_F(RemoveOperationTest, storage_failure) {
 
     sendRemove();
 
-    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 1",
+    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 1",
               _sender.getLastCommand());
 
     sendReply(*op, -1, api::ReturnCode::INTERNAL_FAILURE);
@@ -190,12 +186,9 @@ TEST_F(RemoveOperationTest, multiple_copies) {
 
     sendRemove();
 
-    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 1,"
-              "Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 2,"
-              "Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 3",
+    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 1,"
+              "Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 2,"
+              "Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 3",
               _sender.getCommands(true, true));
 
     replyToMessage(*op, 0, 34);
@@ -212,8 +205,7 @@ TEST_F(RemoveOperationTest, can_send_remove_when_all_replica_nodes_retired) {
     addNodesToBucketDB(bucketId, "0=123");
     sendRemove();
 
-    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, "
-              "timestamp 100) => 0",
+    ASSERT_EQ("Remove(BucketId(0x4000000000000593), id:test:test::uri, timestamp 100) => 0",
               _sender.getLastCommand());
 }
 
@@ -231,9 +223,7 @@ TEST_F(ExtRemoveOperationTest, conditional_removes_are_instantly_successful_when
     ASSERT_NO_FATAL_FAILURE(set_up_tas_remove_with_2_nodes(ReplicaState::NONE));
     ASSERT_EQ("", _sender.getCommands(true));
     ASSERT_EQ(_sender.replies().size(), 1);
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, not found) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, not found) "
               "ReturnCode(NONE)",
               _sender.getLastReply());
 }
@@ -257,9 +247,7 @@ TEST_F(ExtRemoveOperationTest, matching_condition_probe_sends_unconditional_remo
     ASSERT_TRUE(_sender.replies().empty());
     reply_with(make_remove_reply(3, 50)); // remove from node 0
     ASSERT_EQ(_sender.replies().size(), 1);
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, removed doc from 50) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, removed doc from 50) "
               "ReturnCode(NONE)",
               _sender.getLastReply());
 }
@@ -271,9 +259,7 @@ TEST_F(ExtRemoveOperationTest, mismatching_condition_probe_fails_op_with_tas_err
     reply_with(make_get_reply(1, 50, false, false));
 
     ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true));
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, not found) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, not found) "
               "ReturnCode(TEST_AND_SET_CONDITION_FAILED, Condition did not match document)",
               _sender.getLastReply());
 }
@@ -286,9 +272,7 @@ TEST_F(ExtRemoveOperationTest, not_found_condition_probe_fails_op_with_tas_error
     reply_with(make_get_reply(1, 0, false, false));
 
     ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true));
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, not found) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, not found) "
               "ReturnCode(TEST_AND_SET_CONDITION_FAILED, Document does not exist)",
               _sender.getLastReply());
 }
@@ -300,9 +284,7 @@ TEST_F(ExtRemoveOperationTest, failed_condition_probe_fails_op_with_returned_err
     reply_with(make_failure_reply(1));
 
     ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true));
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, not found) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, not found) "
               "ReturnCode(ABORTED, Failed during write repair condition probe step. Reason: "
               "One or more replicas failed during test-and-set condition evaluation)",
               _sender.getLastReply());
@@ -319,9 +301,7 @@ TEST_F(ExtRemoveOperationTest, cancellation_during_condition_probe_fails_operati
     reply_with(make_get_reply(1, 50, false, true));
 
     ASSERT_EQ("Get => 1,Get => 0", _sender.getCommands(true));
-    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), "
-              "id:test:test::uri, "
-              "timestamp 100, not found) "
+    EXPECT_EQ("RemoveReply(BucketId(0x0000000000000000), id:test:test::uri, timestamp 100, not found) "
               "ReturnCode(ABORTED, Failed during write repair condition probe step. Reason: "
               "Operation has been cancelled (likely due to a cluster state change))",
               _sender.getLastReply());
diff --git a/storage/src/vespa/storage/config/distributorconfiguration.cpp b/storage/src/vespa/storage/config/distributorconfiguration.cpp
index 7e22d2f7dba..8ac8c3e7414 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_condition_probing(false),
       _enable_operation_cancellation(false),
       _minimumReplicaCountingMode(ReplicaCountingMode::TRUSTED)
 {
@@ -151,7 +150,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_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 bbd0ed8ea45..07e247f7e44 100644
--- a/storage/src/vespa/storage/config/distributorconfiguration.h
+++ b/storage/src/vespa/storage/config/distributorconfiguration.h
@@ -231,13 +231,6 @@ public:
         return _max_activation_inhibited_out_of_sync_groups;
     }
 
-    void set_enable_condition_probing(bool enable) noexcept {
-        _enable_condition_probing = enable;
-    }
-    [[nodiscard]] bool enable_condition_probing() const noexcept {
-        return _enable_condition_probing;
-    }
-
     [[nodiscard]] bool enable_operation_cancellation() const noexcept {
         return _enable_operation_cancellation;
     }
@@ -282,7 +275,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_condition_probing;
     bool _enable_operation_cancellation;
 
     ReplicaCountingMode _minimumReplicaCountingMode;
diff --git a/storage/src/vespa/storage/config/stor-distributormanager.def b/storage/src/vespa/storage/config/stor-distributormanager.def
index c77d1b3eac3..7e0d557776c 100644
--- a/storage/src/vespa/storage/config/stor-distributormanager.def
+++ b/storage/src/vespa/storage/config/stor-distributormanager.def
@@ -174,11 +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, 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.
-enable_condition_probing bool default=true
-
 ## If true, changes in the cluster where a subset of the nodes become unavailable or buckets
 ## change ownership between distributors will trigger an explicit cancellation of all pending
 ## requests partially or fully "invalidated" by such a change.
diff --git a/storage/src/vespa/storage/distributor/operations/external/check_condition.cpp b/storage/src/vespa/storage/distributor/operations/external/check_condition.cpp
index 76603df87c1..5437386d531 100644
--- a/storage/src/vespa/storage/distributor/operations/external/check_condition.cpp
+++ b/storage/src/vespa/storage/distributor/operations/external/check_condition.cpp
@@ -246,10 +246,6 @@ CheckCondition::create_if_inconsistent_replicas(const document::Bucket& bucket,
                                                 PersistenceOperationMetricSet& condition_probe_metrics,
                                                 uint32_t trace_level)
 {
-    // TODO move this check to the caller?
-    if (!op_ctx.distributor_config().enable_condition_probing()) {
-        return {};
-    }
     auto entries = get_bucket_database_entries(bucket_space, bucket.getBucketId());
     if (entries.empty()) {
         return {}; // Not found