Merge pull request ClickHouse#78405 from jaymebrd/fail-fast-system-sync-replica-lightweight-from

fix: fail `system sync replica lightweight from foo` if replica foo doesn't exist

Author: tavplubix

src/Storages/MergeTree/ReplicatedMergeTreeQueue.cpp

@@ -42,6 +42,7 @@ namespace ErrorCodes
     extern const int LOGICAL_ERROR;
     extern const int UNEXPECTED_NODE_IN_ZOOKEEPER;
     extern const int ABORTED;
+    extern const int BAD_ARGUMENTS;
 }
 
 
@@ -2424,10 +2425,26 @@ ReplicatedMergeTreeQueue::addSubscriber(ReplicatedMergeTreeQueue::SubscriberCall
         std::unordered_set<String> existing_replicas;
         if (!src_replicas.empty())
         {
-            Strings unfiltered_hosts;
-            unfiltered_hosts = storage.getZooKeeper()->getChildren(zookeeper_path + "/replicas");
+            Strings unfiltered_hosts = storage.getZooKeeper()->getChildren(zookeeper_path + "/replicas");
+            existing_replicas.reserve(unfiltered_hosts.size());
+
             for (const auto & host : unfiltered_hosts)
-                existing_replicas.insert(host);
+                existing_replicas.emplace(host);
+
+            for (const auto & requested_replica : src_replicas)
+            {
+                if (!existing_replicas.contains(requested_replica))
+                {
+                    throw Exception(
+                        ErrorCodes::BAD_ARGUMENTS,
+                        "SYSTEM SYNC REPLICA FROM '{}' failed: replica does not exist. "
+                        "Available replicas at '{}/replicas': {}",
+                        requested_replica,
+                        zookeeper_path,
+                        fmt::join(unfiltered_hosts, ", ")
+                    );
+                }
+            }
         }
 
         out_entry_names.reserve(queue.size());

tests/queries/0_stateless/02962_system_sync_replica_lightweight_from_modifier.reference

@@ -1,3 +1,5 @@
+Testing sync from non-existent replica...
+Error correctly detected for non-existent replica
 Replication did not hang: synced all replicas of test_table_
 Consistency: 1
 Test completed

tests/queries/0_stateless/02962_system_sync_replica_lightweight_from_modifier.sh

@@ -1,5 +1,5 @@
 #!/usr/bin/env bash
-# Tags: zookeeper, no-parallel, no-fasttest
+# Tags: zookeeper, no-parallel, no-fasttest, no-shared-merge-tree
 
 CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 # shellcheck source=../shell_config.sh
@@ -57,13 +57,33 @@ function mutations_thread() {
     done
 }
 
+function consistency_table_sync_non_existent_replica() {
+    echo "Testing sync from non-existent replica..."
+    local NON_EXISTENT_REPLICA="non_existent_replica_$RANDOM"
+
+    SYNC_OUTPUT=$($CLICKHOUSE_CLIENT --query "SYSTEM SYNC REPLICA test_table_1 LIGHTWEIGHT FROM '$NON_EXISTENT_REPLICA'" 2>&1)
+    EXIT_CODE=$?
+
+    if ! echo "$SYNC_OUTPUT" | grep -q "failed: replica does not exist"; then
+        echo "FAILED: Error was not detected properly for non-existent replica"
+        echo
+        echo "Full output from SYSTEM SYNC command (exit code $EXIT_CODE):"
+        echo
+        echo "$SYNC_OUTPUT"
+        exit 1
+    fi
+
+    echo "Error correctly detected for non-existent replica"
+}
+
 export -f insert_thread
 export -f sync_and_drop_replicas
 export -f optimize_thread
 export -f mutations_thread
 
 TIMEOUT=30
 
+consistency_table_sync_non_existent_replica
 timeout $TIMEOUT bash -c insert_thread 2> /dev/null &
 timeout $TIMEOUT bash -c sync_and_drop_replicas 2> /dev/null &
 timeout $TIMEOUT bash -c optimize_thread 2> /dev/null &