ddl: Fix frequent schema GC under multi tenant (#8248) (#8266)

close #8256

dbms/src/Databases/IDatabase.h

@@ -49,7 +49,7 @@ class IDatabaseIterator
     virtual const String & name() const = 0;
     virtual StoragePtr & table() const = 0;
 
-    virtual ~IDatabaseIterator() {}
+    virtual ~IDatabaseIterator() = default;
 };
 
 using DatabaseIteratorPtr = std::unique_ptr<IDatabaseIterator>;
@@ -154,7 +154,7 @@ class IDatabase : public std::enable_shared_from_this<IDatabase>
     /// Delete metadata, the deletion of which differs from the recursive deletion of the directory, if any.
     virtual void drop(const Context & context) = 0;
 
-    virtual ~IDatabase() {}
+    virtual ~IDatabase() = default;
 };
 
 using DatabasePtr = std::shared_ptr<IDatabase>;

dbms/src/Interpreters/InterpreterCreateQuery.cpp

@@ -576,31 +576,31 @@ BlockIO InterpreterCreateQuery::createTable(ASTCreateQuery & create)
                 // Thus, we choose to do a retry here to wait the table created completed.
                 if (e.code() == ErrorCodes::TABLE_ALREADY_EXISTS || e.code() == ErrorCodes::DDL_GUARD_IS_ACTIVE)
                 {
+                    auto log = Logger::get(fmt::format("InterpreterCreateQuery {} {}", database_name, table_name));
                     LOG_WARNING(
-                        Logger::get("InterpreterCreateQuery"),
+                        log,
                         "createTable failed with error code is {}, error info is {}, stack_info is {}",
                         e.code(),
                         e.displayText(),
                         e.getStackTrace().toString());
-                    for (int i = 0; i < 20; i++) // retry for 400ms
+                    const size_t max_retry = 50;
+                    const int wait_useconds = 20000;
+                    for (size_t i = 0; i < max_retry; i++) // retry
                     {
                         if (context.isTableExist(database_name, table_name))
-                        {
                             return {};
-                        }
-                        else
-                        {
-                            const int wait_useconds = 20000;
-                            LOG_ERROR(
-                                Logger::get("InterpreterCreateQuery"),
-                                "createTable failed but table not exist now, \nWe will sleep for {} ms and try again.",
-                                wait_useconds / 1000);
-                            usleep(wait_useconds); // sleep 20ms
-                        }
+
+                        // sleep a while and retry
+                        LOG_ERROR(
+                            log,
+                            "createTable failed but table not exist now, \nWe will sleep for {} ms and try again.",
+                            wait_useconds / 1000);
+                        usleep(wait_useconds); // sleep 20ms
                     }
                     LOG_ERROR(
-                        Logger::get("InterpreterCreateQuery"),
-                        "still failed to createTable in InterpreterCreateQuery for retry 20 times");
+                        log,
+                        "still failed to createTable in InterpreterCreateQuery for retry {} times",
+                        max_retry);
                     e.rethrow();
                 }
                 else

dbms/src/Storages/KVStore/KVStore.h

@@ -225,10 +225,10 @@ class KVStore final : private boost::noncopyable
 
     /// Create `runner_cnt` threads to run ReadIndexWorker asynchronously and automatically.
     /// If there is other runtime framework, DO NOT invoke it.
-    void asyncRunReadIndexWorkers();
+    void asyncRunReadIndexWorkers() const;
 
     /// Stop workers after there is no more read-index task.
-    void stopReadIndexWorkers();
+    void stopReadIndexWorkers() const;
 
     /// TODO: if supported by runtime framework, run one round for specific runner by `id`.
     void runOneRoundOfReadIndexRunner(size_t runner_id);

dbms/src/Storages/KVStore/Read/ReadIndexWorker.cpp

@@ -755,7 +755,7 @@ ReadIndexWorkerManager::ReadIndexWorkerManager(
     ReadIndexWorkerManager::FnGetTickTime && fn_min_dur_handle_region,
     size_t runner_cnt)
     : proxy_helper(proxy_helper_)
-    , logger(&Poco::Logger::get("ReadIndexWorkers"))
+    , logger(Logger::get("ReadIndexWorkers"))
 {
     for (size_t i = 0; i < runner_cnt; ++i)
         runners.emplace_back(std::make_unique<ReadIndexRunner>(
@@ -931,7 +931,7 @@ void KVStore::initReadIndexWorkers(
     read_index_worker_manager = ptr;
 }
 
-void KVStore::asyncRunReadIndexWorkers()
+void KVStore::asyncRunReadIndexWorkers() const
 {
     if (!read_index_worker_manager)
         return;
@@ -940,13 +940,12 @@ void KVStore::asyncRunReadIndexWorkers()
     read_index_worker_manager->asyncRun();
 }
 
-void KVStore::stopReadIndexWorkers()
+void KVStore::stopReadIndexWorkers() const
 {
     if (!read_index_worker_manager)
         return;
 
     assert(this->proxy_helper);
-
     read_index_worker_manager->stop();
 }
 
@@ -1012,13 +1011,13 @@ ReadIndexWorkerManager::ReadIndexRunner::ReadIndexRunner(
     size_t id_,
     size_t runner_cnt_,
     ReadIndexWorkers & workers_,
-    Poco::Logger * logger_,
+    LoggerPtr logger_,
     FnGetTickTime fn_min_dur_handle_region_,
     AsyncWaker::NotifierPtr global_notifier_)
     : id(id_)
     , runner_cnt(runner_cnt_)
     , workers(workers_)
-    , logger(logger_)
+    , logger(std::move(logger_))
     , fn_min_dur_handle_region(std::move(fn_min_dur_handle_region_))
     , global_notifier(std::move(global_notifier_))
 {}

dbms/src/Storages/KVStore/Read/ReadIndexWorker.h

@@ -136,14 +136,14 @@ class ReadIndexWorkerManager : boost::noncopyable
             size_t id_,
             size_t runner_cnt_,
             ReadIndexWorkers & workers_,
-            Poco::Logger * logger_,
+            LoggerPtr logger_,
             FnGetTickTime fn_min_dur_handle_region_,
             AsyncWaker::NotifierPtr global_notifier_);
 
         const size_t id;
         const size_t runner_cnt;
         ReadIndexWorkers & workers;
-        Poco::Logger * logger;
+        LoggerPtr logger;
         const FnGetTickTime fn_min_dur_handle_region;
         /// The workers belonged to runner share same notifier.
         AsyncWaker::NotifierPtr global_notifier;
@@ -157,7 +157,7 @@ class ReadIndexWorkerManager : boost::noncopyable
     std::vector<std::unique_ptr<ReadIndexRunner>> runners;
     /// Each worker controls read-index process of region(region_id % worker_cnt == worker_id).
     ReadIndexWorkers workers;
-    Poco::Logger * logger;
+    LoggerPtr logger;
 };
 
 struct ReadIndexNotifyCtrl;
@@ -359,4 +359,4 @@ struct MockStressTestCfg
     static bool enable;
 };
 
-} // namespace DB
+} // namespace DB