pFad - Phone/Frame/Anonymizer/Declutterfier! Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

URL: http://github.com/Dwrite/ClickHouse/commit/9c0963c0a8a770856a2252425ea4be6b979150fe

rossorigen="anonymous" media="all" rel="stylesheet" href="https://github.githubassets.com/assets/global-9c8f61f9f58ad7b2.css" /> Do not use global context as main context for the client · Dwrite/ClickHouse@9c0963c · GitHub
Skip to content

Commit 9c0963c

Browse files
azatazat
committed
Do not use global context as main context for the client
The problem is that clietn calls setSettings() in the global settings, but this produce a data-race with accessing global settings all over the places [1]: - ThreadStatus::finalizePerformanceCounters() - ThreadStatus::initGlobalProfiler() [1]: ClickHouse#81893 (comment) v2: Initialize query_id in a proper context v3: Fix apply settings from config on the client v4 + v5: Fix other settings adjustments
1 parent a645dd9 commit 9c0963c

4 files changed

Lines changed: 17 additions & 30 deletions

File tree

‎programs/client/Client.cpp‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -325,20 +325,20 @@ void Client::initialize(Poco::Util::Application & self)
325325
config().setString("password", env_password);
326326

327327
/// settings and limits could be specified in config file, but passed settings has higher priority
328-
for (const auto & setting : global_context->getSettingsRef().getUnchangedNames())
328+
for (const auto & setting : client_context->getSettingsRef().getUnchangedNames())
329329
{
330330
String name{setting};
331331
if (config().has(name))
332-
global_context->setSetting(name, config().getString(name));
332+
client_context->setSetting(name, config().getString(name));
333333
}
334334

335335
/// Set path for format schema files
336336
if (config().has("format_schema_path"))
337-
global_context->setFormatSchemaPath(fs::weakly_canonical(config().getString("format_schema_path")));
337+
client_context->setFormatSchemaPath(fs::weakly_canonical(config().getString("format_schema_path")));
338338

339339
/// Set the path for google proto files
340340
if (config().has("google_protos_path"))
341-
global_context->setGoogleProtosPath(fs::weakly_canonical(config().getString("google_protos_path")));
341+
client_context->setGoogleProtosPath(fs::weakly_canonical(config().getString("google_protos_path")));
342342
}
343343

344344

@@ -358,7 +358,8 @@ try
358358
registerAggregateFunctions();
359359

360360
processConfig();
361-
adjustSettings();
361+
adjustSettings(client_context);
362+
362363
initTTYBuffer(
363364
toProgressOption(config().getString("progress", "default")), toProgressOption(config().getString("progress-table", "default")));
364365
initKeystrokeInterceptor();
@@ -886,9 +887,6 @@ void Client::processOptions(
886887
if (options.count("opentelemetry-tracestate"))
887888
global_context->getClientTraceContext().tracestate = options["opentelemetry-tracestate"].as<std::string>();
888889

889-
/// In case of clickhouse-client the `client_context` can be just an alias for the `global_context`.
890-
/// (There is no need to copy the context because clickhouse-client has no background tasks so it won't use that context in parallel.)
891-
client_context = global_context;
892890
initClientContext();
893891

894892
/// Allow to pass-through unknown settings to the server.
@@ -921,7 +919,7 @@ void Client::processConfig()
921919

922920
query_id = config().getString("query_id", "");
923921
if (!query_id.empty())
924-
global_context->setCurrentQueryId(query_id);
922+
client_context->setCurrentQueryId(query_id);
925923
}
926924

927925
if (is_interactive || delayed_interactive)

‎programs/local/LocalServer.cpp‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ try
643643
/// Must be called after we stopped initializing the global context and changing its settings.
644644
/// After this point the global context must be stayed almost unchanged till shutdown,
645645
/// and all necessary changes must be made to the client context instead.
646-
createClientContext();
646+
initClientContext();
647647

648648
if (is_interactive)
649649
{
@@ -902,7 +902,7 @@ void LocalServer::processConfig()
902902
/// NOTE: it is important to apply any overrides before
903903
/// `setDefaultProfiles` calls since it will copy current context (i.e.
904904
/// there is separate context for Buffer tables).
905-
adjustSettings();
905+
adjustSettings(global_context);
906906
applySettingsOverridesForLocal(global_context);
907907
applyCmdOptions(global_context);
908908

@@ -1080,16 +1080,6 @@ void LocalServer::applyCmdOptions(ContextMutablePtr context)
10801080
}
10811081

10821082

1083-
void LocalServer::createClientContext()
1084-
{
1085-
/// In case of clickhouse-local it's necessary to use a separate context for client-related purposes.
1086-
/// We can't just change the global context because it is used in background tasks (for example, in merges)
1087-
/// which don't expect that the global context can suddenly change.
1088-
client_context = Context::createCopy(global_context);
1089-
initClientContext();
1090-
}
1091-
1092-
10931083
void LocalServer::processOptions(const OptionsDescription &, const CommandLineOptions & options, const std::vector<Arguments> &, const std::vector<Arguments> &)
10941084
{
10951085
if (options.count("path"))

‎src/Client/ClientBase.cpp‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -813,33 +813,34 @@ void ClientBase::initLogsOutputStream()
813813
}
814814
}
815815

816-
void ClientBase::adjustSettings()
816+
void ClientBase::adjustSettings(ContextMutablePtr context)
817817
{
818-
Settings settings = global_context->getSettingsCopy();
818+
Settings settings = context->getSettingsCopy();
819819

820820
/// NOTE: Do not forget to set changed=false to avoid sending it to the server (to avoid breakage read only profiles)
821821

822822
/// Do not limit pretty format output in case of --pager specified or in case of stdout is not a tty.
823823
if (!pager.empty() || !stdout_is_a_tty)
824824
{
825-
if (!global_context->getSettingsRef()[Setting::output_format_pretty_max_rows].changed)
825+
if (!context->getSettingsRef()[Setting::output_format_pretty_max_rows].changed)
826826
{
827827
settings[Setting::output_format_pretty_max_rows] = std::numeric_limits<UInt64>::max();
828828
settings[Setting::output_format_pretty_max_rows].changed = false;
829829
}
830830

831-
if (!global_context->getSettingsRef()[Setting::output_format_pretty_max_value_width].changed)
831+
if (!context->getSettingsRef()[Setting::output_format_pretty_max_value_width].changed)
832832
{
833833
settings[Setting::output_format_pretty_max_value_width] = std::numeric_limits<UInt64>::max();
834834
settings[Setting::output_format_pretty_max_value_width].changed = false;
835835
}
836836
}
837837

838-
global_context->setSettings(settings);
838+
context->setSettings(settings);
839839
}
840840

841841
void ClientBase::initClientContext()
842842
{
843+
client_context = Context::createCopy(global_context);
843844
client_context->setClientName(std::string(DEFAULT_CLIENT_NAME));
844845
client_context->setQuotaClientKey(getClientConfiguration().getString("quota_key", ""));
845846
client_context->setQueryKindInitial();
@@ -2888,7 +2889,7 @@ void ClientBase::applySettingsFromServerIfNeeded()
28882889
changes_to_apply.push_back(change);
28892890
}
28902891

2891-
global_context->applySettingsChanges(changes_to_apply);
2892+
client_context->applySettingsChanges(changes_to_apply);
28922893
}
28932894

28942895
void ClientBase::startKeystrokeInterceptorIfExists()

‎src/Client/ClientBase.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,7 @@ class ClientBase
271271
static bool isFileDescriptorSuitableForInput(int fd);
272272

273273
/// Adjust some settings after command line options and config had been processed.
274-
void adjustSettings();
274+
void adjustSettings(ContextMutablePtr context);
275275

276276
/// Initializes the client context.
277277
void initClientContext();
@@ -288,8 +288,6 @@ class ClientBase
288288
/// This holder may not be initialized in case if we run the client in the embedded mode (SSH).
289289
SharedContextHolder shared_context;
290290
ContextMutablePtr global_context;
291-
292-
/// Client context is a context used only by the client to parse queries, process query parameters and to connect to clickhouse-server.
293291
ContextMutablePtr client_context;
294292

295293
String default_database;

0 commit comments

Comments
 (0)
pFad - Phonifier reborn

Pfad - The Proxy pFad © 2024 Your Company Name. All rights reserved.




Check this box to remove all script contents from the fetched content.



Check this box to remove all images from the fetched content.


Check this box to remove all CSS styles from the fetched content.


Check this box to keep images inefficiently compressed and original size.

Note: This service is not intended for secure transactions such as banking, social media, email, or purchasing. Use at your own risk. We assume no liability whatsoever for broken pages.


Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy