Merge pull request #61476 from Avogar/better-stress-upgrade-checks

Ignore DROP queries in stress test with 1/2 probability and TRUNCATE Memory/JOIN table instead of ignoring DROP in upgrade check

programs/client/Client.cpp

@@ -950,6 +950,7 @@ void Client::addOptions(OptionsDescription & options_description)
         ("opentelemetry-tracestate", po::value<std::string>(), "OpenTelemetry tracestate header as described by W3C Trace Context recommendation")
 
         ("no-warnings", "disable warnings when client connects to server")
+        /// TODO: Left for compatibility as it's used in upgrade check, remove after next release and use server setting ignore_drop_queries_probability
         ("fake-drop", "Ignore all DROP queries, should be used only for testing")
         ("accept-invalid-certificate", "Ignore certificate verification errors, equal to config parameters openSSL.client.invalidCertificateHandler.name=AcceptCertificateHandler and openSSL.client.verificationMode=none")
     ;
@@ -1093,7 +1094,7 @@ void Client::processOptions(const OptionsDescription & options_description,
     if (options.count("no-warnings"))
         config().setBool("no-warnings", true);
     if (options.count("fake-drop"))
-        fake_drop = true;
+        config().setString("ignore_drop_queries_probability", "1");
     if (options.count("accept-invalid-certificate"))
     {
         config().setString("openSSL.client.invalidCertificateHandler.name", "AcceptCertificateHandler");

src/Client/ClientBase.cpp

@@ -958,12 +958,8 @@ void ClientBase::processTextAsSingleQuery(const String & full_query)
         processError(full_query);
 }
 
-
 void ClientBase::processOrdinaryQuery(const String & query_to_execute, ASTPtr parsed_query)
 {
-    if (fake_drop && parsed_query->as<ASTDropQuery>())
-        return;
-
     auto query = query_to_execute;
 
     /// Rewrite query only when we have query parameters.

src/Client/ClientBase.h

@@ -315,8 +315,6 @@ class ClientBase : public Poco::Util::Application, public IHints<2>
     QueryProcessingStage::Enum query_processing_stage;
     ClientInfo::QueryKind query_kind;
 
-    bool fake_drop = false;
-
     struct HostAndPort
     {
         String host;

src/Core/Settings.h

@@ -870,6 +870,7 @@ class IColumn;
     M(Bool, optimize_uniq_to_count, true, "Rewrite uniq and its variants(except uniqUpTo) to count if subquery has distinct or group by clause.", 0) \
     M(Bool, use_variant_as_common_type, false, "Use Variant as a result type for if/multiIf in case when there is no common type for arguments", 0) \
     M(Bool, enable_order_by_all, true, "Enable sorting expression ORDER BY ALL.", 0) \
+    M(Float, ignore_drop_queries_probability, 0, "If enabled, server will ignore all DROP table queries with specified probability (for Memory and JOIN engines it will replcase DROP to TRUNCATE). Used for testing purposes", 0) \
     M(Bool, traverse_shadow_remote_data_paths, false, "Traverse shadow directory when query system.remote_data_paths", 0) \
     M(Bool, geo_distance_returns_float64_on_float64_arguments, true, "If all four arguments to `geoDistance`, `greatCircleDistance`, `greatCircleAngle` functions are Float64, return Float64 and use double precision for internal calculations. In previous ClickHouse versions, the functions always returned Float32.", 0) \
     M(Bool, allow_get_client_http_header, false, "Allow to use the function `getClientHTTPHeader` which lets to obtain a value of an the current HTTP request's header. It is not enabled by default for security reasons, because some headers, such as `Cookie`, could contain sensitive info. Note that the `X-ClickHouse-*` and `Authentication` headers are always restricted and cannot be obtained with this function.", 0) \

src/Core/SettingsChangesHistory.h

@@ -86,6 +86,7 @@ namespace SettingsChangesHistory
 static std::map<ClickHouseVersion, SettingsChangesHistory::SettingsChanges> settings_changes_history =
 {
     {"24.4", {{"input_format_json_throw_on_bad_escape_sequence", true, true, "Allow to save JSON strings with bad escape sequences"},
+              {"ignore_drop_queries_probability", 0, 0, "Allow to ignore drop queries in server with specified probability for testing purposes"},
               {"lightweight_deletes_sync", 2, 2, "The same as 'mutation_sync', but controls only execution of lightweight deletes"},
               {"query_cache_system_table_handling", "save", "throw", "The query cache no longer caches results of queries against system tables"},
               }},

src/Interpreters/InterpreterDropQuery.cpp

@@ -116,7 +116,7 @@ BlockIO InterpreterDropQuery::executeToTable(ASTDropQuery & query)
     return res;
 }
 
-BlockIO InterpreterDropQuery::executeToTableImpl(ContextPtr context_, ASTDropQuery & query, DatabasePtr & db, UUID & uuid_to_wait)
+BlockIO InterpreterDropQuery::executeToTableImpl(const ContextPtr & context_, ASTDropQuery & query, DatabasePtr & db, UUID & uuid_to_wait)
 {
     /// NOTE: it does not contain UUID, we will resolve it with locked DDLGuard
     auto table_id = StorageID(query);
@@ -163,6 +163,19 @@ BlockIO InterpreterDropQuery::executeToTableImpl(ContextPtr context_, ASTDropQue
                 "Table {} is not a Dictionary",
                 table_id.getNameForLogs());
 
+        if (settings.ignore_drop_queries_probability != 0 && ast_drop_query.kind == ASTDropQuery::Kind::Drop && std::uniform_real_distribution<>(0.0, 1.0)(thread_local_rng) <= settings.ignore_drop_queries_probability)
+        {
+            ast_drop_query.sync = false;
+            if (table->storesDataOnDisk())
+            {
+                LOG_TEST(getLogger("InterpreterDropQuery"), "Ignore DROP TABLE query for table {}.{}", table_id.database_name, table_id.table_name);
+                return {};
+            }
+
+            LOG_TEST(getLogger("InterpreterDropQuery"), "Replace DROP TABLE query to TRUNCATE TABLE for table {}.{}", table_id.database_name, table_id.table_name);
+            ast_drop_query.kind = ASTDropQuery::Truncate;
+        }
+
         /// Now get UUID, so we can wait for table data to be finally dropped
         table_id.uuid = database->tryGetTableUUID(table_id.table_name);
 

src/Interpreters/InterpreterDropQuery.h

@@ -41,7 +41,7 @@ class InterpreterDropQuery : public IInterpreter, WithMutableContext
     BlockIO executeToDatabaseImpl(const ASTDropQuery & query, DatabasePtr & database, std::vector<UUID> & uuids_to_wait);
 
     BlockIO executeToTable(ASTDropQuery & query);
-    BlockIO executeToTableImpl(ContextPtr context_, ASTDropQuery & query, DatabasePtr & db, UUID & uuid_to_wait);
+    BlockIO executeToTableImpl(const ContextPtr& context_, ASTDropQuery & query, DatabasePtr & db, UUID & uuid_to_wait);
 
     static void waitForTableToBeActuallyDroppedOrDetached(const ASTDropQuery & query, const DatabasePtr & db, const UUID & uuid_to_wait);
 

tests/ci/stress.py

@@ -71,6 +71,11 @@ def get_options(i: int, upgrade_check: bool) -> str:
     if random.random() < 0.3:
         client_options.append(f"http_make_head_request={random.randint(0, 1)}")
 
+    # TODO: After release 24.3 use ignore_drop_queries_probability for both
+    #       stress test and upgrade check
+    if not upgrade_check:
+        client_options.append("ignore_drop_queries_probability=0.5")
+
     if client_options:
         options.append(" --client-option " + " ".join(client_options))
 

tests/clickhouse-test

@@ -3175,6 +3175,8 @@ def parse_args():
         help="Do not run shard related tests",
     )
 
+    # TODO: Remove upgrade-check option after release 24.3 and use
+    #       ignore_drop_queries_probability option in stress.py as in stress tests
     group.add_argument(
         "--upgrade-check",
         action="store_true",

tests/queries/0_stateless/03013_ignore_drop_queries_probability.reference

@@ -0,0 +1,2 @@
+42
+42

tests/queries/0_stateless/03013_ignore_drop_queries_probability.sql

@@ -0,0 +1,18 @@
+create table test_memory (number UInt64) engine=Memory;
+insert into test_memory select 42;
+drop table test_memory settings ignore_drop_queries_probability=1;
+select * from test_memory;
+drop table test_memory;
+
+create table test_merge_tree (number UInt64) engine=MergeTree order by number;
+insert into test_merge_tree select 42;
+drop table test_merge_tree settings ignore_drop_queries_probability=1;
+select * from test_merge_tree;
+drop table test_merge_tree;
+
+create table test_join (number UInt64) engine=Join(ALL, LEFT, number);
+insert into test_join select 42;
+drop table test_join settings ignore_drop_queries_probability=1;
+select * from test_join;
+drop table test_join;
+