Merge pull request #30151 from vespa-engine/balder/enable_condition_probing

Condition probing has long been default

5 files changed, 13 insertions, 69 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());