diff --git a/src/Interpreters/InterpreterSystemQuery.cpp b/src/Interpreters/InterpreterSystemQuery.cpp index 123ff6ba2ca..df0c5635af9 100644 --- a/src/Interpreters/InterpreterSystemQuery.cpp +++ b/src/Interpreters/InterpreterSystemQuery.cpp @@ -56,7 +56,6 @@ namespace ErrorCodes extern const int NOT_IMPLEMENTED; extern const int TIMEOUT_EXCEEDED; extern const int TABLE_WAS_NOT_DROPPED; - extern const int NO_ZOOKEEPER; } @@ -472,12 +471,6 @@ void InterpreterSystemQuery::restoreReplica() { getContext()->checkAccess(AccessType::SYSTEM_RESTORE_REPLICA, table_id); - const zkutil::ZooKeeperPtr & zookeeper = getContext()->getZooKeeper(); - - if (zookeeper->expired()) - throw Exception(ErrorCodes::NO_ZOOKEEPER, - "Cannot restore table metadata because ZooKeeper session has expired"); - const StoragePtr table_ptr = DatabaseCatalog::instance().getTable(table_id, getContext()); auto * const table_replicated_ptr = dynamic_cast(table_ptr.get()); @@ -485,24 +478,7 @@ void InterpreterSystemQuery::restoreReplica() if (table_replicated_ptr == nullptr) throw Exception(ErrorCodes::BAD_ARGUMENTS, table_is_not_replicated.data(), table_id.getNameForLogs()); - auto & table_replicated = *table_replicated_ptr; - - StorageReplicatedMergeTree::Status status; - table_replicated.getStatus(status); - - if (!status.is_readonly) - throw Exception(ErrorCodes::BAD_ARGUMENTS, "Replica must be readonly"); - - const String replica_name = table_replicated.getReplicaName(); - const String& zk_root_path = status.zookeeper_path; - - if (String replica_path = zk_root_path + "replicas/" + replica_name; zookeeper->exists(replica_path)) - throw Exception(ErrorCodes::BAD_ARGUMENTS, - "Replica path is present at {} -- nothing to restore. " - "If you are sure that metadata it lost and replica path contain some garbage, " - "then use SYSTEM DROP REPLICA query first.", replica_path); - - table_replicated.restoreMetadataInZooKeeper(); + table_replicated_ptr->restoreMetadataInZooKeeper(); } StoragePtr InterpreterSystemQuery::tryRestartReplica(const StorageID & replica, ContextMutablePtr system_context, bool need_ddl_guard) diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp index a23859e7b5e..5379c3f89c1 100644 --- a/src/Storages/StorageReplicatedMergeTree.cpp +++ b/src/Storages/StorageReplicatedMergeTree.cpp @@ -774,7 +774,8 @@ void StorageReplicatedMergeTree::drop() /// or metadata of staled replica were removed manually, /// in this case, has_metadata_in_zookeeper = false, and we also permit to drop the table. - if (has_metadata_in_zookeeper) + bool maybe_has_metadata_in_zookeeper = !has_metadata_in_zookeeper.has_value() || *has_metadata_in_zookeeper; + if (maybe_has_metadata_in_zookeeper) { /// Table can be shut down, restarting thread is not active /// and calling StorageReplicatedMergeTree::getZooKeeper()/getAuxiliaryZooKeeper() won't suffice. @@ -4801,12 +4802,22 @@ bool StorageReplicatedMergeTree::getFakePartCoveringAllPartsInPartition(const St void StorageReplicatedMergeTree::restoreMetadataInZooKeeper() { LOG_INFO(log, "Restoring replica metadata"); + if (!is_readonly) + throw Exception(ErrorCodes::BAD_ARGUMENTS, "Replica must be readonly"); - if (!is_readonly || has_metadata_in_zookeeper) - throw Exception(ErrorCodes::LOGICAL_ERROR, "It's a bug: replica is not readonly"); + if (getZooKeeper()->exists(replica_path)) + throw Exception(ErrorCodes::BAD_ARGUMENTS, + "Replica path is present at {} - nothing to restore. " + "If you are sure that metadata is lost and replica path contain some garbage, " + "then use SYSTEM DROP REPLICA query first.", replica_path); + + if (has_metadata_in_zookeeper.has_value() && *has_metadata_in_zookeeper) + throw Exception(ErrorCodes::LOGICAL_ERROR, "Replica has metadata in ZooKeeper: " + "it's a bug or a result of manual intervention into ZooKeeper"); if (are_restoring_replica.exchange(true)) throw Exception(ErrorCodes::CONCURRENT_ACCESS_NOT_SUPPORTED, "Replica restoration in progress"); + SCOPE_EXIT({ are_restoring_replica.store(false); }); auto metadata_snapshot = getInMemoryMetadataPtr(); @@ -4847,8 +4858,6 @@ void StorageReplicatedMergeTree::restoreMetadataInZooKeeper() LOG_INFO(log, "Attached all partitions, starting table"); startup(); - - are_restoring_replica.store(false); } void StorageReplicatedMergeTree::dropPartNoWaitNoThrow(const String & part_name) diff --git a/src/Storages/StorageReplicatedMergeTree.h b/src/Storages/StorageReplicatedMergeTree.h index c91152ca0f3..54a1d897bd7 100644 --- a/src/Storages/StorageReplicatedMergeTree.h +++ b/src/Storages/StorageReplicatedMergeTree.h @@ -322,7 +322,7 @@ private: /// If true, the table is offline and can not be written to it. std::atomic_bool is_readonly {false}; /// If false - ZooKeeper is available, but there is no table metadata. It's safe to drop table in this case. - bool has_metadata_in_zookeeper = true; + std::optional has_metadata_in_zookeeper; static constexpr auto default_zookeeper_name = "default"; String zookeeper_name;