diff --git a/docs/en/engines/table-engines/mergetree-family/mergetree.md b/docs/en/engines/table-engines/mergetree-family/mergetree.md index bb7e57ecbf6..228b2c8884f 100644 --- a/docs/en/engines/table-engines/mergetree-family/mergetree.md +++ b/docs/en/engines/table-engines/mergetree-family/mergetree.md @@ -870,11 +870,11 @@ Tags: - `load_balancing` - Policy for disk balancing, `round_robin` or `least_used`. - `least_used_ttl_ms` - Configure timeout (in milliseconds) for the updating available space on all disks (`0` - update always, `-1` - never update, default is `60000`). Note, if the disk can be used by ClickHouse only and is not subject to a online filesystem resize/shrink you can use `-1`, in all other cases it is not recommended, since eventually it will lead to incorrect space distribution. - `prefer_not_to_merge` — You should not use this setting. Disables merging of data parts on this volume (this is harmful and leads to performance degradation). When this setting is enabled (don't do it), merging data on this volume is not allowed (which is bad). This allows (but you don't need it) controlling (if you want to control something, you're making a mistake) how ClickHouse works with slow disks (but ClickHouse knows better, so please don't use this setting). -- `volume_priority` — Defines the relative order in which volumes are filled. The lower is the value -- the higher the priority. Value can be a non-negative whole number. Default priority value is `0`. - * If _all_ volumes tagged, they are prioritized in given order. - * If _some_ volumes tagged, those without this tag have higher priority than those having the tag. Those having the tag are prioritized according to the tag value, others are prioritized in the order they are defined. - * If _no_ volume tagged, their priority is set correspondingly to their order they are declared in configuration. - * If two or more volumes have the same setting value, they are prioritized in the order they are defined. +- `volume_priority` — Defines the priority (order) in which volumes are filled. Lower value means higher priority. The parameter values should be natural numbers and collectively cover the range from 1 to N (lowest priority given) without skipping any numbers. + * If _all_ volumes are tagged, they are prioritized in given order. + * If only _some_ volumes are tagged, those without the tag have the lowest priority, and they are prioritized in the order they are defined in config. + * If _no_ volumes are tagged, their priority is set correspondingly to their order they are declared in configuration. + * Two volumes cannot have the same priority value. Configuration examples: diff --git a/docs/ru/engines/table-engines/mergetree-family/mergetree.md b/docs/ru/engines/table-engines/mergetree-family/mergetree.md index 9dcf4bd450b..faa492d4d85 100644 --- a/docs/ru/engines/table-engines/mergetree-family/mergetree.md +++ b/docs/ru/engines/table-engines/mergetree-family/mergetree.md @@ -688,11 +688,11 @@ TTL d + INTERVAL 1 MONTH GROUP BY k1, k2 SET x = max(x), y = min(y); - `load_balancing` - политика балансировки дисков, `round_robin` или `least_used`. - `least_used_ttl_ms` - устанавливает таймаут (в миллисекундах) для обновления доступного пространства на всех дисках (`0` - обновлять всегда, `-1` - никогда не обновлять, значение по умолчанию - `60000`). Обратите внимание, если диск используется только ClickHouse и не будет подвергаться изменению размеров файловой системы на лету, можно использовать значение `-1`. Во всех остальных случаях это не рекомендуется, так как в конечном итоге это приведет к неправильному распределению пространства. - `prefer_not_to_merge` — эту настройку лучше не использовать. Она отключает слияние частей данных на этом томе (что потенциально вредно и может привести к замедлению). Когда эта настройка включена (не делайте этого), объединение данных на этом томе запрещено (что плохо). Это позволяет (но вам это не нужно) контролировать (если вы хотите что-то контролировать, вы делаете ошибку), как ClickHouse взаимодействует с медленными дисками (но ClickHouse лучше знает, поэтому, пожалуйста, не используйте эту настройку). -- `volume_priority` — определяет _относительный_ порядок заполнения томов. Том с меньшим значением имеет больший приоритет. Значениями могут быть целые неотрицательные числа. Значение приоритета по умолчанию -- `0`. +- `volume_priority` — Определяет приоритет (порядок), в котором заполняются тома. Чем меньше значение -- тем выше приоритет. Значения параметра должны быть натуральными числами и охватывать диапазон от 1 до N (N - наибольшее значение параметра из указанных) без пропусков. * Если _все_ тома имеют этот параметр, они приоритизируются в указанном порядке. - * Если его имеют лишь _некоторые_, то не имеющие этого параметра тома имеют более высокий приоритет чем те, у которых он указан. Те, у которых он указан, приоритизируются в соответствии со значением тега, приоритет остальных определяется порядком описания в конфигурационном файле относительно друг друга. + * Если его имеют лишь _некоторые_, то не имеющие этого параметра тома имеют самый низкий приоритет. Те, у которых он указан, приоритизируются в соответствии со значением тега, приоритет остальных определяется порядком описания в конфигурационном файле относительно друг друга. * Если _ни одному_ тому не присвоен этот параметр, их порядок определяется порядком описания в конфигурационном файле. - * Если нескольким томам присвоено одно и то же значение приоритета, порядок их приоритетов между собой определяется порядком описания в конфигурационном файле. + * Приоритет нескольких томов не может быть одинаковым. Примеры конфигураций: diff --git a/src/Disks/IVolume.h b/src/Disks/IVolume.h index 6c4282835a1..bc6706a1829 100644 --- a/src/Disks/IVolume.h +++ b/src/Disks/IVolume.h @@ -92,8 +92,8 @@ protected: const String name; public: - /// Volume priority. 0 (default) means no explicit priority was set. - UInt64 volume_priority = 0; + /// Volume priority. Maximum UInt64 value by default (lowest possible priority) + UInt64 volume_priority; /// Max size of reservation, zero means unlimited size UInt64 max_data_part_size = 0; /// Should a new data part be synchronously moved to a volume according to ttl on insert diff --git a/src/Disks/StoragePolicy.cpp b/src/Disks/StoragePolicy.cpp index ea986e8e50f..529201f7740 100644 --- a/src/Disks/StoragePolicy.cpp +++ b/src/Disks/StoragePolicy.cpp @@ -28,6 +28,7 @@ namespace ErrorCodes extern const int BAD_ARGUMENTS; extern const int EXCESSIVE_ELEMENT_IN_CONFIG; extern const int NO_ELEMENTS_IN_CONFIG; + extern const int INVALID_CONFIG_PARAMETER; extern const int UNKNOWN_POLICY; extern const int UNKNOWN_VOLUME; extern const int LOGICAL_ERROR; @@ -56,7 +57,8 @@ StoragePolicy::StoragePolicy( config.keys(volumes_prefix, keys); } - bool has_information_about_priority = false; + std::set volume_priorities; + for (const auto & attr_name : keys) { if (!std::all_of(attr_name.begin(), attr_name.end(), isWordCharASCII)) @@ -64,17 +66,31 @@ StoragePolicy::StoragePolicy( "Volume name can contain only alphanumeric and '_' in storage policy {} ({})", backQuote(name), attr_name); volumes.emplace_back(createVolumeFromConfig(attr_name, config, volumes_prefix + "." + attr_name, disks)); - if (!has_information_about_priority && volumes.back()->volume_priority) - has_information_about_priority = true; + + UInt64 last_priority = volumes.back()->volume_priority; + if (last_priority != std::numeric_limits::max()) + { + if (volume_priorities.find(last_priority) == volume_priorities.end()) + volume_priorities.insert(last_priority); + else + throw Exception( + ErrorCodes::INVALID_CONFIG_PARAMETER, + "volume_priority values must be unique across the policy"); + } } - if (has_information_about_priority) - std::stable_sort(volumes.begin(), volumes.end(), - [](const VolumePtr a, const VolumePtr b) - { - return a->volume_priority < b->volume_priority; - } - ); + if (!volume_priorities.empty()) + { + /// Check that priority values cover the range from 1 to N (lowest explicit priority) + if (*volume_priorities.begin() != 1 || *volume_priorities.rbegin() != volume_priorities.size()) + throw Exception( + ErrorCodes::INVALID_CONFIG_PARAMETER, + "volume_priority values must cover the range from 1 to N (lowest priority specified) without gaps"); + + std::stable_sort( + volumes.begin(), volumes.end(), + [](const VolumePtr a, const VolumePtr b) { return a->volume_priority < b->volume_priority; }); + } if (volumes.empty() && name == DEFAULT_STORAGE_POLICY_NAME) { diff --git a/src/Disks/VolumeJBOD.cpp b/src/Disks/VolumeJBOD.cpp index 3f7ee870455..a0c71583a22 100644 --- a/src/Disks/VolumeJBOD.cpp +++ b/src/Disks/VolumeJBOD.cpp @@ -23,9 +23,7 @@ VolumeJBOD::VolumeJBOD( { LoggerPtr logger = getLogger("StorageConfiguration"); - auto has_volume_priority = config.has(config_prefix + ".volume_priority"); - if (has_volume_priority) - volume_priority = config.getUInt64(config_prefix + ".volume_priority", 0); + volume_priority = config.getUInt64(config_prefix + ".volume_priority", std::numeric_limits::max()); auto has_max_bytes = config.has(config_prefix + ".max_data_part_size_bytes"); auto has_max_ratio = config.has(config_prefix + ".max_data_part_size_ratio"); diff --git a/tests/config/config.d/storage_conf_02961.xml b/tests/config/config.d/storage_conf_02961.xml index abee180bb86..436a5628e51 100644 --- a/tests/config/config.d/storage_conf_02961.xml +++ b/tests/config/config.d/storage_conf_02961.xml @@ -25,16 +25,16 @@ disk1_02961 2 - + disk2_02961 - + disk3_02961 1 - + disk4_02961 - + 0.2 diff --git a/tests/config/install.sh b/tests/config/install.sh index cfe810cda84..f314a9104b7 100755 --- a/tests/config/install.sh +++ b/tests/config/install.sh @@ -178,6 +178,7 @@ if [[ -n "$EXPORT_S3_STORAGE_POLICIES" ]]; then ln -sf $SRC_PATH/config.d/storage_conf.xml $DEST_SERVER_PATH/config.d/ ln -sf $SRC_PATH/config.d/storage_conf_02944.xml $DEST_SERVER_PATH/config.d/ ln -sf $SRC_PATH/config.d/storage_conf_02963.xml $DEST_SERVER_PATH/config.d/ + ln -sf $SRC_PATH/config.d/storage_conf_02961.xml $DEST_SERVER_PATH/config.d/ ln -sf $SRC_PATH/users.d/s3_cache.xml $DEST_SERVER_PATH/users.d/ ln -sf $SRC_PATH/users.d/s3_cache_new.xml $DEST_SERVER_PATH/users.d/ fi diff --git a/tests/queries/0_stateless/02961_storage_config_volume_priority.reference b/tests/queries/0_stateless/02961_storage_config_volume_priority.reference index ac4e63b175d..ba48e75ae25 100644 --- a/tests/queries/0_stateless/02961_storage_config_volume_priority.reference +++ b/tests/queries/0_stateless/02961_storage_config_volume_priority.reference @@ -1,4 +1,9 @@ -vol_untagged1_02961 1 -vol_untagged2_02961 2 -vol2_02961 3 -vol1_02961 4 +vol2_02961 1 +vol1_02961 2 +vol_untagged2_02961 3 +vol_untagged1_02961 4 +check non-unique values dont work +1 +check no gaps in range allowed +1 +restore valid config diff --git a/tests/queries/0_stateless/02961_storage_config_volume_priority.sh b/tests/queries/0_stateless/02961_storage_config_volume_priority.sh new file mode 100755 index 00000000000..4e085541a8d --- /dev/null +++ b/tests/queries/0_stateless/02961_storage_config_volume_priority.sh @@ -0,0 +1,51 @@ +#!/usr/bin/env bash +# Tags: no-fasttest, no-parallel, no-random-settings + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + + +$CLICKHOUSE_CLIENT --query " +SELECT + volume_name, + volume_priority +FROM system.storage_policies +WHERE policy_name = 'policy_02961' +ORDER BY volume_priority ASC; +" + +config_path=/etc/clickhouse-server/config.d/storage_conf_02961.xml +config_path_tmp=$config_path.tmp + +echo 'check non-unique values dont work' +cat $config_path \ +| sed "s|2<\/volume_priority>|1<\/volume_priority>|" \ +> $config_path_tmp +mv $config_path_tmp $config_path + +$CLICKHOUSE_CLIENT -nm --query " +set send_logs_level='error'; +SYSTEM RELOAD CONFIG" 2>&1 | grep -c 'volume_priority values must be unique across the policy' + +#first, restore original values +cat $config_path \ +| sed '0,/1<\/volume_priority>/s//2<\/volume_priority>/' \ +> $config_path_tmp +mv $config_path_tmp $config_path + +echo 'check no gaps in range allowed' +cat $config_path \ +| sed '0,/1<\/volume_priority>/s//3<\/volume_priority>/' \ +> $config_path_tmp +mv $config_path_tmp $config_path + +$CLICKHOUSE_CLIENT -nm --query " +set send_logs_level='error'; +SYSTEM RELOAD CONFIG" 2>&1 | grep -c 'volume_priority values must cover the range from 1 to N (lowest priority specified) without gaps' + +echo 'restore valid config' +cat $config_path \ +| sed '0,/3<\/volume_priority>/s//1<\/volume_priority>/' \ +> $config_path_tmp +mv $config_path_tmp $config_path diff --git a/tests/queries/0_stateless/02961_storage_config_volume_priority.sql b/tests/queries/0_stateless/02961_storage_config_volume_priority.sql deleted file mode 100644 index 6680bf7fcde..00000000000 --- a/tests/queries/0_stateless/02961_storage_config_volume_priority.sql +++ /dev/null @@ -1,7 +0,0 @@ -SELECT - volume_name, - volume_priority -FROM system.storage_policies -WHERE policy_name = 'policy_02961' -ORDER BY volume_priority ASC; -