From d16adddb417f55a4caeccd59b4fbf8d489356b19 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Wed, 7 Oct 2020 22:09:51 +0300 Subject: [PATCH] Fix drop of materialized view with inner table in Atomic database Materialized view drop the inner table (if any) drop the drop() method, and this will lead to essure recursive drop() from the drop worker thread, which will hang: 3 std::__1::condition_variable::wait<> () 4 DB::DatabaseCatalog::waitTableFinallyDropped (this=0x7ffff7831500, uuid=...) at ../src/Interpreters/DatabaseCatalog.cpp:828 5 DB::InterpreterDropQuery::executeToTable (this=this@entry=0x7fff16ff88a0, table_id_=..., query=...) at ../src/Interpreters/InterpreterDropQuery.cpp:135 6 DB::InterpreterDropQuery::execute (this=this@entry=0x7fff16ff88a0) at ../contrib/libcxx/include/string:1474 7 DB::executeDropQuery (kind=kind@entry=DB::ASTDropQuery::Drop, global_context=..., target_table_id=...) at ../src/Storages/StorageMaterializedView.cpp:156 8 DB::StorageMaterializedView::drop (this=0x7ffefc348f40) at ../src/Storages/StorageMaterializedView.cpp:169 9 DB::DatabaseCatalog::dropTableFinally (this=this@entry=0x7ffff7831500, table=...) at ../src/Interpreters/DatabaseCatalog.cpp:775 10 DB::DatabaseCatalog::dropTableDataTask (this=0x7ffff7831500) at ../src/Interpreters/DatabaseCatalog.cpp:745 Fix this by dropping the inner table just before scheduling job the the drop worker thread. --- src/Databases/DatabaseAtomic.cpp | 4 ++++ src/Storages/StorageMaterializedView.cpp | 14 ++++++++++---- src/Storages/StorageMaterializedView.h | 1 + 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Databases/DatabaseAtomic.cpp b/src/Databases/DatabaseAtomic.cpp index 46d56e97436..a9dbae8ec92 100644 --- a/src/Databases/DatabaseAtomic.cpp +++ b/src/Databases/DatabaseAtomic.cpp @@ -115,6 +115,10 @@ void DatabaseAtomic::dropTable(const Context &, const String & table_name, bool table_name_to_path.erase(table_name); } tryRemoveSymlink(table_name); + /// Remove the inner table (if any) to avoid deadlock + /// (due to attemp to execute DROP from the worker thread) + if (auto * mv = dynamic_cast(table.get())) + mv->dropInnerTable(no_delay); /// Notify DatabaseCatalog that table was dropped. It will remove table data in background. /// Cleanup is performed outside of database to allow easily DROP DATABASE without waiting for cleanup to complete. DatabaseCatalog::instance().enqueueDroppedTableCleanup(table->getStorageID(), table, table_metadata_path_drop, no_delay); diff --git a/src/Storages/StorageMaterializedView.cpp b/src/Storages/StorageMaterializedView.cpp index a2e3fae0951..2488c07304f 100644 --- a/src/Storages/StorageMaterializedView.cpp +++ b/src/Storages/StorageMaterializedView.cpp @@ -141,7 +141,7 @@ BlockOutputStreamPtr StorageMaterializedView::write(const ASTPtr & query, const } -static void executeDropQuery(ASTDropQuery::Kind kind, Context & global_context, const StorageID & target_table_id) +static void executeDropQuery(ASTDropQuery::Kind kind, Context & global_context, const StorageID & target_table_id, bool no_delay) { if (DatabaseCatalog::instance().tryGetTable(target_table_id, global_context)) { @@ -150,7 +150,7 @@ static void executeDropQuery(ASTDropQuery::Kind kind, Context & global_context, drop_query->database = target_table_id.database_name; drop_query->table = target_table_id.table_name; drop_query->kind = kind; - drop_query->no_delay = true; + drop_query->no_delay = no_delay; ASTPtr ast_drop_query = drop_query; InterpreterDropQuery drop_interpreter(ast_drop_query, global_context); drop_interpreter.execute(); @@ -165,14 +165,20 @@ void StorageMaterializedView::drop() if (!select_query.select_table_id.empty()) DatabaseCatalog::instance().removeDependency(select_query.select_table_id, table_id); + dropInnerTable(true); +} + +void StorageMaterializedView::dropInnerTable(bool no_delay) +{ if (has_inner_table && tryGetTargetTable()) - executeDropQuery(ASTDropQuery::Kind::Drop, global_context, target_table_id); + executeDropQuery(ASTDropQuery::Kind::Drop, global_context, target_table_id, no_delay); + has_inner_table = false; } void StorageMaterializedView::truncate(const ASTPtr &, const StorageMetadataPtr &, const Context &, TableExclusiveLockHolder &) { if (has_inner_table) - executeDropQuery(ASTDropQuery::Kind::Truncate, global_context, target_table_id); + executeDropQuery(ASTDropQuery::Kind::Truncate, global_context, target_table_id, true); } void StorageMaterializedView::checkStatementCanBeForwarded() const diff --git a/src/Storages/StorageMaterializedView.h b/src/Storages/StorageMaterializedView.h index 1ee4246c7f1..c5188108c18 100644 --- a/src/Storages/StorageMaterializedView.h +++ b/src/Storages/StorageMaterializedView.h @@ -36,6 +36,7 @@ public: BlockOutputStreamPtr write(const ASTPtr & query, const StorageMetadataPtr & /*metadata_snapshot*/, const Context & context) override; void drop() override; + void dropInnerTable(bool no_delay); void truncate(const ASTPtr &, const StorageMetadataPtr &, const Context &, TableExclusiveLockHolder &) override;