From 4bf32cb15bf78827bd86c1310741ebe005acc8b9 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 16 Oct 2020 02:46:36 +0300 Subject: [PATCH 1/6] Fix destruction order of Suggest #16035 --- programs/client/Client.cpp | 9 ++++++--- programs/client/Suggest.h | 13 ++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/programs/client/Client.cpp b/programs/client/Client.cpp index 3cd584c0e55..8ccdfd8af65 100644 --- a/programs/client/Client.cpp +++ b/programs/client/Client.cpp @@ -218,6 +218,8 @@ private: QueryFuzzer fuzzer; int query_fuzzer_runs = 0; + std::optional suggest; + /// We will format query_id in interactive mode in various ways, the default is just to print Query id: ... std::vector> query_id_formats; @@ -577,10 +579,11 @@ private: if (print_time_to_stderr) throw Exception("time option could be specified only in non-interactive mode", ErrorCodes::BAD_ARGUMENTS); + suggest.emplace(); if (server_revision >= Suggest::MIN_SERVER_REVISION && !config().getBool("disable_suggestion", false)) { /// Load suggestion data from the server. - Suggest::instance().load(connection_parameters, config().getInt("suggestion_limit")); + suggest->load(connection_parameters, config().getInt("suggestion_limit")); } /// Load command history if present. @@ -607,7 +610,7 @@ private: highlight_callback = highlight; ReplxxLineReader lr( - Suggest::instance(), + *suggest, history_file, config().has("multiline"), query_extenders, @@ -615,7 +618,7 @@ private: highlight_callback); #elif defined(USE_READLINE) && USE_READLINE - ReadlineLineReader lr(Suggest::instance(), history_file, config().has("multiline"), query_extenders, query_delimiters); + ReadlineLineReader lr(*suggest, history_file, config().has("multiline"), query_extenders, query_delimiters); #else LineReader lr(history_file, config().has("multiline"), query_extenders, query_delimiters); #endif diff --git a/programs/client/Suggest.h b/programs/client/Suggest.h index b13289ac322..03332088cbe 100644 --- a/programs/client/Suggest.h +++ b/programs/client/Suggest.h @@ -18,10 +18,11 @@ namespace ErrorCodes class Suggest : public LineReader::Suggest, boost::noncopyable { public: - static Suggest & instance() + Suggest(); + ~Suggest() { - static Suggest instance; - return instance; + if (loading_thread.joinable()) + loading_thread.join(); } void load(const ConnectionParameters & connection_parameters, size_t suggestion_limit); @@ -30,12 +31,6 @@ public: static constexpr int MIN_SERVER_REVISION = 54406; private: - Suggest(); - ~Suggest() - { - if (loading_thread.joinable()) - loading_thread.join(); - } void loadImpl(Connection & connection, const ConnectionTimeouts & timeouts, size_t suggestion_limit); void fetch(Connection & connection, const ConnectionTimeouts & timeouts, const std::string & query); From 2efd740dc7cbd3159e269eab5e03e1a48fbfb8c6 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 16 Oct 2020 03:07:56 +0300 Subject: [PATCH 2/6] Added a test --- .../01526_client_start_and_exit.reference | 1 + .../01526_client_start_and_exit.sh | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 tests/queries/0_stateless/01526_client_start_and_exit.reference create mode 100755 tests/queries/0_stateless/01526_client_start_and_exit.sh diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.reference b/tests/queries/0_stateless/01526_client_start_and_exit.reference new file mode 100644 index 00000000000..7326d960397 --- /dev/null +++ b/tests/queries/0_stateless/01526_client_start_and_exit.reference @@ -0,0 +1 @@ +Ok diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.sh b/tests/queries/0_stateless/01526_client_start_and_exit.sh new file mode 100755 index 00000000000..d3fd2cb73b2 --- /dev/null +++ b/tests/queries/0_stateless/01526_client_start_and_exit.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash + +CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +. "$CURDIR"/../shell_config.sh + +# Create a huge amount of tables, so Suggest will take a time to load +seq 1 1000 | sed -r -e 's/(.+)/CREATE TABLE IF NOT EXISTS test\1 (x UInt8) ENGINE = Memory;/' | ${CLICKHOUSE_CLIENT} -n + +function stress() +{ + while true; do + ./"$CURDIR"/01526_client_start_and_exit.expect + done +} + +export -f stress + +for _ in {1..10}; do + timeout 3 bash -c stress & +done + +wait +echo 'Ok' + +seq 1 1000 | sed -r -e 's/(.+)/DROP TABLE test\1;/' | ${CLICKHOUSE_CLIENT} -n From 90a3e95aabac19ab4dae30819dc579a88428c297 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Fri, 16 Oct 2020 03:38:32 +0300 Subject: [PATCH 3/6] Add missing file --- .../0_stateless/01526_client_start_and_exit.expect | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100755 tests/queries/0_stateless/01526_client_start_and_exit.expect diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.expect b/tests/queries/0_stateless/01526_client_start_and_exit.expect new file mode 100755 index 00000000000..4ad0eea5d97 --- /dev/null +++ b/tests/queries/0_stateless/01526_client_start_and_exit.expect @@ -0,0 +1,12 @@ +#!/usr/bin/expect -f + +log_user 0 +set timeout 5 +match_max 100000 + +if ![info exists env(CLICKHOUSE_PORT_TCP)] {set env(CLICKHOUSE_PORT_TCP) 9000} + +spawn clickhouse-client --port "$env(CLICKHOUSE_PORT_TCP)" +expect ":) " +send -- "\4" +expect eof From 8cc463f549d29e5c11ea8664e5c17eb05ab50033 Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Sun, 25 Oct 2020 05:31:58 +0300 Subject: [PATCH 4/6] Better test --- .../queries/0_stateless/01526_client_start_and_exit.expect | 4 ++-- .../0_stateless/01526_client_start_and_exit.reference | 2 +- tests/queries/0_stateless/01526_client_start_and_exit.sh | 7 ++----- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.expect b/tests/queries/0_stateless/01526_client_start_and_exit.expect index 4ad0eea5d97..003439ffa54 100755 --- a/tests/queries/0_stateless/01526_client_start_and_exit.expect +++ b/tests/queries/0_stateless/01526_client_start_and_exit.expect @@ -1,12 +1,12 @@ #!/usr/bin/expect -f -log_user 0 +log_user 1 set timeout 5 match_max 100000 if ![info exists env(CLICKHOUSE_PORT_TCP)] {set env(CLICKHOUSE_PORT_TCP) 9000} -spawn clickhouse-client --port "$env(CLICKHOUSE_PORT_TCP)" +spawn bash -c "clickhouse-client --port $env(CLICKHOUSE_PORT_TCP) && echo $?" expect ":) " send -- "\4" expect eof diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.reference b/tests/queries/0_stateless/01526_client_start_and_exit.reference index 7326d960397..e3e2e7b22af 100644 --- a/tests/queries/0_stateless/01526_client_start_and_exit.reference +++ b/tests/queries/0_stateless/01526_client_start_and_exit.reference @@ -1 +1 @@ -Ok +Loaded 10000 queries. diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.sh b/tests/queries/0_stateless/01526_client_start_and_exit.sh index d3fd2cb73b2..abcf51d8c50 100755 --- a/tests/queries/0_stateless/01526_client_start_and_exit.sh +++ b/tests/queries/0_stateless/01526_client_start_and_exit.sh @@ -4,12 +4,12 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$CURDIR"/../shell_config.sh # Create a huge amount of tables, so Suggest will take a time to load -seq 1 1000 | sed -r -e 's/(.+)/CREATE TABLE IF NOT EXISTS test\1 (x UInt8) ENGINE = Memory;/' | ${CLICKHOUSE_CLIENT} -n +${CLICKHOUSE_CLIENT} -q "SELECT 'CREATE TABLE test_' || hex(randomPrintableASCII(40)) || '(x UInt8) Engine=Memory;' FROM numbers(10000)" --format=TSVRaw | ${CLICKHOUSE_BENCHMARK} -c32 -i 10000 -d 0 2>&1 | grep -F 'Loaded 10000 queries' function stress() { while true; do - ./"$CURDIR"/01526_client_start_and_exit.expect + ./"$CURDIR"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$' done } @@ -20,6 +20,3 @@ for _ in {1..10}; do done wait -echo 'Ok' - -seq 1 1000 | sed -r -e 's/(.+)/DROP TABLE test\1;/' | ${CLICKHOUSE_CLIENT} -n From 6762f967687d8f0e504f0c8f42d4fe39beefad9f Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sun, 25 Oct 2020 08:25:21 +0300 Subject: [PATCH 5/6] Update 01526_client_start_and_exit.sh --- tests/queries/0_stateless/01526_client_start_and_exit.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.sh b/tests/queries/0_stateless/01526_client_start_and_exit.sh index abcf51d8c50..0b3a2ee6b4f 100755 --- a/tests/queries/0_stateless/01526_client_start_and_exit.sh +++ b/tests/queries/0_stateless/01526_client_start_and_exit.sh @@ -9,7 +9,7 @@ ${CLICKHOUSE_CLIENT} -q "SELECT 'CREATE TABLE test_' || hex(randomPrintableASCII function stress() { while true; do - ./"$CURDIR"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$' + "$CURDIR"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$' done } From 690a3b431075f1676c67c661633708451e0908cd Mon Sep 17 00:00:00 2001 From: Alexey Milovidov Date: Wed, 28 Oct 2020 04:24:30 +0300 Subject: [PATCH 6/6] Fix test --- tests/queries/0_stateless/01526_client_start_and_exit.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/queries/0_stateless/01526_client_start_and_exit.sh b/tests/queries/0_stateless/01526_client_start_and_exit.sh index 0b3a2ee6b4f..c179be79d03 100755 --- a/tests/queries/0_stateless/01526_client_start_and_exit.sh +++ b/tests/queries/0_stateless/01526_client_start_and_exit.sh @@ -9,10 +9,11 @@ ${CLICKHOUSE_CLIENT} -q "SELECT 'CREATE TABLE test_' || hex(randomPrintableASCII function stress() { while true; do - "$CURDIR"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$' + "${CURDIR}"/01526_client_start_and_exit.expect | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$' done } +export CURDIR export -f stress for _ in {1..10}; do