From 22da0420affcfd7854e9b5037c8de22bbe81fc2c Mon Sep 17 00:00:00 2001 From: Anton Popov Date: Wed, 19 Oct 2022 20:26:16 +0000 Subject: [PATCH 01/55] fix possible restart errors after failed quorum insert --- .../MergeTree/ReplicatedMergeTreeSink.cpp | 10 +++++++++- ..._manual_write_to_replicas_quorum.reference | 20 +++++++++---------- .../01459_manual_write_to_replicas_quorum.sh | 20 +++++++++++++++++-- 3 files changed, 37 insertions(+), 13 deletions(-) diff --git a/src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp b/src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp index 0abea5977c3..feb3d9c3e9e 100644 --- a/src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp +++ b/src/Storages/MergeTree/ReplicatedMergeTreeSink.cpp @@ -576,7 +576,15 @@ void ReplicatedMergeTreeSink::commitPart( else if (multi_code == Coordination::Error::ZNODEEXISTS && failed_op_path == quorum_info.status_path) { storage.unlockSharedData(*part); - transaction.rollback(); + + /// Part was not committed to keeper. + /// So make it temporary and remove immediately to avoid its resurrection after restart. + transaction.rollbackPartsToTemporaryState(); + + part->is_temp = true; + part->renameTo(temporary_part_relative_path, false, builder); + builder->commit(); + throw Exception("Another quorum insert has been already started", ErrorCodes::UNSATISFIED_QUORUM_FOR_PREVIOUS_WRITE); } else diff --git a/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.reference b/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.reference index 52dea650ebc..812fa4477cb 100644 --- a/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.reference +++ b/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.reference @@ -1,10 +1,10 @@ -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 -100 0 99 4950 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 +200 0 199 19900 diff --git a/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.sh b/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.sh index 6eabc9ae1b5..5a666454531 100755 --- a/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.sh +++ b/tests/queries/0_stateless/01459_manual_write_to_replicas_quorum.sh @@ -19,7 +19,7 @@ done valid_exceptions_to_retry='Quorum for previous write has not been satisfied yet|Another quorum insert has been already started|Unexpected logical error while adding block' -function thread { +function thread1 { for x in {0..99}; do while true; do $CLICKHOUSE_CLIENT --insert_quorum 5 --insert_quorum_parallel 0 --query "INSERT INTO r$1 SELECT $x" 2>&1 | grep -qE "$valid_exceptions_to_retry" || break @@ -28,7 +28,23 @@ function thread { } for i in $(seq 1 $NUM_REPLICAS); do - thread $i & + thread1 $i & +done + +wait + +function thread2 { + for x in {100..199}; do + while true; do + $CLICKHOUSE_CLIENT --query "DETACH TABLE r$1" + $CLICKHOUSE_CLIENT --query "ATTACH TABLE r$1" + $CLICKHOUSE_CLIENT --insert_quorum 5 --insert_quorum_parallel 0 --query "INSERT INTO r$1 SELECT $x" 2>&1 | grep -qE "$valid_exceptions_to_retry" || break + done + done +} + +for i in $(seq 1 $NUM_REPLICAS); do + thread2 $i & done wait From 0a303433fcc99ea7ef7286d34db86e2fd61792d4 Mon Sep 17 00:00:00 2001 From: lzydmxy <13126752315@163.com> Date: Thu, 15 Dec 2022 17:06:09 +0800 Subject: [PATCH 02/55] make no limits on the maximum size of the result for the view --- src/Storages/StorageView.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Storages/StorageView.cpp b/src/Storages/StorageView.cpp index a55d7ad3c09..a43fe96396b 100644 --- a/src/Storages/StorageView.cpp +++ b/src/Storages/StorageView.cpp @@ -81,6 +81,20 @@ bool hasJoin(const ASTSelectWithUnionQuery & ast) return false; } +/** There are no limits on the maximum size of the result for the view. + * Since the result of the view is not the result of the entire query. + */ +ContextPtr getViewContext(ContextPtr context) +{ + auto view_context = Context::createCopy(context); + Settings view_settings = context->getSettings(); + view_settings.max_result_rows = 0; + view_settings.max_result_bytes = 0; + view_settings.extremes = false; + view_context->setSettings(view_settings); + return view_context; +} + } StorageView::StorageView( @@ -123,7 +137,7 @@ void StorageView::read( } auto options = SelectQueryOptions(QueryProcessingStage::Complete, 0, false, query_info.settings_limit_offset_done); - InterpreterSelectWithUnionQuery interpreter(current_inner_query, context, options, column_names); + InterpreterSelectWithUnionQuery interpreter(current_inner_query, getViewContext(context), options, column_names); interpreter.addStorageLimits(*query_info.storage_limits); interpreter.buildQueryPlan(query_plan); From 5d35b19e7e65ca96824c6ad3f3929d9bdc495170 Mon Sep 17 00:00:00 2001 From: lzydmxy <13126752315@163.com> Date: Sat, 17 Dec 2022 00:47:05 +0800 Subject: [PATCH 03/55] Add tests --- .../02501_limits_on_result_for_view.reference | 1 + .../02501_limits_on_result_for_view.sql | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/queries/0_stateless/02501_limits_on_result_for_view.reference create mode 100644 tests/queries/0_stateless/02501_limits_on_result_for_view.sql diff --git a/tests/queries/0_stateless/02501_limits_on_result_for_view.reference b/tests/queries/0_stateless/02501_limits_on_result_for_view.reference new file mode 100644 index 00000000000..0691f67b202 --- /dev/null +++ b/tests/queries/0_stateless/02501_limits_on_result_for_view.reference @@ -0,0 +1 @@ +52 diff --git a/tests/queries/0_stateless/02501_limits_on_result_for_view.sql b/tests/queries/0_stateless/02501_limits_on_result_for_view.sql new file mode 100644 index 00000000000..17e6024d973 --- /dev/null +++ b/tests/queries/0_stateless/02501_limits_on_result_for_view.sql @@ -0,0 +1,25 @@ +DROP TABLE IF EXISTS 02501_test; +DROP TABLE IF EXISTS 02501_dist; +DROP VIEW IF EXISTS 02501_view; + + +-- create local table +CREATE TABLE 02501_test(`a` UInt64) ENGINE = Memory; + +-- create dist table +CREATE TABLE 02501_dist(`a` UInt64) ENGINE = Distributed(test_cluster_two_shards, currentDatabase(), 02501_test); + +-- create view +CREATE VIEW 02501_view(`a` UInt64) AS SELECT a FROM 02501_dist; + +-- insert data +insert into 02501_test values(5),(6),(7),(8); + +-- test +SELECT * from 02501_view settings max_result_rows = 1; -- { serverError 396 } +SELECT sum(a) from 02501_view settings max_result_rows = 1; + + +DROP TABLE IF EXISTS 02501_test; +DROP TABLE IF EXISTS 02501_dist; +DROP VIEW IF EXISTS 02501_view; \ No newline at end of file From 4bc76dfdb4fbedbd2f35ccb99baf89fafef78a35 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 18 Dec 2022 06:20:56 +0100 Subject: [PATCH 04/55] Add a test for #36038 --- .../02504_parse_datetime_best_effort_calebeaires.reference | 1 + .../02504_parse_datetime_best_effort_calebeaires.sql | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.reference create mode 100644 tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.sql diff --git a/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.reference b/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.reference new file mode 100644 index 00000000000..f5420eeb6ba --- /dev/null +++ b/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.reference @@ -0,0 +1 @@ +('Date','2148-06-07') ('Date32','1969-01-01') ('DateTime','2105-02-07 17:10:16') ('DateTime','2105-02-07 17:10:16') ('DateTime64(3)','1969-01-01 10:42:00.000') diff --git a/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.sql b/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.sql new file mode 100644 index 00000000000..74b974d7440 --- /dev/null +++ b/tests/queries/0_stateless/02504_parse_datetime_best_effort_calebeaires.sql @@ -0,0 +1,5 @@ +CREATE TEMPORARY TABLE my_table (col_date Date, col_date32 Date32, col_datetime DateTime, col_datetime32 DateTime32, col_datetime64 DateTime64); +insert into `my_table` (`col_date`, `col_date32`, `col_datetime`, `col_datetime32`, `col_datetime64`) values (parseDateTime64BestEffort('1969-01-01'), '1969-01-01', parseDateTime64BestEffort('1969-01-01 10:42:00'), parseDateTime64BestEffort('1969-01-01 10:42:00'), parseDateTime64BestEffort('1969-01-01 10:42:00')); + +-- The values for Date32 and DateTime64 will be year 1969, while the values of Date, DateTime will contain a value affected by implementation-defined overflow and can be arbitrary. +SELECT * APPLY(x -> (toTypeName(x), x)) FROM my_table; From 975906361e9cbb086f1b901a77392d5930db884e Mon Sep 17 00:00:00 2001 From: save-my-heart Date: Tue, 20 Dec 2022 12:51:42 +0800 Subject: [PATCH 05/55] fix explain ast insert with data --- src/Parsers/parseQuery.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Parsers/parseQuery.cpp b/src/Parsers/parseQuery.cpp index 4a0c60da48d..da8450ac301 100644 --- a/src/Parsers/parseQuery.cpp +++ b/src/Parsers/parseQuery.cpp @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -263,7 +264,19 @@ ASTPtr tryParseQuery( ASTInsertQuery * insert = nullptr; if (parse_res) - insert = res->as(); + { + if (auto * explain = res->as()) + { + if (auto explained_query = explain->getExplainedQuery()) + { + insert = explained_query->as(); + } + } + else + { + insert = res->as(); + } + } // If parsed query ends at data for insertion. Data for insertion could be // in any format and not necessary be lexical correct, so we can't perform From 6858f3fc01b2e45fe2213bfd528d6bdeeb3630e3 Mon Sep 17 00:00:00 2001 From: save-my-heart Date: Thu, 22 Dec 2022 23:00:54 +0800 Subject: [PATCH 06/55] add test --- tests/queries/0_stateless/02504_explain_ast_insert.reference | 4 ++++ tests/queries/0_stateless/02504_explain_ast_insert.sql | 2 ++ 2 files changed, 6 insertions(+) create mode 100644 tests/queries/0_stateless/02504_explain_ast_insert.reference create mode 100644 tests/queries/0_stateless/02504_explain_ast_insert.sql diff --git a/tests/queries/0_stateless/02504_explain_ast_insert.reference b/tests/queries/0_stateless/02504_explain_ast_insert.reference new file mode 100644 index 00000000000..1c149a0f2f4 --- /dev/null +++ b/tests/queries/0_stateless/02504_explain_ast_insert.reference @@ -0,0 +1,4 @@ +InsertQuery (children 1) + Identifier test +InsertQuery (children 1) + Identifier test diff --git a/tests/queries/0_stateless/02504_explain_ast_insert.sql b/tests/queries/0_stateless/02504_explain_ast_insert.sql new file mode 100644 index 00000000000..fc50feebaa4 --- /dev/null +++ b/tests/queries/0_stateless/02504_explain_ast_insert.sql @@ -0,0 +1,2 @@ +explain ast insert into test values balabala; +explain ast insert into test format TabSeparated balabala; \ No newline at end of file From 91c1f9de8f115686bbc1b46e91e6d1ef15788fec Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Mon, 26 Dec 2022 18:13:50 +0100 Subject: [PATCH 07/55] Faster server startup after stress test --- docker/test/stress/run.sh | 3 +++ src/Common/ZooKeeper/ZooKeeperImpl.cpp | 1 - tests/config/config.d/zookeeper_fault_injection.xml | 5 ----- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/docker/test/stress/run.sh b/docker/test/stress/run.sh index 984f35d5f27..021da2f6875 100644 --- a/docker/test/stress/run.sh +++ b/docker/test/stress/run.sh @@ -337,6 +337,9 @@ zgrep -Fa " received signal " /test_output/gdb.log > /dev/null \ if [ "$DISABLE_BC_CHECK" -ne "1" ]; then echo -e "Backward compatibility check\n" + # Disable ZooKeeper fault injection, because it is harmful for the backward compatibility check. + sudo rm /etc/clickhouse-server/config.d/zookeeper_fault_injection.xml + echo "Get previous release tag" previous_release_tag=$(clickhouse-client --version | grep -o "[0-9]*\.[0-9]*\.[0-9]*\.[0-9]*" | get_previous_release_tag) echo $previous_release_tag diff --git a/src/Common/ZooKeeper/ZooKeeperImpl.cpp b/src/Common/ZooKeeper/ZooKeeperImpl.cpp index 7cbe7d7b0f2..74d06b392f9 100644 --- a/src/Common/ZooKeeper/ZooKeeperImpl.cpp +++ b/src/Common/ZooKeeper/ZooKeeperImpl.cpp @@ -342,7 +342,6 @@ ZooKeeper::ZooKeeper( default_acls.emplace_back(std::move(acl)); } - /// It makes sense (especially, for async requests) to inject a fault in two places: /// pushRequest (before request is sent) and receiveEvent (after request was executed). if (0 < args.send_fault_probability && args.send_fault_probability <= 1) diff --git a/tests/config/config.d/zookeeper_fault_injection.xml b/tests/config/config.d/zookeeper_fault_injection.xml index 45d3cc8193d..ca9dbed0576 100644 --- a/tests/config/config.d/zookeeper_fault_injection.xml +++ b/tests/config/config.d/zookeeper_fault_injection.xml @@ -1,10 +1,5 @@ - - localhost - 9181 - -