Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 15 commits into from Apr 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion programs/client/Client.cpp
Expand Up @@ -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")
;
Expand Down Expand Up @@ -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");
Expand Down
4 changes: 0 additions & 4 deletions src/Client/ClientBase.cpp
Expand Up @@ -949,12 +949,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.
Expand Down
2 changes: 0 additions & 2 deletions src/Client/ClientBase.h
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/Core/Settings.h
Expand Up @@ -869,6 +869,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) \
Expand Down
1 change: 1 addition & 0 deletions src/Core/SettingsChangesHistory.h
Expand Up @@ -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"},
}},
{"24.3", {{"s3_connect_timeout_ms", 1000, 1000, "Introduce new dedicated setting for s3 connection timeout"},
Expand Down
15 changes: 14 additions & 1 deletion src/Interpreters/InterpreterDropQuery.cpp
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
2 changes: 1 addition & 1 deletion src/Interpreters/InterpreterDropQuery.h
Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions tests/ci/stress.py
Expand Up @@ -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))

Expand Down
2 changes: 2 additions & 0 deletions tests/clickhouse-test
Expand Up @@ -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",
Expand Down
@@ -0,0 +1,2 @@
42
42
@@ -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;