thevar1able/ClickHouse
1
0
Fork 0
mirror of https://github.com/ClickHouse/ClickHouse.git synced 2024-11-26 17:41:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

Improvements based on review

Browse Source
This commit is contained in:
Raúl Marín 2024-04-29 20:46:37 +02:00
parent 30d10bf242
commit 987558e18d
5 changed files with 26 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/Core/SettingsQuirks.cpp
View File

@ -90,7 +90,7 @@ void applySettingsQuirks(Settings & settings, LoggerPtr log)
}
}
void doSettingsSanityCheckClamp(Settings & current_settings)
void doSettingsSanityCheckClamp(Settings & current_settings, LoggerPtr log)
{
auto getCurrentValue = [&current_settings](const std::string_view name) -> Field
{
@ -101,9 +101,13 @@ void doSettingsSanityCheckClamp(Settings & current_settings)
};
UInt64 max_threads = getCurrentValue("max_threads").get<UInt64>();
UInt64 max_threads_max_value = 256 * getNumberOfPhysicalCPUCores();
UInt64 max_threads_max_value = 65536 * getNumberOfPhysicalCPUCores();
if (max_threads > max_threads_max_value)
{
if (log)
LOG_WARNING(log, "Sanity check: Too many threads requested ({}). Reduced to {}", max_threads, max_threads_max_value);
current_settings.set("max_threads", max_threads_max_value);
}
constexpr UInt64 max_sane_block_rows_size = 4294967296; // 2^32
std::unordered_set<String> block_rows_settings{
@ -118,7 +122,11 @@ void doSettingsSanityCheckClamp(Settings & current_settings)
{
auto block_size = getCurrentValue(setting).get<UInt64>();
if (block_size > max_sane_block_rows_size)
{
if (log)
LOG_WARNING(log, "Sanity check: '{}' value is too high ({}). Reduced to {}", setting, block_size, max_sane_block_rows_size);
current_settings.set(setting, max_sane_block_rows_size);
}
}
}
}

2
src/Core/SettingsQuirks.h
View File

@ -11,5 +11,5 @@ struct Settings;
void applySettingsQuirks(Settings & settings, LoggerPtr log = nullptr);
/// Verify that some settings have sane values. Alters the value to a reasonable one if not
void doSettingsSanityCheckClamp(Settings & settings);
void doSettingsSanityCheckClamp(Settings & settings, LoggerPtr log);
}

18
src/Interpreters/Context.cpp
View File

@ -1378,14 +1378,14 @@ contextSanityClampSettingsWithLock(const Context & context, Settings & settings,
{
const auto type = context.getApplicationType();
if (type == Context::ApplicationType::LOCAL || type == Context::ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
ALWAYS_INLINE inline void contextSanityClampSettings(const Context & context, Settings & settings)
{
const auto type = context.getApplicationType();
if (type == Context::ApplicationType::LOCAL || type == Context::ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
}
@ -2191,35 +2191,35 @@ void Context::checkSettingsConstraintsWithLock(const SettingsProfileElements & p
{
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.check(settings, profile_elements, source);
if (getApplicationType() == ApplicationType::LOCAL || getApplicationType() == ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::checkSettingsConstraintsWithLock(const SettingChange & change, SettingSource source)
{
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.check(settings, change, source);
if (getApplicationType() == ApplicationType::LOCAL || getApplicationType() == ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::checkSettingsConstraintsWithLock(const SettingsChanges & changes, SettingSource source)
{
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.check(settings, changes, source);
if (getApplicationType() == ApplicationType::LOCAL || getApplicationType() == ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::checkSettingsConstraintsWithLock(SettingsChanges & changes, SettingSource source)
{
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.check(settings, changes, source);
if (getApplicationType() == ApplicationType::LOCAL || getApplicationType() == ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::clampToSettingsConstraintsWithLock(SettingsChanges & changes, SettingSource source)
{
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.clamp(settings, changes, source);
if (getApplicationType() == ApplicationType::LOCAL || getApplicationType() == ApplicationType::SERVER)
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::checkMergeTreeSettingsConstraintsWithLock(const MergeTreeSettings & merge_tree_settings, const SettingsChanges & changes) const
@ -2243,7 +2243,7 @@ void Context::checkSettingsConstraints(const SettingsChanges & changes, SettingS
{
SharedLockGuard lock(mutex);
getSettingsConstraintsAndCurrentProfilesWithLock()->constraints.check(settings, changes, source);
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
}
void Context::checkSettingsConstraints(SettingsChanges & changes, SettingSource source)
@ -4484,7 +4484,7 @@ void Context::setDefaultProfiles(const Poco::Util::AbstractConfiguration & confi
setCurrentProfile(shared->system_profile_name);
applySettingsQuirks(settings, getLogger("SettingsQuirks"));
doSettingsSanityCheckClamp(settings);
doSettingsSanityCheckClamp(settings, getLogger("SettingsSanity"));
shared->buffer_profile_name = config.getString("buffer_profile", shared->system_profile_name);
buffer_context = Context::createCopy(shared_from_this());

1
tests/queries/0_stateless/02994_sanity_check_settings.reference
View File

@ -16,3 +16,4 @@ ExpressionTransform
Limit
(ReadFromStorage)
Zeros 0 → 1
4294967296

5
tests/queries/0_stateless/02994_sanity_check_settings.sql
View File

@ -1,3 +1,4 @@
SET send_logs_level = 'error';
CREATE TABLE data_02052_1_wide0__fuzz_48
(
`key` Nullable(Int64),
@ -22,3 +23,7 @@ FROM system.zeros LIMIT 10
SETTINGS max_block_size = 9223372036854775806, max_rows_to_read = 20, read_overflow_mode = 'break';
EXPLAIN PIPELINE SELECT zero + 1 AS x FROM system.zeros LIMIT 10 SETTINGS max_block_size = 9223372036854775806, max_rows_to_read = 20, read_overflow_mode = 'break';
-- Verify that we clamp odd values to something slightly saner
SET max_block_size = 9223372036854775806;
SELECT value FROM system.settings WHERE name = 'max_block_size';