Merge pull request #31013 from vitlibar/no-sessions-in-clickhouse-dictionary-source

Disable using session log for the local session inside Clickhouse dictionary source
This commit is contained in:
Vitaly Baranov 2021-11-04 18:04:22 +03:00 committed by GitHub
commit 78e9af8f0a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 28 additions and 27 deletions

View File

@ -65,14 +65,12 @@ ClickHouseDictionarySource::ClickHouseDictionarySource(
const DictionaryStructure & dict_struct_,
const Configuration & configuration_,
const Block & sample_block_,
ContextMutablePtr context_,
std::shared_ptr<Session> local_session_)
ContextMutablePtr context_)
: update_time{std::chrono::system_clock::from_time_t(0)}
, dict_struct{dict_struct_}
, configuration{configuration_}
, query_builder{dict_struct, configuration.db, "", configuration.table, configuration.query, configuration.where, IdentifierQuotingStyle::Backticks}
, sample_block{sample_block_}
, local_session(local_session_)
, context(context_)
, pool{createPool(configuration)}
, load_all_query{query_builder.composeLoadAllQuery()}
@ -86,7 +84,6 @@ ClickHouseDictionarySource::ClickHouseDictionarySource(const ClickHouseDictionar
, invalidate_query_response{other.invalidate_query_response}
, query_builder{dict_struct, configuration.db, "", configuration.table, configuration.query, configuration.where, IdentifierQuotingStyle::Backticks}
, sample_block{other.sample_block}
, local_session(other.local_session)
, context(Context::createCopy(other.context))
, pool{createPool(configuration)}
, load_all_query{other.load_all_query}
@ -252,17 +249,18 @@ void registerDictionarySourceClickHouse(DictionarySourceFactory & factory)
};
ContextMutablePtr context;
std::shared_ptr<Session> local_session;
if (configuration.is_local)
{
/// Start local session in case when the dictionary is loaded in-process (without TCP communication).
local_session = std::make_shared<Session>(global_context, ClientInfo::Interface::LOCAL);
local_session->authenticate(configuration.user, configuration.password, {});
context = local_session->makeQueryContext();
context->applySettingsChanges(readSettingsFromDictionaryConfig(config, config_prefix));
/// We should set user info even for the case when the dictionary is loaded in-process (without TCP communication).
Session session(global_context, ClientInfo::Interface::LOCAL);
session.authenticate(configuration.user, configuration.password, {});
context = session.makeQueryContext();
}
else
context = copyContextAndApplySettingsFromDictionaryConfig(global_context, config, config_prefix);
{
context = Context::createCopy(global_context);
}
context->applySettingsChanges(readSettingsFromDictionaryConfig(config, config_prefix));
String dictionary_name = config.getString(".dictionary.name", "");
String dictionary_database = config.getString(".dictionary.database", "");
@ -270,7 +268,7 @@ void registerDictionarySourceClickHouse(DictionarySourceFactory & factory)
if (dictionary_name == configuration.table && dictionary_database == configuration.db)
throw Exception(ErrorCodes::BAD_ARGUMENTS, "ClickHouseDictionarySource table cannot be dictionary table");
return std::make_unique<ClickHouseDictionarySource>(dict_struct, configuration, sample_block, context, local_session);
return std::make_unique<ClickHouseDictionarySource>(dict_struct, configuration, sample_block, context);
};
factory.registerSource("clickhouse", create_table_source);

View File

@ -39,8 +39,7 @@ public:
const DictionaryStructure & dict_struct_,
const Configuration & configuration_,
const Block & sample_block_,
ContextMutablePtr context_,
std::shared_ptr<Session> local_session_);
ContextMutablePtr context_);
/// copy-constructor is provided in order to support cloneability
ClickHouseDictionarySource(const ClickHouseDictionarySource & other);
@ -82,7 +81,6 @@ private:
mutable std::string invalidate_query_response;
ExternalQueryBuilder query_builder;
Block sample_block;
std::shared_ptr<Session> local_session;
ContextMutablePtr context;
ConnectionPoolWithFailoverPtr pool;
const std::string load_all_query;

View File

@ -381,7 +381,6 @@ public:
/// Sets the current user assuming that he/she is already authenticated.
/// WARNING: This function doesn't check password!
/// Normally you shouldn't call this function. Use the Session class to do authentication instead.
void setUser(const UUID & user_id_);
UserPtr getUser() const;

View File

@ -246,6 +246,7 @@ void Session::shutdownNamedSessions()
Session::Session(const ContextPtr & global_context_, ClientInfo::Interface interface_)
: auth_id(UUIDHelpers::generateV4()),
global_context(global_context_),
interface(interface_),
log(&Poco::Logger::get(String{magic_enum::enum_name(interface_)} + "-Session"))
{
prepared_client_info.emplace();
@ -418,6 +419,11 @@ ContextMutablePtr Session::makeQueryContext(ClientInfo && query_client_info) con
std::shared_ptr<SessionLog> Session::getSessionLog() const
{
/// For the LOCAL interface we don't send events to the session log
/// because the LOCAL interface is internal, it does nothing with networking.
if (interface == ClientInfo::Interface::LOCAL)
return nullptr;
// take it from global context, since it outlives the Session and always available.
// please note that server may have session_log disabled, hence this may return nullptr.
return global_context->getSessionLog();

View File

@ -79,6 +79,7 @@ private:
mutable bool notified_session_log_about_login = false;
const UUID auth_id;
const ContextPtr global_context;
const ClientInfo::Interface interface;
/// ClientInfo that will be copied to a session context when it's created.
std::optional<ClientInfo> prepared_client_info;

View File

@ -9,7 +9,7 @@
<password></password>
<db>dict</db>
<table>dep_y</table>
<invalidate_query>SELECT intDiv(count(), 5) from dict.dep_y</invalidate_query>
<invalidate_query>SELECT intDiv(count(), 4) from dict.dep_y</invalidate_query>
</clickhouse>
</source>
<!-- ExternalLoader::PeriodicUpdater::check_period_sec=5 anyway -->

View File

@ -59,20 +59,19 @@ def test_get_data(started_cluster):
query("INSERT INTO test.elements VALUES (3, 'fire', 30, 8)")
# Wait for dictionaries to be reloaded.
assert_eq_with_retry(instance, "SELECT dictHas('dep_y', toUInt64(3))", "1", sleep_time=2, retry_count=10)
assert query("SELECT dictGetString('dep_x', 'a', toUInt64(3))") == "XX\n"
assert query("SELECT dictGetString('dep_y', 'a', toUInt64(3))") == "fire\n"
assert query("SELECT dictGetString('dep_z', 'a', toUInt64(3))") == "ZZ\n"
# dep_x and dep_z are updated only when there `intDiv(count(), 5)` is changed.
query("INSERT INTO test.elements VALUES (4, 'ether', 404, 0.001)")
assert_eq_with_retry(instance, "SELECT dictHas('dep_x', toUInt64(4))", "1", sleep_time=2, retry_count=10)
assert_eq_with_retry(instance, "SELECT dictHas('dep_x', toUInt64(3))", "1", sleep_time=2, retry_count=10)
assert query("SELECT dictGetString('dep_x', 'a', toUInt64(3))") == "fire\n"
assert query("SELECT dictGetString('dep_y', 'a', toUInt64(3))") == "fire\n"
assert query("SELECT dictGetString('dep_z', 'a', toUInt64(3))") == "fire\n"
assert query("SELECT dictGetString('dep_x', 'a', toUInt64(4))") == "ether\n"
# dep_z (and hence dep_x) are updated only when there `intDiv(count(), 4)` is changed, now `count()==4`,
# so dep_x and dep_z are not going to be updated after the following INSERT.
query("INSERT INTO test.elements VALUES (4, 'ether', 404, 0.001)")
assert_eq_with_retry(instance, "SELECT dictHas('dep_y', toUInt64(4))", "1", sleep_time=2, retry_count=10)
assert query("SELECT dictGetString('dep_x', 'a', toUInt64(4))") == "XX\n"
assert query("SELECT dictGetString('dep_y', 'a', toUInt64(4))") == "ether\n"
assert query("SELECT dictGetString('dep_z', 'a', toUInt64(4))") == "ether\n"
assert query("SELECT dictGetString('dep_z', 'a', toUInt64(4))") == "ZZ\n"
def dependent_tables_assert():
res = instance.query("select database || '.' || name from system.tables")