From c3dd968931e31db7bc59483b85e67acc961dbafd Mon Sep 17 00:00:00 2001 From: bharatnc Date: Mon, 31 Aug 2020 21:42:27 -0700 Subject: [PATCH] fix ALTER LIVE VIEW lock issue This PR fixes a lock issue that happens while executing `ALTER LIVE VIEW` query with the `REFRESH` command that results in a exception. The problem is that lock is currently being acquired in `InterpreterALterQuery.cpp` in the `InterpreterAlterQuery::execute()` method and lock is again being reacquired in `StorageLiveView.cpp` in the ` StorageLiveView::refresh` method. This removes that extra lock. Before fix: ```sql --create table CREATE TABLE test0 ( c0 UInt64 ) ENGINE = MergeTree() PARTITION BY c0 ORDER BY c0; -- enable experimental_live_view :) SET allow_experimental_live_view=1 -- create live view; :) CREATE LIVE VIEW live1 AS SELECT * FROM table0; -- alter live view results in exception :) ALTER LIVE VIEW live1 REFRESH; ... ... Received exception from server (version 20.8.1): Code: 49. DB::Exception: Received from localhost:9000. DB::Exception: RWLockImpl::getLock(): RWLock is already locked in exclusive mode. ``` After fix: ```sql :) ALTER LIVE VIEW live1 REFRESH; ALTER LIVE VIEW live1 REFRESH Ok. 0 rows in set. Elapsed: 0.016 sec. ``` --- src/Interpreters/InterpreterAlterQuery.cpp | 2 +- src/Storages/LiveView/StorageLiveView.cpp | 5 +++-- src/Storages/LiveView/StorageLiveView.h | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Interpreters/InterpreterAlterQuery.cpp b/src/Interpreters/InterpreterAlterQuery.cpp index e0313803e9a..8cf581eb463 100644 --- a/src/Interpreters/InterpreterAlterQuery.cpp +++ b/src/Interpreters/InterpreterAlterQuery.cpp @@ -101,7 +101,7 @@ BlockIO InterpreterAlterQuery::execute() switch (command.type) { case LiveViewCommand::REFRESH: - live_view->refresh(context); + live_view->refresh(); break; } } diff --git a/src/Storages/LiveView/StorageLiveView.cpp b/src/Storages/LiveView/StorageLiveView.cpp index 54ac5bcc791..4da02365232 100644 --- a/src/Storages/LiveView/StorageLiveView.cpp +++ b/src/Storages/LiveView/StorageLiveView.cpp @@ -518,9 +518,10 @@ void StorageLiveView::drop() condition.notify_all(); } -void StorageLiveView::refresh(const Context & context) +void StorageLiveView::refresh() { - auto table_lock = lockExclusively(context.getCurrentQueryId(), context.getSettingsRef().lock_acquire_timeout); + // Lock is already acquired exclusively from InterperterAlterQuery.cpp InterpreterAlterQuery::execute() method. + // So, reacquiring lock is not needed and will result in an exception. { std::lock_guard lock(mutex); if (getNewBlocks()) diff --git a/src/Storages/LiveView/StorageLiveView.h b/src/Storages/LiveView/StorageLiveView.h index 43afd169a92..0c099d01a29 100644 --- a/src/Storages/LiveView/StorageLiveView.h +++ b/src/Storages/LiveView/StorageLiveView.h @@ -122,7 +122,7 @@ public: void startup() override; void shutdown() override; - void refresh(const Context & context); + void refresh(); Pipe read( const Names & column_names,