diff --git a/programs/local/LocalServer.cpp b/programs/local/LocalServer.cpp index 133d629bbb1..7d7fd8ff32d 100644 --- a/programs/local/LocalServer.cpp +++ b/programs/local/LocalServer.cpp @@ -654,8 +654,9 @@ void LocalServer::processConfig() if (!config().has("only-system-tables")) { + DatabaseCatalog::instance().createBackgroundTasks(); loadMetadata(global_context); - DatabaseCatalog::instance().loadDatabases(); + DatabaseCatalog::instance().startupBackgroundCleanup(); } /// For ClickHouse local if path is not set the loader will be disabled. diff --git a/programs/server/Server.cpp b/programs/server/Server.cpp index b97b48d9c68..34d266b6519 100644 --- a/programs/server/Server.cpp +++ b/programs/server/Server.cpp @@ -1656,11 +1656,12 @@ try /// that may execute DROP before loadMarkedAsDroppedTables() in background, /// and so loadMarkedAsDroppedTables() will find it and try to add, and UUID will overlap. database_catalog.loadMarkedAsDroppedTables(); + database_catalog.createBackgroundTasks(); /// Then, load remaining databases loadMetadata(global_context, default_database); convertDatabasesEnginesIfNeed(global_context); startupSystemTables(); - database_catalog.loadDatabases(); + database_catalog.startupBackgroundCleanup(); /// After loading validate that default database exists database_catalog.assertDatabaseExists(default_database); /// Load user-defined SQL functions. diff --git a/src/Interpreters/DatabaseCatalog.cpp b/src/Interpreters/DatabaseCatalog.cpp index ad5d9d4d325..2132c6d39f1 100644 --- a/src/Interpreters/DatabaseCatalog.cpp +++ b/src/Interpreters/DatabaseCatalog.cpp @@ -152,20 +152,30 @@ void DatabaseCatalog::initializeAndLoadTemporaryDatabase() attachDatabase(TEMPORARY_DATABASE, db_for_temporary_and_external_tables); } -void DatabaseCatalog::loadDatabases() +void DatabaseCatalog::createBackgroundTasks() { + /// It has to be done before databases are loaded (to avoid a race condition on initialization) if (Context::getGlobalContextInstance()->getApplicationType() == Context::ApplicationType::SERVER && unused_dir_cleanup_period_sec) { auto cleanup_task_holder = getContext()->getSchedulePool().createTask("DatabaseCatalog", [this]() { this->cleanupStoreDirectoryTask(); }); cleanup_task = std::make_unique(std::move(cleanup_task_holder)); + } + + auto task_holder = getContext()->getSchedulePool().createTask("DatabaseCatalog", [this](){ this->dropTableDataTask(); }); + drop_task = std::make_unique(std::move(task_holder)); +} + +void DatabaseCatalog::startupBackgroundCleanup() +{ + /// And it has to be done after all databases are loaded, otherwise cleanup_task may remove something that should not be removed + if (cleanup_task) + { (*cleanup_task)->activate(); /// Do not start task immediately on server startup, it's not urgent. (*cleanup_task)->scheduleAfter(unused_dir_hide_timeout_sec * 1000); } - auto task_holder = getContext()->getSchedulePool().createTask("DatabaseCatalog", [this](){ this->dropTableDataTask(); }); - drop_task = std::make_unique(std::move(task_holder)); (*drop_task)->activate(); std::lock_guard lock{tables_marked_dropped_mutex}; if (!tables_marked_dropped.empty()) diff --git a/src/Interpreters/DatabaseCatalog.h b/src/Interpreters/DatabaseCatalog.h index ba3625626da..11a855ebd79 100644 --- a/src/Interpreters/DatabaseCatalog.h +++ b/src/Interpreters/DatabaseCatalog.h @@ -135,8 +135,9 @@ public: static DatabaseCatalog & instance(); static void shutdown(); + void createBackgroundTasks(); void initializeAndLoadTemporaryDatabase(); - void loadDatabases(); + void startupBackgroundCleanup(); void loadMarkedAsDroppedTables(); /// Get an object that protects the table from concurrently executing multiple DDL operations. diff --git a/tests/integration/test_broken_detached_part_clean_up/test.py b/tests/integration/test_broken_detached_part_clean_up/test.py index d39946102ef..5b18fa34494 100644 --- a/tests/integration/test_broken_detached_part_clean_up/test.py +++ b/tests/integration/test_broken_detached_part_clean_up/test.py @@ -222,9 +222,11 @@ def test_store_cleanup(started_cluster): node1.wait_for_log_line( "Removing access rights for unused directory", timeout=60, - look_behind_lines=1000, + look_behind_lines=1000000, + ) + node1.wait_for_log_line( + "directories from store", timeout=60, look_behind_lines=1000000 ) - node1.wait_for_log_line("directories from store", look_behind_lines=1000) store = node1.exec_in_container(["ls", f"{path_to_data}/store"]) assert "100" in store @@ -277,10 +279,14 @@ def test_store_cleanup(started_cluster): ) node1.wait_for_log_line( - "Removing unused directory", timeout=90, look_behind_lines=1000 + "Removing unused directory", timeout=90, look_behind_lines=1000000 + ) + node1.wait_for_log_line( + "directories from store", timeout=90, look_behind_lines=1000000 + ) + node1.wait_for_log_line( + "Nothing to clean up from store/", timeout=90, look_behind_lines=1000000 ) - node1.wait_for_log_line("directories from store") - node1.wait_for_log_line("Nothing to clean up from store/") store = node1.exec_in_container(["ls", f"{path_to_data}/store"]) assert "100" in store diff --git a/tests/integration/test_dictionaries_all_layouts_separate_sources/test_cassandra.py b/tests/integration/test_dictionaries_all_layouts_separate_sources/test_cassandra.py index 2213623379a..90287e19bd0 100644 --- a/tests/integration/test_dictionaries_all_layouts_separate_sources/test_cassandra.py +++ b/tests/integration/test_dictionaries_all_layouts_separate_sources/test_cassandra.py @@ -2,6 +2,11 @@ import os import math import pytest +# FIXME This test is too flaky +# https://github.com/ClickHouse/ClickHouse/issues/33006 + +pytestmark = pytest.mark.skip + from .common import * from helpers.cluster import ClickHouseCluster