Skip to content

Commit

Permalink
Merge pull request ClickHouse#16047 from ClickHouse/suggest-destructi…
Browse files Browse the repository at this point in the history
…on-order

Fix destruction order of Suggest ClickHouse#16035
  • Loading branch information
alexey-milovidov authored Oct 29, 2020
2 parents 0faf2bc + 690a3b4 commit ae4d66a
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 12 deletions.
9 changes: 6 additions & 3 deletions programs/client/Client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,8 @@ class Client : public Poco::Util::Application
QueryFuzzer fuzzer;
int query_fuzzer_runs = 0;

std::optional<Suggest> suggest;

/// We will format query_id in interactive mode in various ways, the default is just to print Query id: ...
std::vector<std::pair<String, String>> query_id_formats;

Expand Down Expand Up @@ -577,10 +579,11 @@ class Client : public Poco::Util::Application
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.
Expand All @@ -607,15 +610,15 @@ class Client : public Poco::Util::Application
highlight_callback = highlight;

ReplxxLineReader lr(
Suggest::instance(),
*suggest,
history_file,
config().has("multiline"),
query_extenders,
query_delimiters,
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
Expand Down
13 changes: 4 additions & 9 deletions programs/client/Suggest.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -30,12 +31,6 @@ class Suggest : public LineReader::Suggest, boost::noncopyable
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);
Expand Down
12 changes: 12 additions & 0 deletions tests/queries/0_stateless/01526_client_start_and_exit.expect
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/usr/bin/expect -f

log_user 1
set timeout 5
match_max 100000

if ![info exists env(CLICKHOUSE_PORT_TCP)] {set env(CLICKHOUSE_PORT_TCP) 9000}

spawn bash -c "clickhouse-client --port $env(CLICKHOUSE_PORT_TCP) && echo $?"
expect ":) "
send -- "\4"
expect eof
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Loaded 10000 queries.
23 changes: 23 additions & 0 deletions tests/queries/0_stateless/01526_client_start_and_exit.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#!/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
${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 | grep -v -P 'ClickHouse client|Connecting|Connected|:\) Bye\.|^\s*$|spawn bash|^0\s*$'
done
}

export CURDIR
export -f stress

for _ in {1..10}; do
timeout 3 bash -c stress &
done

wait

0 comments on commit ae4d66a

Please sign in to comment.