From 7ed19c2a2d15dd3541cb855f5be36835a96d0836 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Fri, 20 Dec 2019 15:19:52 +0300 Subject: [PATCH 01/11] Added test for ALTER TTL. Co-authored-by: Vitaliy Zakaznikov --- dbms/tests/integration/test_ttl_move/test.py | 83 ++++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/dbms/tests/integration/test_ttl_move/test.py b/dbms/tests/integration/test_ttl_move/test.py index 5c252a573aa..7ec22952381 100644 --- a/dbms/tests/integration/test_ttl_move/test.py +++ b/dbms/tests/integration/test_ttl_move/test.py @@ -479,3 +479,86 @@ def test_ttls_do_not_work_after_alter(started_cluster, name, engine, positive): finally: node1.query("DROP TABLE IF EXISTS {}".format(name)) + + +@pytest.mark.parametrize("name,engine,positive", [ + ("mt_test_alter_multiple_ttls_positive", "MergeTree()", True), + ("mt_replicated_test_alter_multiple_ttls_positive", "ReplicatedMergeTree('/clickhouse/replicated_test_alter_multiple_ttls_positive', '1')", True), + ("mt_test_alter_multiple_ttls_negative", "MergeTree()", False), + ("mt_replicated_test_alter_multiple_ttls_negative", "ReplicatedMergeTree('/clickhouse/replicated_test_alter_multiple_ttls_negative', '1')", False), +]) +def test_alter_multiple_ttls(started_cluster, name, engine, positive): + """Copyright 2019, Altinity LTD + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License.""" + """Check that when multiple TTL expressions are set + and before any parts are inserted the TTL expressions + are changed with ALTER command then all old + TTL expressions are removed and the + the parts are moved to the specified disk or volume or + deleted if the new TTL expression is triggered + and are not moved or deleted when it is not. + """ + now = time.time() + try: + node1.query(""" + CREATE TABLE {name} ( + p1 Int64, + s1 String, + d1 DateTime + ) ENGINE = {engine} + ORDER BY tuple() + PARTITION BY p1 + TTL d1 + INTERVAL 30 SECOND TO DISK 'jbod2', + d1 + INTERVAL 60 SECOND TO VOLUME 'external' + SETTINGS storage_policy='jbods_with_external', merge_with_ttl_timeout=0 + """.format(name=name, engine=engine)) + + node1.query(""" + ALTER TABLE {name} MODIFY + TTL d1 + INTERVAL 0 SECOND TO DISK 'jbod2', + d1 + INTERVAL 5 SECOND TO VOLUME 'external', + d1 + INTERVAL 10 SECOND DELETE + """.format(name=name)) + + for p in range(3): + data = [] # 6MB in total + now = time.time() + for i in range(2): + p1 = p + s1 = get_random_string(1024 * 1024) # 1MB + d1 = now - 1 if i > 0 or positive else now + 300 + data.append("({}, '{}', toDateTime({}))".format(p1, s1, d1)) + node1.query("INSERT INTO {name} (p1, s1, d1) VALUES {values}".format(name=name, values=",".join(data))) + + used_disks = get_used_disks_for_table(node1, name) + assert set(used_disks) == {"jbod2"} if positive else {"jbod1", "jbod2"} + + assert node1.query("SELECT count() FROM {name}".format(name=name)).splitlines() == ["6"] + + time.sleep(5) + + used_disks = get_used_disks_for_table(node1, name) + assert set(used_disks) == {"external"} if positive else {"jbod1", "jbod2"} + + assert node1.query("SELECT count() FROM {name}".format(name=name)).splitlines() == ["6"] + + time.sleep(5) + + node1.query("OPTIMIZE TABLE {name} FINAL".format(name=name)) + + assert node1.query("SELECT count() FROM {name}".format(name=name)).splitlines() == ["6"] + assert r == "0" if positive else "6", error() + + finally: + node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) From cff368c1d1a25ce365c3518d418a6eb20eab34c2 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Mon, 23 Dec 2019 09:49:16 +0300 Subject: [PATCH 02/11] Updated `test_alter_multiple_ttls`. --- dbms/tests/integration/test_ttl_move/test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dbms/tests/integration/test_ttl_move/test.py b/dbms/tests/integration/test_ttl_move/test.py index 7ec22952381..ad41c5e85d9 100644 --- a/dbms/tests/integration/test_ttl_move/test.py +++ b/dbms/tests/integration/test_ttl_move/test.py @@ -557,8 +557,7 @@ limitations under the License.""" node1.query("OPTIMIZE TABLE {name} FINAL".format(name=name)) - assert node1.query("SELECT count() FROM {name}".format(name=name)).splitlines() == ["6"] - assert r == "0" if positive else "6", error() + assert node1.query("SELECT count() FROM {name}".format(name=name)).splitlines() == ["0"] if positive else ["3"] finally: node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) From 77944584fc70199eff1c33d699131de0782c77a9 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Mon, 23 Dec 2019 08:58:56 +0300 Subject: [PATCH 03/11] Fixed replicated alters of TTLs. --- dbms/src/Storages/StorageReplicatedMergeTree.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Storages/StorageReplicatedMergeTree.cpp b/dbms/src/Storages/StorageReplicatedMergeTree.cpp index d7f7a22895b..e37c3b893d7 100644 --- a/dbms/src/Storages/StorageReplicatedMergeTree.cpp +++ b/dbms/src/Storages/StorageReplicatedMergeTree.cpp @@ -534,7 +534,7 @@ void StorageReplicatedMergeTree::setTableStructure(ColumnsDescription new_column if (metadata_diff.ttl_table_changed) { - ParserExpression parser; + ParserTTLExpressionList parser; new_ttl_table_ast = parseQuery(parser, metadata_diff.new_ttl_table, 0); } From a940a2568454d005d6379c5a8e35947f0f22875b Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Sun, 22 Dec 2019 15:35:38 +0100 Subject: [PATCH 04/11] Add a way to set multiple parent profiles --- dbms/src/Access/SettingsConstraints.cpp | 14 +++++++++++--- dbms/src/Core/Settings.cpp | 2 +- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/dbms/src/Access/SettingsConstraints.cpp b/dbms/src/Access/SettingsConstraints.cpp index a044b7a0dc1..a4276a03257 100644 --- a/dbms/src/Access/SettingsConstraints.cpp +++ b/dbms/src/Access/SettingsConstraints.cpp @@ -217,9 +217,17 @@ const SettingsConstraints::Constraint * SettingsConstraints::tryGetConstraint(si void SettingsConstraints::setProfile(const String & profile_name, const Poco::Util::AbstractConfiguration & config) { - String parent_profile = "profiles." + profile_name + ".profile"; - if (config.has(parent_profile)) - setProfile(parent_profile, config); // Inheritance of one profile from another. + String elem = "profiles." + profile_name; + + Poco::Util::AbstractConfiguration::Keys config_keys; + config.keys(elem, config_keys); + + for (const std::string & key : config_keys) + { + if (key == "profile" || key.find("profile[") == 0) /// Inheritance of profiles from the current one. + setProfile(config.getString(elem + "." + key), config); + else continue; + } String path_to_constraints = "profiles." + profile_name + ".constraints"; if (config.has(path_to_constraints)) diff --git a/dbms/src/Core/Settings.cpp b/dbms/src/Core/Settings.cpp index 717261e298d..ede3a660cb3 100644 --- a/dbms/src/Core/Settings.cpp +++ b/dbms/src/Core/Settings.cpp @@ -37,7 +37,7 @@ void Settings::setProfile(const String & profile_name, const Poco::Util::Abstrac { if (key == "constraints") continue; - if (key == "profile") /// Inheritance of one profile from another. + if (key == "profile" || key.find("profile[") == 0) /// Inheritance of profiles from the current one. setProfile(config.getString(elem + "." + key), config); else set(key, config.getString(elem + "." + key)); From 332fe8e8f0d9210189694aaf73ed245475b74f39 Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Sun, 22 Dec 2019 15:41:41 +0100 Subject: [PATCH 05/11] Link zh/settings_profiles to en/settings_profiles since they are the same --- .../operations/settings/settings_profiles.md | 67 +------------------ 1 file changed, 1 insertion(+), 66 deletions(-) mode change 100644 => 120000 docs/zh/operations/settings/settings_profiles.md diff --git a/docs/zh/operations/settings/settings_profiles.md b/docs/zh/operations/settings/settings_profiles.md deleted file mode 100644 index c335e249212..00000000000 --- a/docs/zh/operations/settings/settings_profiles.md +++ /dev/null @@ -1,66 +0,0 @@ - -# Settings profiles - -A settings profile is a collection of settings grouped under the same name. Each ClickHouse user has a profile. -To apply all the settings in a profile, set the `profile` setting. - -Example: - -Install the `web` profile. - -``` sql -SET profile = 'web' -``` - -Settings profiles are declared in the user config file. This is usually `users.xml`. - -Example: - -```xml - - - - - - 8 - - - - - 1000000000 - 100000000000 - - 1000000 - any - - 1000000 - 1000000000 - - 100000 - 100000000 - break - - 600 - 1000000 - 15 - - 25 - 100 - 50 - - 2 - 25 - 50 - 100 - - 1 - - -``` - -The example specifies two profiles: `default` and `web`. The `default` profile has a special purpose: it must always be present and is applied when starting the server. In other words, the `default` profile contains default settings. The `web` profile is a regular profile that can be set using the `SET` query or using a URL parameter in an HTTP query. - -Settings profiles can inherit from each other. To use inheritance, indicate the `profile` setting before the other settings that are listed in the profile. - - -[Original article](https://clickhouse.yandex/docs/en/operations/settings/settings_profiles/) diff --git a/docs/zh/operations/settings/settings_profiles.md b/docs/zh/operations/settings/settings_profiles.md new file mode 120000 index 00000000000..35d9747ad56 --- /dev/null +++ b/docs/zh/operations/settings/settings_profiles.md @@ -0,0 +1 @@ +../../../en/operations/settings/settings_profiles.md \ No newline at end of file From 08506bdc967e47ce9bf1d8679503c893752b133f Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Sun, 22 Dec 2019 15:52:25 +0100 Subject: [PATCH 06/11] Update documentation regarding multiple profile inheritance --- docs/en/operations/settings/settings_profiles.md | 2 +- docs/ru/operations/settings/settings_profiles.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/operations/settings/settings_profiles.md b/docs/en/operations/settings/settings_profiles.md index c335e249212..2a5ce4f1fbf 100644 --- a/docs/en/operations/settings/settings_profiles.md +++ b/docs/en/operations/settings/settings_profiles.md @@ -60,7 +60,7 @@ Example: The example specifies two profiles: `default` and `web`. The `default` profile has a special purpose: it must always be present and is applied when starting the server. In other words, the `default` profile contains default settings. The `web` profile is a regular profile that can be set using the `SET` query or using a URL parameter in an HTTP query. -Settings profiles can inherit from each other. To use inheritance, indicate the `profile` setting before the other settings that are listed in the profile. +Settings profiles can inherit from each other. To use inheritance, indicate one or multiple `profile` settings before the other settings that are listed in the profile. [Original article](https://clickhouse.yandex/docs/en/operations/settings/settings_profiles/) diff --git a/docs/ru/operations/settings/settings_profiles.md b/docs/ru/operations/settings/settings_profiles.md index a120c388880..ede30bdf5a4 100644 --- a/docs/ru/operations/settings/settings_profiles.md +++ b/docs/ru/operations/settings/settings_profiles.md @@ -60,6 +60,6 @@ SET profile = 'web' В примере задано два профиля: `default` и `web`. Профиль `default` имеет специальное значение - он всегда обязан присутствовать и применяется при запуске сервера. То есть, профиль `default` содержит настройки по умолчанию. Профиль `web` - обычный профиль, который может быть установлен с помощью запроса `SET` или с помощью параметра URL при запросе по HTTP. -Профили настроек могут наследоваться от друг-друга - это реализуется указанием настройки `profile` перед остальными настройками, перечисленными в профиле. +Профили настроек могут наследоваться от друг-друга - это реализуется указанием одной или нескольких настроек `profile` перед остальными настройками, перечисленными в профиле. [Оригинальная статья](https://clickhouse.yandex/docs/ru/operations/settings/settings_profiles/) From e0f43e99e238386cbf0b4ac9f216a761c160dd74 Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Sun, 22 Dec 2019 23:10:33 +0100 Subject: [PATCH 07/11] Add tests for multiple profiles inheritance --- .../__init__.py | 0 .../configs/combined_profile.xml | 46 +++++++++++++ .../test_inherit_multiple_profiles/test.py | 65 +++++++++++++++++++ 3 files changed, 111 insertions(+) create mode 100644 dbms/tests/integration/test_inherit_multiple_profiles/__init__.py create mode 100644 dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml create mode 100644 dbms/tests/integration/test_inherit_multiple_profiles/test.py diff --git a/dbms/tests/integration/test_inherit_multiple_profiles/__init__.py b/dbms/tests/integration/test_inherit_multiple_profiles/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml b/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml new file mode 100644 index 00000000000..6c8ced8b72d --- /dev/null +++ b/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml @@ -0,0 +1,46 @@ + + + + 2 + + + + + + + + 1234567890 + + + 1234567889 + 1234567891 + + + + + 654321 + + + 654320 + 654322 + + + + + profile_1 + profile_2 + profile_3 + 2 + + + + + + + ::/0 + + default + combined_profile + + + diff --git a/dbms/tests/integration/test_inherit_multiple_profiles/test.py b/dbms/tests/integration/test_inherit_multiple_profiles/test.py new file mode 100644 index 00000000000..be905c867e9 --- /dev/null +++ b/dbms/tests/integration/test_inherit_multiple_profiles/test.py @@ -0,0 +1,65 @@ +import pytest + +from helpers.client import QueryRuntimeException +from helpers.cluster import ClickHouseCluster +from helpers.test_tools import TSV + + +cluster = ClickHouseCluster(__file__) +instance = cluster.add_instance('instance', + user_configs=['configs/combined_profile.xml']) +q = instance.query + + +@pytest.fixture(scope="module") +def started_cluster(): + try: + cluster.start() + + yield cluster + + finally: + cluster.shutdown() + + +def test_combined_profile(started_cluster): + settings = q(''' +SELECT name, value FROM system.settings + WHERE name IN + ('max_insert_block_size', 'max_network_bytes', + 'max_parallel_replicas', 'readonly') + AND changed +ORDER BY name +''', user='test_combined_profile') + + expected1 = '''\ +max_insert_block_size 654321 +max_network_bytes 1234567890 +max_parallel_replicas 2 +readonly 2''' + + assert TSV(settings) == TSV(expected1) + + with pytest.raises(QueryRuntimeException) as exc: + q(''' + SET max_insert_block_size = 1000; + ''', user='test_combined_profile') + + assert ("max_insert_block_size shouldn't be less than 654320." in + str(exc.value)) + + with pytest.raises(QueryRuntimeException) as exc: + q(''' + SET max_network_bytes = 2000000000; + ''', user='test_combined_profile') + + assert ("max_network_bytes shouldn't be greater than 1234567891." in + str(exc.value)) + + with pytest.raises(QueryRuntimeException) as exc: + q(''' + SET max_parallel_replicas = 1000; + ''', user='test_combined_profile') + + assert ('max_parallel_replicas should not be changed.' in + str(exc.value)) From 06fb37dd421e18e0b2b06d164598b8c5f1854758 Mon Sep 17 00:00:00 2001 From: "Mikhail f. Shiryaev" Date: Fri, 27 Dec 2019 18:14:30 +0100 Subject: [PATCH 08/11] Add tests and documentation for settings and constraints order --- .../configs/combined_profile.xml | 13 +++++++++++++ .../test_inherit_multiple_profiles/test.py | 11 ++++++++++- docs/en/operations/settings/settings_profiles.md | 2 +- docs/ru/operations/settings/settings_profiles.md | 2 +- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml b/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml index 6c8ced8b72d..4fbb7dcf3ff 100644 --- a/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml +++ b/dbms/tests/integration/test_inherit_multiple_profiles/configs/combined_profile.xml @@ -2,7 +2,12 @@ 2 + + 200000000 + + 100000000 + @@ -10,7 +15,11 @@ 1234567890 + 300000000 + + 200000000 + 1234567889 1234567891 @@ -19,7 +28,11 @@ 654321 + 400000000 + + 300000000 + 654320 654322 diff --git a/dbms/tests/integration/test_inherit_multiple_profiles/test.py b/dbms/tests/integration/test_inherit_multiple_profiles/test.py index be905c867e9..1540196f9b6 100644 --- a/dbms/tests/integration/test_inherit_multiple_profiles/test.py +++ b/dbms/tests/integration/test_inherit_multiple_profiles/test.py @@ -26,7 +26,7 @@ def test_combined_profile(started_cluster): settings = q(''' SELECT name, value FROM system.settings WHERE name IN - ('max_insert_block_size', 'max_network_bytes', + ('max_insert_block_size', 'max_network_bytes', 'max_query_size', 'max_parallel_replicas', 'readonly') AND changed ORDER BY name @@ -36,6 +36,7 @@ ORDER BY name max_insert_block_size 654321 max_network_bytes 1234567890 max_parallel_replicas 2 +max_query_size 400000000 readonly 2''' assert TSV(settings) == TSV(expected1) @@ -63,3 +64,11 @@ readonly 2''' assert ('max_parallel_replicas should not be changed.' in str(exc.value)) + + with pytest.raises(QueryRuntimeException) as exc: + q(''' + SET max_memory_usage = 1000; + ''', user='test_combined_profile') + + assert ("max_memory_usage shouldn't be less than 300000000." in + str(exc.value)) diff --git a/docs/en/operations/settings/settings_profiles.md b/docs/en/operations/settings/settings_profiles.md index 2a5ce4f1fbf..095bb362936 100644 --- a/docs/en/operations/settings/settings_profiles.md +++ b/docs/en/operations/settings/settings_profiles.md @@ -60,7 +60,7 @@ Example: The example specifies two profiles: `default` and `web`. The `default` profile has a special purpose: it must always be present and is applied when starting the server. In other words, the `default` profile contains default settings. The `web` profile is a regular profile that can be set using the `SET` query or using a URL parameter in an HTTP query. -Settings profiles can inherit from each other. To use inheritance, indicate one or multiple `profile` settings before the other settings that are listed in the profile. +Settings profiles can inherit from each other. To use inheritance, indicate one or multiple `profile` settings before the other settings that are listed in the profile. In case when one setting is defined in different profiles, the latest defined is used. [Original article](https://clickhouse.yandex/docs/en/operations/settings/settings_profiles/) diff --git a/docs/ru/operations/settings/settings_profiles.md b/docs/ru/operations/settings/settings_profiles.md index ede30bdf5a4..8b4e2316fe6 100644 --- a/docs/ru/operations/settings/settings_profiles.md +++ b/docs/ru/operations/settings/settings_profiles.md @@ -60,6 +60,6 @@ SET profile = 'web' В примере задано два профиля: `default` и `web`. Профиль `default` имеет специальное значение - он всегда обязан присутствовать и применяется при запуске сервера. То есть, профиль `default` содержит настройки по умолчанию. Профиль `web` - обычный профиль, который может быть установлен с помощью запроса `SET` или с помощью параметра URL при запросе по HTTP. -Профили настроек могут наследоваться от друг-друга - это реализуется указанием одной или нескольких настроек `profile` перед остальными настройками, перечисленными в профиле. +Профили настроек могут наследоваться от друг-друга - это реализуется указанием одной или нескольких настроек `profile` перед остальными настройками, перечисленными в профиле. Если одна настройка указана в нескольких профилях, используется последнее из значений. [Оригинальная статья](https://clickhouse.yandex/docs/ru/operations/settings/settings_profiles/) From 4569a68e12a70c384134bf6dc8556850671f15c0 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Fri, 27 Dec 2019 21:20:02 +0300 Subject: [PATCH 09/11] Empty commit to re-run checks. From 9f7b56e926ecfa68af20ee4c4073a7cb9fa814f5 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 28 Dec 2019 06:52:21 +0300 Subject: [PATCH 10/11] Update SettingsConstraints.cpp --- dbms/src/Access/SettingsConstraints.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dbms/src/Access/SettingsConstraints.cpp b/dbms/src/Access/SettingsConstraints.cpp index a4276a03257..64460aaa8f1 100644 --- a/dbms/src/Access/SettingsConstraints.cpp +++ b/dbms/src/Access/SettingsConstraints.cpp @@ -224,9 +224,10 @@ void SettingsConstraints::setProfile(const String & profile_name, const Poco::Ut for (const std::string & key : config_keys) { - if (key == "profile" || key.find("profile[") == 0) /// Inheritance of profiles from the current one. + if (key == "profile" || 0 == key.compare(0, strlen("profile["), "profile[")) /// Inheritance of profiles from the current one. setProfile(config.getString(elem + "." + key), config); - else continue; + else + continue; } String path_to_constraints = "profiles." + profile_name + ".constraints"; From 9d2692098347f28c8aaf49f80861fffa69f8b790 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 28 Dec 2019 06:52:47 +0300 Subject: [PATCH 11/11] Update Settings.cpp --- dbms/src/Core/Settings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/src/Core/Settings.cpp b/dbms/src/Core/Settings.cpp index ede3a660cb3..fe6e0c18b85 100644 --- a/dbms/src/Core/Settings.cpp +++ b/dbms/src/Core/Settings.cpp @@ -37,7 +37,7 @@ void Settings::setProfile(const String & profile_name, const Poco::Util::Abstrac { if (key == "constraints") continue; - if (key == "profile" || key.find("profile[") == 0) /// Inheritance of profiles from the current one. + if (key == "profile" || 0 == key.compare(0, strlen("profile["), "profile[")) /// Inheritance of profiles from the current one. setProfile(config.getString(elem + "." + key), config); else set(key, config.getString(elem + "." + key));