From 5cbbcd9cdb726c49d85f37b56931a82f72a3c8c8 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Tue, 19 Nov 2019 09:44:10 +0300 Subject: [PATCH 1/3] Added disk info to `system.detached_parts`. --- dbms/src/Storages/MergeTree/MergeTreeData.cpp | 12 +++++++++++- dbms/src/Storages/MergeTree/MergeTreeData.h | 4 ++++ dbms/src/Storages/MergeTree/MergeTreePartInfo.h | 2 ++ .../Storages/System/StorageSystemDetachedParts.cpp | 2 ++ 4 files changed, 19 insertions(+), 1 deletion(-) diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.cpp b/dbms/src/Storages/MergeTree/MergeTreeData.cpp index 728f69f477e..802b2dce25c 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.cpp +++ b/dbms/src/Storages/MergeTree/MergeTreeData.cpp @@ -2933,7 +2933,7 @@ MergeTreeData::getDetachedParts() const { std::vector res; - for (const String & path : getDataPaths()) + for (const auto & [path, disk] : getDataPathsWithDisks()) { for (Poco::DirectoryIterator it(path + "detached"); it != Poco::DirectoryIterator(); ++it) @@ -2944,6 +2944,7 @@ MergeTreeData::getDetachedParts() const auto & part = res.back(); DetachedPartInfo::tryParseDetachedPartName(dir_name, part, format_version); + part.disk = disk->getName(); } } return res; @@ -3327,6 +3328,15 @@ Strings MergeTreeData::getDataPaths() const return res; } +MergeTreeData::PathsWithDisks MergeTreeData::getDataPathsWithDisks() const +{ + PathsWithDisks res; + auto disks = storage_policy->getDisks(); + for (const auto & disk : disks) + res.emplace_back(getFullPathOnDisk(disk), disk); + return res; +} + void MergeTreeData::freezePartitionsByMatcher(MatcherFn matcher, const String & with_name, const Context & context) { String clickhouse_path = Poco::Path(context.getPath()).makeAbsolute().toString(); diff --git a/dbms/src/Storages/MergeTree/MergeTreeData.h b/dbms/src/Storages/MergeTree/MergeTreeData.h index b03cbd8cb70..464cfcea52c 100644 --- a/dbms/src/Storages/MergeTree/MergeTreeData.h +++ b/dbms/src/Storages/MergeTree/MergeTreeData.h @@ -669,6 +669,10 @@ public: Strings getDataPaths() const override; + using PathWithDisk = std::pair; + using PathsWithDisks = std::vector; + PathsWithDisks getDataPathsWithDisks() const; + /// Reserves space at least 1MB DiskSpace::ReservationPtr reserveSpace(UInt64 expected_size); diff --git a/dbms/src/Storages/MergeTree/MergeTreePartInfo.h b/dbms/src/Storages/MergeTree/MergeTreePartInfo.h index 9fe0fbab533..2cf423f325a 100644 --- a/dbms/src/Storages/MergeTree/MergeTreePartInfo.h +++ b/dbms/src/Storages/MergeTree/MergeTreePartInfo.h @@ -95,6 +95,8 @@ struct DetachedPartInfo : public MergeTreePartInfo String dir_name; String prefix; + String disk; + /// If false, MergeTreePartInfo is in invalid state (directory name was not successfully parsed). bool valid_name; diff --git a/dbms/src/Storages/System/StorageSystemDetachedParts.cpp b/dbms/src/Storages/System/StorageSystemDetachedParts.cpp index b95a299af68..acda98203db 100644 --- a/dbms/src/Storages/System/StorageSystemDetachedParts.cpp +++ b/dbms/src/Storages/System/StorageSystemDetachedParts.cpp @@ -35,6 +35,7 @@ protected: {"table", std::make_shared()}, {"partition_id", std::make_shared(std::make_shared())}, {"name", std::make_shared()}, + {"disk", std::make_shared()}, {"reason", std::make_shared(std::make_shared())}, {"min_block_number", std::make_shared(std::make_shared())}, {"max_block_number", std::make_shared(std::make_shared())}, @@ -66,6 +67,7 @@ protected: new_columns[i++]->insert(info.table); new_columns[i++]->insert(p.valid_name ? p.partition_id : Field()); new_columns[i++]->insert(p.dir_name); + new_columns[i++]->insert(p.disk); new_columns[i++]->insert(p.valid_name ? p.prefix : Field()); new_columns[i++]->insert(p.valid_name ? p.min_block : Field()); new_columns[i++]->insert(p.valid_name ? p.max_block : Field()); From 67d2703d30a66a362c81b292da056e1ea17ad005 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Tue, 19 Nov 2019 14:29:52 +0300 Subject: [PATCH 2/3] Fixed tests to support disk name in `system.detached_parts`. --- .../0_stateless/00502_custom_partitioning_local.reference | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbms/tests/queries/0_stateless/00502_custom_partitioning_local.reference b/dbms/tests/queries/0_stateless/00502_custom_partitioning_local.reference index fff28819e74..7b14a2d4edc 100644 --- a/dbms/tests/queries/0_stateless/00502_custom_partitioning_local.reference +++ b/dbms/tests/queries/0_stateless/00502_custom_partitioning_local.reference @@ -9,7 +9,7 @@ Sum before DETACH PARTITION: Sum after DETACH PARTITION: 0 system.detached_parts after DETACH PARTITION: -default not_partitioned all all_1_2_1 1 2 1 +default not_partitioned all all_1_2_1 default 1 2 1 *** Partitioned by week *** Parts before OPTIMIZE: 1999-12-27 19991227_1_1_0 From 008e1efa753f4e22a1ef5d008ed29d3d98eb0c50 Mon Sep 17 00:00:00 2001 From: Vladimir Chebotarev Date: Tue, 19 Nov 2019 14:43:33 +0300 Subject: [PATCH 3/3] Added test for disk name in `system.detached_parts`. --- .../integration/test_multiple_disks/test.py | 32 ++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/dbms/tests/integration/test_multiple_disks/test.py b/dbms/tests/integration/test_multiple_disks/test.py index 446eca88142..38ed618845f 100644 --- a/dbms/tests/integration/test_multiple_disks/test.py +++ b/dbms/tests/integration/test_multiple_disks/test.py @@ -768,12 +768,42 @@ def test_concurrent_alter_move_and_drop(start_cluster, name, engine): node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) +@pytest.mark.parametrize("name,engine", [ + ("detach_attach_mt","MergeTree()"), + ("replicated_detach_attach_mt","ReplicatedMergeTree('/clickhouse/replicated_detach_attach_mt', '1')",), +]) +def test_detach_attach(start_cluster, name, engine): + try: + node1.query(""" + CREATE TABLE {name} ( + s1 String + ) ENGINE = {engine} + ORDER BY tuple() + SETTINGS storage_policy='moving_jbod_with_external' + """.format(name=name, engine=engine)) + + data = [] # 5MB in total + for i in range(5): + data.append(get_random_string(1024 * 1024)) # 1MB row + node1.query("INSERT INTO {} VALUES {}".format(name, ','.join(["('" + x + "')" for x in data]))) + + node1.query("ALTER TABLE {} DETACH PARTITION tuple()".format(name)) + assert node1.query("SELECT count() FROM {}".format(name)).strip() == "0" + + assert node1.query("SELECT disk FROM system.detached_parts WHERE table = '{}'".format(name)).strip() == "jbod1" + + node1.query("ALTER TABLE {} ATTACH PARTITION tuple()".format(name)) + assert node1.query("SELECT count() FROM {}".format(name)).strip() == "5" + + finally: + node1.query("DROP TABLE IF EXISTS {name}".format(name=name)) + + @pytest.mark.parametrize("name,engine", [ ("mutating_mt","MergeTree()"), ("replicated_mutating_mt","ReplicatedMergeTree('/clickhouse/replicated_mutating_mt', '1')",), ]) def test_mutate_to_another_disk(start_cluster, name, engine): - try: node1.query(""" CREATE TABLE {name} (