From 599627e83bf7456e07d7a4af87e9fc2ed2cd7a16 Mon Sep 17 00:00:00 2001 From: Anton Ivashkin Date: Wed, 19 Mar 2025 13:04:12 +0100 Subject: [PATCH] Fix mutations after turning on zero-copy --- src/Storages/MergeTree/DataPartsExchange.cpp | 2 + src/Storages/StorageReplicatedMergeTree.cpp | 4 ++ ...ations_after_zero_copy_turned_on.reference | 1 + ...391_mutations_after_zero_copy_turned_on.sh | 33 ++++++++++++++++ ...cation_after_zero_copy_turned_on.reference | 3 ++ ..._replication_after_zero_copy_turned_on.sql | 39 +++++++++++++++++++ 6 files changed, 82 insertions(+) create mode 100644 tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.reference create mode 100755 tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.sh create mode 100644 tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.reference create mode 100644 tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.sql diff --git a/src/Storages/MergeTree/DataPartsExchange.cpp b/src/Storages/MergeTree/DataPartsExchange.cpp index 061ee3562038..8196a37c18b9 100644 --- a/src/Storages/MergeTree/DataPartsExchange.cpp +++ b/src/Storages/MergeTree/DataPartsExchange.cpp @@ -208,6 +208,8 @@ void Service::processQuery(const HTMLForm & params, ReadBuffer & /*body*/, Write auto disk_type = part->getDataPartStorage().getDiskType(); if (part->getDataPartStorage().supportZeroCopyReplication() && std::find(capabilities.begin(), capabilities.end(), disk_type) != capabilities.end()) { + data.lockSharedData(*part, false, {}); + /// Send metadata if the receiver's capabilities covers the source disk type. response.addCookie({"remote_fs_metadata", disk_type}); sendPartFromDisk(part, out, client_protocol_version, true, send_projections); diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index fa25194aadeb..2d9f6bb553a3 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -10300,6 +10300,8 @@ void StorageReplicatedMergeTree::getZeroCopyLockNodeCreateOps( requests.emplace_back(zkutil::makeCreateRequest(zookeeper_node, "", mode)); if (!path_to_set_hardlinked_files.empty() && !hardlinked_files.empty()) { + /// Node may be missing in rare cases when zero-copy is turned on for an existing table. + zookeeper->checkExistsAndGetCreateAncestorsOps(path_to_set_hardlinked_files + "/", requests); std::string data = boost::algorithm::join(hardlinked_files, "\n"); /// List of files used to detect hardlinks. path_to_set_hardlinked_files -- /// is a path to source part zero copy node. During part removal hardlinked @@ -10312,6 +10314,8 @@ void StorageReplicatedMergeTree::getZeroCopyLockNodeCreateOps( Coordination::Requests ops; if (!path_to_set_hardlinked_files.empty() && !hardlinked_files.empty()) { + /// Node may be missing in rare cases when zero-copy is turned on for an existing table. + zookeeper->checkExistsAndGetCreateAncestorsOps(path_to_set_hardlinked_files + "/", requests); std::string data = boost::algorithm::join(hardlinked_files, "\n"); /// List of files used to detect hardlinks. path_to_set_hardlinked_files -- /// is a path to source part zero copy node. During part removal hardlinked diff --git a/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.reference b/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.reference new file mode 100644 index 000000000000..d00491fd7e5b --- /dev/null +++ b/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.reference @@ -0,0 +1 @@ +1 diff --git a/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.sh b/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.sh new file mode 100755 index 000000000000..b7b46c408af7 --- /dev/null +++ b/tests/queries/0_stateless/03391_mutations_after_zero_copy_turned_on.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Tags: no-parallel, no-fasttest + +set -e + +CUR_DIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) +# shellcheck source=../shell_config.sh +. "$CUR_DIR"/../shell_config.sh + +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS test03391_zero_copy_mutations" +$CLICKHOUSE_CLIENT -m -q " +CREATE TABLE test03391_zero_copy_mutations + (key UInt32, value UInt32) + ENGINE=ReplicatedMergeTree('/clickhouse/tables/$CLICKHOUSE_TEST_ZOOKEEPER_PREFIX/test03391_zero_copy_mutations', 'r1') + ORDER BY key + SETTINGS storage_policy='s3_cache', allow_remote_fs_zero_copy_replication=0 +" + +$CLICKHOUSE_CLIENT -q "INSERT INTO test03391_zero_copy_mutations VALUES (1,1)" +$CLICKHOUSE_CLIENT -q "ALTER TABLE test03391_zero_copy_mutations MODIFY SETTING allow_remote_fs_zero_copy_replication=1" +$CLICKHOUSE_CLIENT -q "ALTER TABLE test03391_zero_copy_mutations UPDATE value=value WHERE 0" + +for i in {1..10} +do + if [ "$(${CLICKHOUSE_CLIENT} -q "SELECT count() FROM system.mutations WHERE table='test03391_zero_copy_mutations' AND is_done=1")" -eq 1 ]; then + break + fi + sleep 1 +done + +echo '1' | $CLICKHOUSE_CLIENT -q "SELECT count() FROM system.mutations WHERE table='test03391_zero_copy_mutations' AND is_done=1" + +$CLICKHOUSE_CLIENT -q "DROP TABLE IF EXISTS test03391_zero_copy_mutations" diff --git a/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.reference b/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.reference new file mode 100644 index 000000000000..e8183f05f5db --- /dev/null +++ b/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.reference @@ -0,0 +1,3 @@ +1 +1 +1 diff --git a/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.sql b/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.sql new file mode 100644 index 000000000000..91e816b3f8a4 --- /dev/null +++ b/tests/queries/0_stateless/03392_replication_after_zero_copy_turned_on.sql @@ -0,0 +1,39 @@ +-- Tags: no-parallel + +DROP TABLE IF EXISTS test03392_zero_copy_replication_r1; +DROP TABLE IF EXISTS test03392_zero_copy_replication_r2; +DROP TABLE IF EXISTS test03392_zero_copy_replication_r3; + +CREATE TABLE test03392_zero_copy_replication_r1 (x UInt64) + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{database}/test03392_zero_copy_replication','1') + ORDER BY x + SETTINGS storage_policy='s3_cache', allow_remote_fs_zero_copy_replication=0; + +INSERT INTO test03392_zero_copy_replication_r1 VALUES (1); + +CREATE TABLE test03392_zero_copy_replication_r2 (x UInt64) + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{database}/test03392_zero_copy_replication','2') + ORDER BY x + SETTINGS storage_policy='s3_cache', allow_remote_fs_zero_copy_replication=0; + +SYSTEM SYNC REPLICA test03392_zero_copy_replication_r2; + +ALTER TABLE test03392_zero_copy_replication_r1 MODIFY SETTING allow_remote_fs_zero_copy_replication=1; +ALTER TABLE test03392_zero_copy_replication_r2 MODIFY SETTING allow_remote_fs_zero_copy_replication=1; + +CREATE TABLE test03392_zero_copy_replication_r3 (x UInt64) + ENGINE = ReplicatedMergeTree('/clickhouse/tables/{database}/test03392_zero_copy_replication','3') + ORDER BY x + SETTINGS storage_policy='s3_cache', allow_remote_fs_zero_copy_replication=1; + +SYSTEM SYNC REPLICA test03392_zero_copy_replication_r3; + +SELECT count() FROM test03392_zero_copy_replication_r1; +SELECT count() FROM test03392_zero_copy_replication_r2; +SELECT count() FROM test03392_zero_copy_replication_r3; + +SYSTEM START MERGES; + +DROP TABLE IF EXISTS test03392_zero_copy_replication_r1; +DROP TABLE IF EXISTS test03392_zero_copy_replication_r2; +DROP TABLE IF EXISTS test03392_zero_copy_replication_r3;