Skip to content

Commit 4bb71b9

Browse files
committed
Fix crash in REFRESHABLE MV in case of ALTER after incorrect shutdown
The problem is that in case of incorrect shutdown, i.e.: 2025.04.03 22:42:03.620982 [ 14841 ] {632881cd-4918-414c-a2ed-27bb6beee664} <Error> executeQuery: Code: 219. DB::Exception: New table appeared in database being dropped or detached. Try again. (DATABASE_NOT_EMPTY) (version 25.4.1.1) (from [::1]:37582) (comment: 03258_refreshable_mv_misc.sh) (query 1, line 2) (in query: drop database test_15_03258;), Stack trace (when copying this message, always include the lines below): 0. ./contrib/llvm-project/libcxx/include/__exception/exception.h:113: Poco::Exception::Exception(String const&, int) @ 0x0000000020b619a0 1. ./ci/tmp/build/./src/Common/Exception.cpp:108: DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x0000000010bd82d4 2. DB::Exception::Exception(PreformattedMessage&&, int) @ 0x00000000086edcc0 3. DB::Exception::Exception<>(int, FormatStringHelperImpl<>) @ 0x00000000086fc67a 4. ./ci/tmp/build/./src/Interpreters/DatabaseCatalog.cpp:615: DB::DatabaseCatalog::detachDatabase(std::shared_ptr<DB::Context const>, String const&, bool, bool) @ 0x00000000180a72d7 5. ./ci/tmp/build/./src/Interpreters/InterpreterDropQuery.cpp:535: DB::InterpreterDropQuery::executeToDatabaseImpl(DB::ASTDropQuery const&, std::shared_ptr<DB::IDatabase>&, std::vector<StrongTypedef<wide::integer<128ul, unsigned int>, DB::UUIDTag>, std::allocator<StrongTypedef<wide::integer<128ul, unsigned int>, DB::UUIDTag>>>&) @ 0x00000000186c1105 6. ./ci/tmp/build/./src/Interpreters/InterpreterDropQuery.cpp:364: DB::InterpreterDropQuery::executeToDatabase(DB::ASTDropQuery const&) @ 0x00000000186bbe78 7. ./ci/tmp/build/./src/Interpreters/InterpreterDropQuery.cpp:100: DB::InterpreterDropQuery::executeSingleDropQuery(std::shared_ptr<DB::IAST> const&) @ 0x00000000186bae97 8. ./ci/tmp/build/./src/Interpreters/InterpreterDropQuery.cpp:73: DB::InterpreterDropQuery::execute() @ 0x00000000186baab0 9. ./ci/tmp/build/./src/Interpreters/executeQuery.cpp:1458: DB::executeQueryImpl(char const*, char const*, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum, DB::ReadBuffer*, std::shared_ptr<DB::IAST>&) @ 0x0000000018be45da 10. ./ci/tmp/build/./src/Interpreters/executeQuery.cpp:1625: DB::executeQuery(String const&, std::shared_ptr<DB::Context>, DB::QueryFlags, DB::QueryProcessingStage::Enum) @ 0x0000000018bdf1ef 11. ./ci/tmp/build/./src/Server/TCPHandler.cpp:665: DB::TCPHandler::runImpl() @ 0x000000001b9cb665 12. ./ci/tmp/build/./src/Server/TCPHandler.cpp:2630: DB::TCPHandler::run() @ 0x000000001b9f23c8 13. ./ci/tmp/build/./base/poco/Net/src/TCPServerConnection.cpp:40: Poco::Net::TCPServerConnection::start() @ 0x0000000020c71da3 14. ./ci/tmp/build/./base/poco/Net/src/TCPServerDispatcher.cpp:115: Poco::Net::TCPServerDispatcher::run() @ 0x0000000020c72612 15. ./ci/tmp/build/./base/poco/Foundation/src/ThreadPool.cpp:205: Poco::PooledThread::run() @ 0x0000000020be7cc3 16. ./ci/tmp/build/./base/poco/Foundation/src/Thread.cpp:45: Poco::(anonymous namespace)::RunnableHolder::run() @ 0x0000000020be6070 17. ./base/poco/Foundation/src/Thread_POSIX.cpp:335: Poco::ThreadImpl::runnableEntry(void*) @ 0x0000000020be442a 18. __tsan_thread_start_func @ 0x0000000008661428 19. ? @ 0x00007f396f9fdac3 20. ? @ 0x00007f396fa8f850 The table will still exist, but the view will be NULL due to shutdown() had been called already. And after this any ALTER of it will lead to crash. So fix this by obtaining info for notifying under lock P.S. I've looked through other places and it is either called under refreshTask() (which should not be called after shutdown()) or already has check for `view` under mutex. Fixes: ClickHouse#78103 v2: do not call RefreshSet::notifyDependents() under lock (will lead to lock order inversion, found by @al13n321)
1 parent 272f2ea commit 4bb71b9

File tree

1 file changed

+11
-1
lines changed

1 file changed

+11
-1
lines changed

src/Storages/MaterializedView/RefreshTask.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,8 @@ void RefreshTask::checkAlterIsPossible(const DB::ASTRefreshStrategy & new_strate
216216

217217
void RefreshTask::alterRefreshParams(const DB::ASTRefreshStrategy & new_strategy)
218218
{
219+
StorageID view_storage_id = StorageID::createEmpty();
220+
219221
{
220222
std::lock_guard guard(mutex);
221223

@@ -235,9 +237,17 @@ void RefreshTask::alterRefreshParams(const DB::ASTRefreshStrategy & new_strategy
235237
refresh_settings = {};
236238
if (new_strategy.settings != nullptr)
237239
refresh_settings.applyChanges(new_strategy.settings->changes);
240+
241+
if (view)
242+
view_storage_id = view->getStorageID();
238243
}
244+
239245
/// In case refresh period changed.
240-
view->getContext()->getRefreshSet().notifyDependents(view->getStorageID());
246+
if (view_storage_id)
247+
{
248+
const auto & refresh_set = Context::getGlobalContextInstance()->getRefreshSet();
249+
refresh_set.notifyDependents(view_storage_id);
250+
}
241251
}
242252

243253
RefreshTask::Info RefreshTask::getInfo() const

0 commit comments

Comments
 (0)