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 9 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 @@ -949,6 +949,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 @@ -1092,7 +1093,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 @@ -867,6 +867,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) \
\
/** Experimental functions */ \
Expand Down
1 change: 1 addition & 0 deletions src/Core/SettingsChangesHistory.h
Expand Up @@ -87,6 +87,7 @@ static std::map<ClickHouseVersion, SettingsChangesHistory::SettingsChanges> sett
{
{"24.3", {{"s3_connect_timeout_ms", 1000, 1000, "Introduce new dedicated setting for s3 connection timeout"},
{"allow_experimental_shared_merge_tree", false, true, "The setting is obsolete"},
{"ignore_drop_queries_probability", 0, 0, "Allow to ignore drop queries in server with specified probability for testing purposes"},
{"use_page_cache_for_disks_without_file_cache", false, false, "Added userspace page cache"},
{"read_from_page_cache_if_exists_otherwise_bypass_cache", false, false, "Added userspace page cache"},
{"page_cache_inject_eviction", false, false, "Added userspace page cache"},
Expand Down
11 changes: 10 additions & 1 deletion src/Interpreters/InterpreterDropQuery.cpp
Expand Up @@ -105,7 +105,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 @@ -152,6 +152,15 @@ 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->getName() != "Memory" && table->getName() != "Join")
Avogar marked this conversation as resolved.
Show resolved Hide resolved
return {};

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 @@ -37,7 +37,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 @@ -3172,6 +3172,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 @@
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;