Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't check dependencies when renaming system tables automatically #48431

Merged
merged 3 commits into from Apr 6, 2023

Conversation

Algunenano
Copy link
Member

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

  • Don't check dependencies when renaming system tables automatically

Documentation entry for user-facing changes

If you are using check_referential_table_dependencies, have a MV over a system table and its schema changes, the automatic table rename fails. For example:

create table t1_dest (a Int64) ENGINE=MergeTree() order by a;
create materialized view t1_dest_mv TO t1_dest AS Select count() from system.query_log;

After changing the schema, on restart you'll get this every few seconds:

abr 05 16:09:20 Mordor clickhouse[364503]: 2023.04.05 14:09:20.197438 [ 364778 ] {} <Debug> SystemLog (system.query_log): Existing table system.query_log for system log has obsolete or different structure. Renaming it to query_log_22.
                                           Old: CREATE TABLE system.query_log (`type` Enum8('QueryStart' = 1, 'QueryFinish' = 2, 'ExceptionBeforeStart' = 3, 'ExceptionWhileProcessing' = 4), `event_date` Date, `event_time` DateTime, `event_time_microseconds` DateTime64(6), `query_start_time` DateTime, `query_start_time_microseconds` DateTime64(6), `query_duration_ms` UInt64, `read_rows` UInt64, `read_bytes` UInt64, `written_rows` UInt64, `written_bytes` UInt64, `result_rows` UInt64, `result_bytes` UInt64, `memory_usage` UInt64, `current_database` String, `query` String, `formatted_query` String, `normalized_query_hash` UInt64, `query_kind` LowCardinality(String), `databases` Array(LowCardinality(String)), `tables` Array(LowCardinality(String)), `columns` Array(LowCardinality(String)), `projections` Array(LowCardinality(String)), `views` Array(LowCardinality(String)), `exception_code` Int32, `exception` String, `stack_trace` String, `is_initial_query` UInt8, `user` String, `query_id` String, `address` IPv6, `port` UInt16, `initial_user` String, `initial_query_id` String, `initial_address` IPv6, `initial_port` UInt16, `initial_query_start_time` DateTime, `initial_query_start_time_microseconds` DateTime64(6), `interface` UInt8, `is_secure` UInt8, `os_user` String, `client_hostname` String, `client_name` String, `client_revision` UInt32, `client_version_major` UInt32, `client_version_minor` UInt32, `client_version_patch` UInt32, `http_method` UInt8, `http_user_agent` String, `http_referer` String, `forwarded_for` String, `quota_key` String, `distributed_depth` UInt64, `revision` UInt32, `log_comment` String, `thread_ids` Array(UInt64), `ProfileEvents` Map(String, UInt64), `Settings` Map(String, String), `used_aggregate_functions` Array(String), `used_aggregate_function_combinators` Array(String), `used_database_engines` Array(String), `used_data_type_families` Array(String), `used_dictionaries` Array(String), `used_formats` Array(String), `used_functions` Array(String), `used_storages` Array(String), `used_table_functions` Array(String), `used_row_policies` Array(LowCardinality(String)), `transaction_id` Tuple(UInt64, UInt64, UUID), `AsyncReadCounters` Map(String, UInt64), `ProfileEvents.Names` Array(String) ALIAS mapKeys(ProfileEvents), `ProfileEvents.Values` Array(UInt64) ALIAS mapValues(ProfileEvents), `Settings.Names` Array(String) ALIAS mapKeys(Settings), `Settings.Values` Array(String) ALIAS mapValues(Settings)) ENGINE = MergeTree PARTITION BY toStartOfWeek(event_date) ORDER BY (event_date, event_time) TTL toStartOfWeek(event_date) + toIntervalMonth(3) SETTINGS index_granularity = 8192
                                           New: CREATE TABLE system.query_log (`type` Enum8('QueryStart' = 1, 'QueryFinish' = 2, 'ExceptionBeforeStart' = 3, 'ExceptionWhileProcessing' = 4), `event_date` Date, `event_time` DateTime, `event_time_microseconds` DateTime64(6), `query_start_time` DateTime, `query_start_time_microseconds` DateTime64(6), `query_duration_ms` UInt64, `read_rows` UInt64, `read_bytes` UInt64, `written_rows` UInt64, `written_bytes` UInt64, `result_rows` UInt64, `result_bytes` UInt64, `memory_usage` UInt64, `current_database` String, `query` String, `formatted_query` String, `normalized_query_hash` UInt64, `query_kind` LowCardinality(String), `databases` Array(LowCardinality(String)), `tables` Array(LowCardinality(String)), `columns` Array(LowCardinality(String)), `projections` Array(LowCardinality(String)), `views` Array(LowCardinality(String)), `exception_code` Int32, `exception` String, `stack_trace` String, `is_initial_query` UInt8, `user` String, `query_id` String, `address` IPv6, `port` UInt16, `initial_user` String, `initial_query_id` String, `initial_address` IPv6, `initial_port` UInt16, `initial_query_start_time` DateTime, `initial_query_start_time_microseconds` DateTime64(6), `interface` UInt8, `is_secure` UInt8, `os_user` String, `client_hostname` String, `client_name` String, `client_revision` UInt32, `client_version_major` UInt32, `client_version_minor` UInt32, `client_version_patch` UInt32, `http_method` UInt8, `http_user_agent` String, `http_referer` String, `forwarded_for` String, `quota_key` String, `distributed_depth` UInt64, `revision` UInt32, `log_comment` String, `thread_ids` Array(UInt64), `ProfileEvents` Map(String, UInt64), `Settings` Map(String, String), `used_aggregate_functions` Array(String), `used_aggregate_function_combinators` Array(String), `used_database_engines` Array(String), `used_data_type_families` Array(String), `used_dictionaries` Array(String), `used_formats` Array(String), `used_functions` Array(String), `used_storages` Array(String), `used_table_functions` Array(String), `used_row_policies` Array(LowCardinality(String)), `transaction_id` Tuple(UInt64, UInt64, UUID), `AsyncReadCounters` Map(String, UInt64), `ProfileEvents.Names` Array(String) ALIAS mapKeys(ProfileEvents), `ProfileEvents.Values` Array(UInt64) ALIAS mapValues(ProfileEvents), `Settings.Names` Array(String) ALIAS mapKeys(Settings), `Settings.Values` Array(String) ALIAS mapValues(Settings)) ENGINE = MergeTree PARTITION BY toStartOfWeek(event_date) ORDER BY (event_date, event_time) TTL toStartOfWeek(event_date) + toIntervalMonth(2) SETTINGS index_granularity = 8192
                                           .
abr 05 16:09:20 Mordor clickhouse[364503]: 2023.04.05 14:09:20.205133 [ 364778 ] {} <Error> void DB::SystemLog<DB::QueryLogElement>::flushImpl(const std::vector<LogElement> &, uint64_t) [LogElement = DB::QueryLogElement]: Code: 630. DB::Exception: Cannot drop or rename system.query_log, because some tables depend on it: test_public_2a0041bf84164ebdb48f202bae030f93.bi_connector_log_view, default.t1_dest_mv, test_public_2a0041bf84164ebdb48f202bae030f93.usage_metrics_processed_log_view, public.processed_usage_log_from_query_log_view, public.usage_metrics_processed_log_view, public.bi_connector_log_view. (HAVE_DEPENDENT_OBJECTS), Stack trace (when copying this message, always include the lines below):
                                           
                                           0. ./build/./contrib/llvm-project/libcxx/include/exception:134: Poco::Exception::Exception(String const&, int) @ 0x1a91e553 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           1. ./build/./src/Common/Exception.cpp:91: DB::Exception::Exception(DB::Exception::MessageMasked&&, int, bool) @ 0x11600b63 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           2. ./build/./contrib/llvm-project/libcxx/include/string:1499: DB::Exception::Exception<DB::StorageID const&, fmt::v8::join_view<std::__wrap_iter<DB::StorageID*>, std::__wrap_iter<DB::StorageID*>, char>>(int, FormatStringHelperImpl<std::type_identity<DB::StorageID const&>::type, std::type_identity<fmt::v8::join_view<std::__wrap_iter<DB::StorageID*>, std::__wrap_iter<DB::StorageID*>, char>>::type>, DB::StorageID const&, fmt::v8::join_view<std::__wrap_iter<DB::StorageID*>, std::__wrap_iter<DB::StorageID*>, char>&&) @ 0x1654adbf in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           3. ./build/./src/Interpreters/DatabaseCatalog.cpp:1255: DB::DatabaseCatalog::checkTableCanBeRemovedOrRenamedUnlocked(DB::StorageID const&, bool, bool, bool) const @ 0x165471e8 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           4. ./build/./src/Interpreters/DatabaseCatalog.cpp:1215: DB::DatabaseCatalog::removeDependencies(DB::StorageID const&, bool, bool, bool) @ 0x16546dcf in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           5. ./build/./src/Interpreters/InterpreterRenameQuery.cpp:136: DB::InterpreterRenameQuery::executeToTables(DB::ASTRenameQuery const&, std::vector<DB::RenameDescription, std::allocator<DB::RenameDescription>> const&, std::map<DB::UniqueTableName, std::unique_ptr<DB::DDLGuard, std::default_delete<DB::DDLGuard>>, std::less<DB::UniqueTableName>, std::allocator<std::pair<DB::UniqueTableName const, std::unique_ptr<DB::DDLGuard, std::default_delete<DB::DDLGuard>>>>>&) @ 0x16e44f4e in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           6. ./build/./contrib/llvm-project/libcxx/include/__tree:1088: DB::InterpreterRenameQuery::execute() @ 0x16e43a9a in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           7. ./build/./src/Interpreters/SystemLog.cpp:0: DB::SystemLog<DB::QueryLogElement>::prepareTable() @ 0x1706761c in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           8. ./build/./contrib/llvm-project/libcxx/include/vector:666: DB::SystemLog<DB::QueryLogElement>::flushImpl(std::vector<DB::QueryLogElement, std::allocator<DB::QueryLogElement>> const&, unsigned long) @ 0x170694d6 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           9. ./build/./src/Interpreters/SystemLog.cpp:0: DB::SystemLog<DB::QueryLogElement>::savingThreadFunction() @ 0x17068a83 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           10. ./build/./src/Common/SystemLogBase.cpp:0: void std::__function::__policy_invoker<void ()>::__call_impl<std::__function::__default_alloc_func<ThreadFromGlobalPoolImpl<true>::ThreadFromGlobalPoolImpl<DB::ISystemLog::startup()::$_0>(DB::ISystemLog::startup()::$_0&&)::'lambda'(), void ()>>(std::__function::__policy_storage const*) @ 0x116ca2be in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           11. ./build/./base/base/../base/wide_integer_impl.h:789: ThreadPoolImpl<std::thread>::worker(std::__list_iterator<std::thread, void*>) @ 0x116bfe73 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           12. ./build/./contrib/llvm-project/libcxx/include/__memory/unique_ptr.h:302: void* std::__thread_proxy[abi:v15000]<std::tuple<std::unique_ptr<std::__thread_struct, std::default_delete<std::__thread_struct>>, void ThreadPoolImpl<std::thread>::scheduleImpl<void>(std::function<void ()>, long, std::optional<unsigned long>, bool)::'lambda0'()>>(void*) @ 0x116c3b40 in /mnt/ch/ClickHouse/build_default/programs/clickhouse
                                           13. ? @ 0x7f40a2293bb5 in ?
                                           14. ? @ 0x7f40a2315d90 in ?
                                            (version 23.4.1.1)

To avoid this we can disable check_referential_table_dependencies on those operation but I'm not sure if this is the best approach. What do you think @vitlibar?

@vitlibar vitlibar self-assigned this Apr 5, 2023
@@ -503,6 +503,7 @@ void SystemLog<LogElement>::prepareTable()
rename->elements.emplace_back(std::move(elem));

auto query_context = Context::createCopy(context);
query_context->setSetting("check_referential_table_dependencies", Field{false});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this change is ok because this operation is performed automatically and this is a system table. Please write a comment here, and also set check_table_dependencies to false for consistency.

@vitlibar
Copy link
Member

vitlibar commented Apr 6, 2023

@vitlibar vitlibar merged commit b45b662 into ClickHouse:master Apr 6, 2023
253 of 255 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants