From 5b2b8d38fa50fd8b1f195c5890be8c103ad65b61 Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Wed, 8 Apr 2020 11:41:13 +0300 Subject: [PATCH 1/7] Download part trough disk interface. --- src/Storages/MergeTree/DataPartsExchange.cpp | 29 ++++++++++---------- src/Storages/MergeTree/MergeTreeData.h | 3 ++ 2 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/Storages/MergeTree/DataPartsExchange.cpp b/src/Storages/MergeTree/DataPartsExchange.cpp index 6373c85a15d..4e40d4a5977 100644 --- a/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/src/Storages/MergeTree/DataPartsExchange.cpp @@ -258,19 +258,20 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( size_t files; readBinary(files, in); + auto disk = reservation->getDisk(); + static const String TMP_PREFIX = "tmp_fetch_"; String tmp_prefix = tmp_prefix_.empty() ? TMP_PREFIX : tmp_prefix_; - String relative_part_path = String(to_detached ? "detached/" : "") + tmp_prefix + part_name; - String absolute_part_path = Poco::Path(data.getFullPathOnDisk(reservation->getDisk()) + relative_part_path + "/").absolute().toString(); - Poco::File part_file(absolute_part_path); + String part_relative_path = String(to_detached ? "detached/" : "") + tmp_prefix + part_name; + String part_download_path = data.getRelativeDataPath() + part_relative_path + "/"; - if (part_file.exists()) - throw Exception("Directory " + absolute_part_path + " already exists.", ErrorCodes::DIRECTORY_ALREADY_EXISTS); + if (disk->exists(part_download_path)) + throw Exception("Directory " + fullPath(disk, part_download_path) + " already exists.", ErrorCodes::DIRECTORY_ALREADY_EXISTS); CurrentMetrics::Increment metric_increment{CurrentMetrics::ReplicatedFetch}; - part_file.createDirectory(); + disk->createDirectories(part_download_path); MergeTreeData::DataPart::Checksums checksums; for (size_t i = 0; i < files; ++i) @@ -283,21 +284,21 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( /// File must be inside "absolute_part_path" directory. /// Otherwise malicious ClickHouse replica may force us to write to arbitrary path. - String absolute_file_path = Poco::Path(absolute_part_path + file_name).absolute().toString(); - if (!startsWith(absolute_file_path, absolute_part_path)) - throw Exception("File path (" + absolute_file_path + ") doesn't appear to be inside part path (" + absolute_part_path + ")." + String absolute_file_path = Poco::Path(part_download_path + file_name).absolute().toString(); + if (!startsWith(absolute_file_path, part_download_path)) + throw Exception("File path (" + absolute_file_path + ") doesn't appear to be inside part path (" + part_download_path + ")." " This may happen if we are trying to download part from malicious replica or logical error.", ErrorCodes::INSECURE_PATH); - WriteBufferFromFile file_out(absolute_file_path); - HashingWriteBuffer hashing_out(file_out); + auto file_out = disk->writeFile(part_download_path + file_name); + HashingWriteBuffer hashing_out(*file_out); copyData(in, hashing_out, file_size, blocker.getCounter()); if (blocker.isCancelled()) { /// NOTE The is_cancelled flag also makes sense to check every time you read over the network, performing a poll with a not very large timeout. /// And now we check it only between read chunks (in the `copyData` function). - part_file.remove(true); + disk->removeRecursive(part_download_path); throw Exception("Fetching of part was cancelled", ErrorCodes::ABORTED); } @@ -305,7 +306,7 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( readPODBinary(expected_hash, in); if (expected_hash != hashing_out.getHash()) - throw Exception("Checksum mismatch for file " + absolute_part_path + file_name + " transferred from " + replica_path, + throw Exception("Checksum mismatch for file " + fullPath(disk, part_download_path + file_name) + " transferred from " + replica_path, ErrorCodes::CHECKSUM_DOESNT_MATCH); if (file_name != "checksums.txt" && @@ -315,7 +316,7 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( assertEOF(in); - MergeTreeData::MutableDataPartPtr new_data_part = data.createPart(part_name, reservation->getDisk(), relative_part_path); + MergeTreeData::MutableDataPartPtr new_data_part = data.createPart(part_name, reservation->getDisk(), part_relative_path); new_data_part->is_temp = true; new_data_part->modification_time = time(nullptr); new_data_part->loadColumnsChecksumsIndexes(true, false); diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h index 125a90d26e0..eb2a0dd8774 100644 --- a/src/Storages/MergeTree/MergeTreeData.h +++ b/src/Storages/MergeTree/MergeTreeData.h @@ -625,6 +625,9 @@ public: return storage_settings.get(); } + /// Get relative table path + String getRelativeDataPath() const { return relative_data_path; } + /// Get table path on disk String getFullPathOnDisk(const DiskPtr & disk) const; From 4ec77fee8b5a49d2f98659a9c817a8c47bd866e9 Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Wed, 8 Apr 2020 13:53:17 +0300 Subject: [PATCH 2/7] Integration test for ReplicatedMergeTree over S3. --- .../test_replicated_merge_tree_s3/__init__.py | 0 .../config.d/bg_processing_pool_conf.xml | 5 + .../configs/config.d/log_conf.xml | 12 ++ .../configs/config.d/storage_conf.xml | 28 +++++ .../configs/config.d/users.xml | 6 + .../configs/config.xml | 20 ++++ .../test_replicated_merge_tree_s3/test.py | 108 ++++++++++++++++++ 7 files changed, 179 insertions(+) create mode 100644 tests/integration/test_replicated_merge_tree_s3/__init__.py create mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml create mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml create mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml create mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml create mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.xml create mode 100644 tests/integration/test_replicated_merge_tree_s3/test.py diff --git a/tests/integration/test_replicated_merge_tree_s3/__init__.py b/tests/integration/test_replicated_merge_tree_s3/__init__.py new file mode 100644 index 00000000000..e69de29bb2d diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml new file mode 100644 index 00000000000..a756c4434ea --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml @@ -0,0 +1,5 @@ + + 0.5 + 0.5 + 0.5 + diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml new file mode 100644 index 00000000000..318a6bca95d --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml @@ -0,0 +1,12 @@ + + 3 + + trace + /var/log/clickhouse-server/log.log + /var/log/clickhouse-server/log.err.log + 1000M + 10 + /var/log/clickhouse-server/stderr.log + /var/log/clickhouse-server/stdout.log + + diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml new file mode 100644 index 00000000000..5b292446c6b --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml @@ -0,0 +1,28 @@ + + + + + s3 + http://minio1:9001/root/data/ + minio + minio123 + + + local + / + + + + + +
+ s3 +
+ + hdd + + + + + + diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml new file mode 100644 index 00000000000..a13b24b278d --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml @@ -0,0 +1,6 @@ + + + + + + diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.xml new file mode 100644 index 00000000000..24b7344df3a --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.xml @@ -0,0 +1,20 @@ + + + 9000 + 127.0.0.1 + + + + true + none + + AcceptCertificateHandler + + + + + 500 + 5368709120 + ./clickhouse/ + users.xml + diff --git a/tests/integration/test_replicated_merge_tree_s3/test.py b/tests/integration/test_replicated_merge_tree_s3/test.py new file mode 100644 index 00000000000..53eb612c281 --- /dev/null +++ b/tests/integration/test_replicated_merge_tree_s3/test.py @@ -0,0 +1,108 @@ +import logging +import random +import string +import time + +import pytest +from helpers.cluster import ClickHouseCluster + +logging.getLogger().setLevel(logging.INFO) +logging.getLogger().addHandler(logging.StreamHandler()) + + +# Creates S3 bucket for tests and allows anonymous read-write access to it. +def prepare_s3_bucket(cluster): + minio_client = cluster.minio_client + + if minio_client.bucket_exists(cluster.minio_bucket): + minio_client.remove_bucket(cluster.minio_bucket) + + minio_client.make_bucket(cluster.minio_bucket) + + +@pytest.fixture(scope="module") +def cluster(): + try: + cluster = ClickHouseCluster(__file__) + + cluster.add_instance("node1", config_dir="configs", with_minio=True, with_zookeeper=True) + cluster.add_instance("node2", config_dir="configs") + cluster.add_instance("node3", config_dir="configs") + + logging.info("Starting cluster...") + cluster.start() + logging.info("Cluster started") + + prepare_s3_bucket(cluster) + logging.info("S3 bucket created") + + yield cluster + finally: + cluster.shutdown() + + +FILES_OVERHEAD = 1 +FILES_OVERHEAD_PER_COLUMN = 2 # Data and mark files +FILES_OVERHEAD_PER_PART = FILES_OVERHEAD_PER_COLUMN * 3 + 2 + 6 + + +def random_string(length): + letters = string.ascii_letters + return ''.join(random.choice(letters) for i in range(length)) + + +def generate_values(date_str, count, sign=1): + data = [[date_str, sign*(i + 1), random_string(10)] for i in range(count)] + data.sort(key=lambda tup: tup[1]) + return ",".join(["('{}',{},'{}')".format(x, y, z) for x, y, z in data]) + + +def create_table(cluster): + create_table_statement = """ + CREATE TABLE s3_test ( + dt Date, + id Int64, + data String, + INDEX min_max (id) TYPE minmax GRANULARITY 3 + ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/test_mutations', '{instance}') + PARTITION BY dt + ORDER BY (dt, id) + SETTINGS + old_parts_lifetime=0, index_granularity=512 + """ + + for node in cluster.instances: + node.query(create_table_statement) + + +@pytest.fixture(autouse=True) +def drop_table(cluster): + yield + for node in cluster.instances: + node.query("DROP TABLE IF EXISTS s3_test") + + minio = cluster.minio_client + assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + + +def test_insert_select_replicated(cluster): + create_table(cluster) + + all_values = "" + for node_idx in range(1, 4): + node = cluster.instances["node" + str(node_idx)] + values = generate_values("2020-01-0" + str(node_idx), 4096) + node.query("INSERT INTO s3_test VALUES {}".format(values)) + if node_idx != 1: + all_values += "," + all_values += values + + # Wait for replication + time.sleep(10) + + for node_idx in range(1, 4): + node = cluster.instances["node" + str(node_idx)] + assert node.query("SELECT * FROM s3_test order by dt, id FORMAT Values") == all_values + + minio = cluster.minio_client + assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 3 * (FILES_OVERHEAD + FILES_OVERHEAD_PER_PART * 3) From 89fe81ed62227f9643881389e8e393da25d19a98 Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Wed, 8 Apr 2020 14:56:31 +0300 Subject: [PATCH 3/7] Integration test for ReplicatedMergeTree over S3 fixes. --- src/Storages/MergeTree/DataPartsExchange.cpp | 3 +-- .../configs/config.xml | 20 --------------- .../test_replicated_merge_tree_s3/test.py | 25 ++++++++----------- 3 files changed, 12 insertions(+), 36 deletions(-) delete mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.xml diff --git a/src/Storages/MergeTree/DataPartsExchange.cpp b/src/Storages/MergeTree/DataPartsExchange.cpp index 4e40d4a5977..9ef7a4d37aa 100644 --- a/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/src/Storages/MergeTree/DataPartsExchange.cpp @@ -254,7 +254,6 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( const ReservationPtr reservation, PooledReadWriteBufferFromHTTP & in) { - size_t files; readBinary(files, in); @@ -285,7 +284,7 @@ MergeTreeData::MutableDataPartPtr Fetcher::downloadPart( /// File must be inside "absolute_part_path" directory. /// Otherwise malicious ClickHouse replica may force us to write to arbitrary path. String absolute_file_path = Poco::Path(part_download_path + file_name).absolute().toString(); - if (!startsWith(absolute_file_path, part_download_path)) + if (!startsWith(absolute_file_path, Poco::Path(part_download_path).absolute().toString())) throw Exception("File path (" + absolute_file_path + ") doesn't appear to be inside part path (" + part_download_path + ")." " This may happen if we are trying to download part from malicious replica or logical error.", ErrorCodes::INSECURE_PATH); diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.xml deleted file mode 100644 index 24b7344df3a..00000000000 --- a/tests/integration/test_replicated_merge_tree_s3/configs/config.xml +++ /dev/null @@ -1,20 +0,0 @@ - - - 9000 - 127.0.0.1 - - - - true - none - - AcceptCertificateHandler - - - - - 500 - 5368709120 - ./clickhouse/ - users.xml - diff --git a/tests/integration/test_replicated_merge_tree_s3/test.py b/tests/integration/test_replicated_merge_tree_s3/test.py index 53eb612c281..118a43a905e 100644 --- a/tests/integration/test_replicated_merge_tree_s3/test.py +++ b/tests/integration/test_replicated_merge_tree_s3/test.py @@ -25,9 +25,9 @@ def cluster(): try: cluster = ClickHouseCluster(__file__) - cluster.add_instance("node1", config_dir="configs", with_minio=True, with_zookeeper=True) - cluster.add_instance("node2", config_dir="configs") - cluster.add_instance("node3", config_dir="configs") + cluster.add_instance("node1", config_dir="configs", macros={'cluster': 'test1'}, with_minio=True, with_zookeeper=True) + cluster.add_instance("node2", config_dir="configs", macros={'cluster': 'test1'}, with_zookeeper=True) + cluster.add_instance("node3", config_dir="configs", macros={'cluster': 'test1'}, with_zookeeper=True) logging.info("Starting cluster...") cluster.start() @@ -64,25 +64,25 @@ def create_table(cluster): id Int64, data String, INDEX min_max (id) TYPE minmax GRANULARITY 3 - ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/test_mutations', '{instance}') + ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/s3', '{instance}') PARTITION BY dt ORDER BY (dt, id) SETTINGS old_parts_lifetime=0, index_granularity=512 """ - for node in cluster.instances: + for node in cluster.instances.values(): node.query(create_table_statement) @pytest.fixture(autouse=True) def drop_table(cluster): yield - for node in cluster.instances: - node.query("DROP TABLE IF EXISTS s3_test") + #for node in cluster.instances.values(): + # node.query("DROP TABLE IF EXISTS s3_test") - minio = cluster.minio_client - assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + #minio = cluster.minio_client + #assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 def test_insert_select_replicated(cluster): @@ -92,17 +92,14 @@ def test_insert_select_replicated(cluster): for node_idx in range(1, 4): node = cluster.instances["node" + str(node_idx)] values = generate_values("2020-01-0" + str(node_idx), 4096) - node.query("INSERT INTO s3_test VALUES {}".format(values)) + node.query("INSERT INTO s3_test VALUES {}".format(values), settings={"insert_quorum": 3}) if node_idx != 1: all_values += "," all_values += values - # Wait for replication - time.sleep(10) - for node_idx in range(1, 4): node = cluster.instances["node" + str(node_idx)] - assert node.query("SELECT * FROM s3_test order by dt, id FORMAT Values") == all_values + assert node.query("SELECT * FROM s3_test order by dt, id FORMAT Values", settings={"select_sequential_consistency": 1}) == all_values minio = cluster.minio_client assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 3 * (FILES_OVERHEAD + FILES_OVERHEAD_PER_PART * 3) From eca6caa8db919f63d6dd61fc77e6f7dc67dad0f2 Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Wed, 8 Apr 2020 15:48:16 +0300 Subject: [PATCH 4/7] Integration tests for MergeTree over S3 code cleanup. --- tests/integration/test_merge_tree_s3/test.py | 12 ++++++++---- .../configs/config.d/bg_processing_pool_conf.xml | 5 ----- .../configs/config.d/log_conf.xml | 12 ------------ .../configs/config.d/users.xml | 6 ------ .../test_replicated_merge_tree_s3/test.py | 15 +++++++++------ 5 files changed, 17 insertions(+), 33 deletions(-) delete mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml delete mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml delete mode 100644 tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml diff --git a/tests/integration/test_merge_tree_s3/test.py b/tests/integration/test_merge_tree_s3/test.py index f69c09631e8..e12e31ebff2 100644 --- a/tests/integration/test_merge_tree_s3/test.py +++ b/tests/integration/test_merge_tree_s3/test.py @@ -84,7 +84,12 @@ def drop_table(cluster): minio = cluster.minio_client node.query("DROP TABLE IF EXISTS s3_test") - assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + try: + assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + finally: + # Remove extra objects to prevent tests cascade failing + for obj in list(minio.list_objects(cluster.minio_bucket, 'data/')): + minio.remove_object(cluster.minio_bucket, obj.object_name) @pytest.mark.parametrize( @@ -210,7 +215,7 @@ def test_attach_detach_partition(cluster): assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == FILES_OVERHEAD + FILES_OVERHEAD_PER_PART_WIDE node.query("ALTER TABLE s3_test DETACH PARTITION '2020-01-04'") - node.query("SET allow_drop_detached=1; ALTER TABLE s3_test DROP DETACHED PARTITION '2020-01-04'") + node.query("ALTER TABLE s3_test DROP DETACHED PARTITION '2020-01-04'", settings={"allow_drop_detached": 1}) assert node.query("SELECT count(*) FROM s3_test FORMAT Values") == "(0)" assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == FILES_OVERHEAD @@ -245,8 +250,7 @@ def test_table_manipulations(cluster): assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == FILES_OVERHEAD + FILES_OVERHEAD_PER_PART_WIDE*2 node.query("RENAME TABLE s3_renamed TO s3_test") - # TODO: Doesn't work with min_max index. - #assert node.query("SET check_query_single_value_result='false'; CHECK TABLE s3_test FORMAT Values") == "(1)" + assert node.query("CHECK TABLE s3_test FORMAT Values") == "(1)" node.query("DETACH TABLE s3_test") node.query("ATTACH TABLE s3_test") diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml deleted file mode 100644 index a756c4434ea..00000000000 --- a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/bg_processing_pool_conf.xml +++ /dev/null @@ -1,5 +0,0 @@ - - 0.5 - 0.5 - 0.5 - diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml deleted file mode 100644 index 318a6bca95d..00000000000 --- a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/log_conf.xml +++ /dev/null @@ -1,12 +0,0 @@ - - 3 - - trace - /var/log/clickhouse-server/log.log - /var/log/clickhouse-server/log.err.log - 1000M - 10 - /var/log/clickhouse-server/stderr.log - /var/log/clickhouse-server/stdout.log - - diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml deleted file mode 100644 index a13b24b278d..00000000000 --- a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/users.xml +++ /dev/null @@ -1,6 +0,0 @@ - - - - - - diff --git a/tests/integration/test_replicated_merge_tree_s3/test.py b/tests/integration/test_replicated_merge_tree_s3/test.py index 118a43a905e..a8b7cf63e38 100644 --- a/tests/integration/test_replicated_merge_tree_s3/test.py +++ b/tests/integration/test_replicated_merge_tree_s3/test.py @@ -67,8 +67,6 @@ def create_table(cluster): ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/s3', '{instance}') PARTITION BY dt ORDER BY (dt, id) - SETTINGS - old_parts_lifetime=0, index_granularity=512 """ for node in cluster.instances.values(): @@ -78,11 +76,16 @@ def create_table(cluster): @pytest.fixture(autouse=True) def drop_table(cluster): yield - #for node in cluster.instances.values(): - # node.query("DROP TABLE IF EXISTS s3_test") + for node in cluster.instances.values(): + node.query("DROP TABLE IF EXISTS s3_test") - #minio = cluster.minio_client - #assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + minio = cluster.minio_client + try: + assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 + finally: + # Remove extra objects to prevent tests cascade failing + for obj in list(minio.list_objects(cluster.minio_bucket, 'data/')): + minio.remove_object(cluster.minio_bucket, obj.object_name) def test_insert_select_replicated(cluster): From ee36750482d6ddca2a3247679293e5fb4a622b29 Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Thu, 9 Apr 2020 18:36:13 +0300 Subject: [PATCH 5/7] Set storage policy explicitly in MergeTree over S3 tests. --- .../configs/config.d/storage_conf.xml | 4 ++-- tests/integration/test_merge_tree_s3/test.py | 4 +++- .../configs/config.d/storage_conf.xml | 11 ++--------- .../integration/test_replicated_merge_tree_s3/test.py | 10 ++++------ 4 files changed, 11 insertions(+), 18 deletions(-) diff --git a/tests/integration/test_merge_tree_s3/configs/config.d/storage_conf.xml b/tests/integration/test_merge_tree_s3/configs/config.d/storage_conf.xml index 5b292446c6b..d097675ca63 100644 --- a/tests/integration/test_merge_tree_s3/configs/config.d/storage_conf.xml +++ b/tests/integration/test_merge_tree_s3/configs/config.d/storage_conf.xml @@ -13,7 +13,7 @@ - +
s3 @@ -22,7 +22,7 @@ hdd - + diff --git a/tests/integration/test_merge_tree_s3/test.py b/tests/integration/test_merge_tree_s3/test.py index e12e31ebff2..4beb33604be 100644 --- a/tests/integration/test_merge_tree_s3/test.py +++ b/tests/integration/test_merge_tree_s3/test.py @@ -67,7 +67,9 @@ def create_table(cluster, table_name, additional_settings=None): PARTITION BY dt ORDER BY (dt, id) SETTINGS - old_parts_lifetime=0, index_granularity=512 + storage_policy = 's3', + old_parts_lifetime=0, + index_granularity=512 """.format(table_name) if additional_settings: diff --git a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml index 5b292446c6b..b32770095fc 100644 --- a/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml +++ b/tests/integration/test_replicated_merge_tree_s3/configs/config.d/storage_conf.xml @@ -7,22 +7,15 @@ minio minio123 - - local - / - - +
s3
- - hdd - - + diff --git a/tests/integration/test_replicated_merge_tree_s3/test.py b/tests/integration/test_replicated_merge_tree_s3/test.py index a8b7cf63e38..8689e7ccf5d 100644 --- a/tests/integration/test_replicated_merge_tree_s3/test.py +++ b/tests/integration/test_replicated_merge_tree_s3/test.py @@ -67,6 +67,7 @@ def create_table(cluster): ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/s3', '{instance}') PARTITION BY dt ORDER BY (dt, id) + SETTINGS storage_policy = 's3' """ for node in cluster.instances.values(): @@ -80,12 +81,9 @@ def drop_table(cluster): node.query("DROP TABLE IF EXISTS s3_test") minio = cluster.minio_client - try: - assert len(list(minio.list_objects(cluster.minio_bucket, 'data/'))) == 0 - finally: - # Remove extra objects to prevent tests cascade failing - for obj in list(minio.list_objects(cluster.minio_bucket, 'data/')): - minio.remove_object(cluster.minio_bucket, obj.object_name) + # Remove extra objects to prevent tests cascade failing + for obj in list(minio.list_objects(cluster.minio_bucket, 'data/')): + minio.remove_object(cluster.minio_bucket, obj.object_name) def test_insert_select_replicated(cluster): From 46c0b65c9542d41c80349c9c33b8bc6ddcad075e Mon Sep 17 00:00:00 2001 From: Pavel Kovalenko Date: Mon, 13 Apr 2020 12:20:38 +0300 Subject: [PATCH 6/7] Minor fix in MergeTree over S3 tests. --- tests/integration/test_merge_tree_s3/test.py | 2 +- tests/integration/test_replicated_merge_tree_s3/test.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_merge_tree_s3/test.py b/tests/integration/test_merge_tree_s3/test.py index 4beb33604be..50cf532e9a4 100644 --- a/tests/integration/test_merge_tree_s3/test.py +++ b/tests/integration/test_merge_tree_s3/test.py @@ -67,7 +67,7 @@ def create_table(cluster, table_name, additional_settings=None): PARTITION BY dt ORDER BY (dt, id) SETTINGS - storage_policy = 's3', + storage_policy='s3', old_parts_lifetime=0, index_granularity=512 """.format(table_name) diff --git a/tests/integration/test_replicated_merge_tree_s3/test.py b/tests/integration/test_replicated_merge_tree_s3/test.py index 8689e7ccf5d..d6b6015a388 100644 --- a/tests/integration/test_replicated_merge_tree_s3/test.py +++ b/tests/integration/test_replicated_merge_tree_s3/test.py @@ -67,7 +67,7 @@ def create_table(cluster): ) ENGINE=ReplicatedMergeTree('/clickhouse/{cluster}/tables/test/s3', '{instance}') PARTITION BY dt ORDER BY (dt, id) - SETTINGS storage_policy = 's3' + SETTINGS storage_policy='s3' """ for node in cluster.instances.values(): From 46a5c75bebf6e84ac88d359980e85055e0b8de91 Mon Sep 17 00:00:00 2001 From: alexey-milovidov Date: Sat, 18 Apr 2020 16:11:57 +0300 Subject: [PATCH 7/7] Update MergeTreeData.h --- src/Storages/MergeTree/MergeTreeData.h | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h index eb2a0dd8774..90eae080cb1 100644 --- a/src/Storages/MergeTree/MergeTreeData.h +++ b/src/Storages/MergeTree/MergeTreeData.h @@ -625,7 +625,6 @@ public: return storage_settings.get(); } - /// Get relative table path String getRelativeDataPath() const { return relative_data_path; } /// Get table path on disk