unify logger create, get, and drop

kuzudb · Feb 16, 2023 · abe8b1e · abe8b1e
1 parent 7285703
commit abe8b1e
Show file tree

Hide file tree

Showing 28 changed files with 182 additions and 49 deletions.
diff --git a/src/catalog/catalog.cpp b/src/catalog/catalog.cpp
@@ -188,11 +188,11 @@ namespace kuzu {
 namespace catalog {
 
 CatalogContent::CatalogContent() : nextTableID{0} {
-    logger = LoggerUtils::getOrCreateLogger("catalog");
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::CATALOG);
 }
 
 CatalogContent::CatalogContent(const std::string& directory) {
-    logger = LoggerUtils::getOrCreateLogger("catalog");
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::CATALOG);
     logger->info("Initializing catalog.");
     readFromFile(directory, DBFileType::ORIGINAL);
     logger->info("Initializing catalog done.");

diff --git a/src/common/csv_reader/csv_reader.cpp b/src/common/csv_reader/csv_reader.cpp
@@ -36,7 +36,8 @@ CSVReader::CSVReader(const std::string& fname, const CSVReaderConfig& config)
 
 CSVReader::CSVReader(
     char* line, uint64_t lineLen, int64_t linePtrStart, const CSVReaderConfig& config)
-    : fd{nullptr}, config{config}, logger{LoggerUtils::getOrCreateLogger("csv_reader")},
+    : fd{nullptr}, config{config}, logger{LoggerUtils::getLogger(
+                                       LoggerConstants::LoggerEnum::CSV_READER)},
       nextLineIsNotProcessed{false}, isEndOfBlock{false},
       nextTokenIsNotProcessed{false}, line{line}, lineCapacity{1024}, lineLen{lineLen},
       linePtrStart{linePtrStart}, linePtrEnd{linePtrStart}, readingBlockStartOffset{0},

diff --git a/src/common/task_system/task_scheduler.cpp b/src/common/task_system/task_scheduler.cpp
@@ -9,7 +9,8 @@ namespace kuzu {
 namespace common {
 
 TaskScheduler::TaskScheduler(uint64_t numThreads)
-    : logger{LoggerUtils::getOrCreateLogger("processor")}, nextScheduledTaskID{0} {
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::PROCESSOR)}, nextScheduledTaskID{
+                                                                                  0} {
     for (auto n = 0u; n < numThreads; ++n) {
         threads.emplace_back([&] { runWorkerThread(); });
     }

diff --git a/src/common/utils.cpp b/src/common/utils.cpp
@@ -6,14 +6,60 @@
 namespace kuzu {
 namespace common {
 
-std::shared_ptr<spdlog::logger> LoggerUtils::getOrCreateLogger(const std::string& loggerName) {
-    std::shared_ptr<spdlog::logger> logger = spdlog::get(loggerName);
-    if (!logger) {
-        logger = spdlog::stdout_logger_mt(loggerName);
+void LoggerUtils::createLogger(LoggerConstants::LoggerEnum loggerEnum) {
+    auto loggerName = getLoggerName(loggerEnum);
+    if (!spdlog::get(loggerName)) {
+        spdlog::stdout_logger_mt(loggerName);
     }
+}
+
+std::shared_ptr<spdlog::logger> LoggerUtils::getLogger(LoggerConstants::LoggerEnum loggerEnum) {
+    auto loggerName = getLoggerName(loggerEnum);
+    std::shared_ptr<spdlog::logger> logger = spdlog::get(loggerName);
+    assert(logger);
     return logger;
 }
 
+void LoggerUtils::dropLogger(LoggerConstants::LoggerEnum loggerEnum) {
+    auto loggerName = getLoggerName(loggerEnum);
+    spdlog::drop(loggerName);
+}
+
+std::string LoggerUtils::getLoggerName(LoggerConstants::LoggerEnum loggerEnum) {
+    switch (loggerEnum) {
+    case LoggerConstants::LoggerEnum::DATABASE: {
+        return "database";
+    } break;
+    case LoggerConstants::LoggerEnum::CSV_READER: {
+        return "csv_reader";
+    } break;
+    case LoggerConstants::LoggerEnum::LOADER: {
+        return "loader";
+    } break;
+    case LoggerConstants::LoggerEnum::PROCESSOR: {
+        return "processor";
+    } break;
+    case LoggerConstants::LoggerEnum::BUFFER_MANAGER: {
+        return "buffer_manager";
+    } break;
+    case LoggerConstants::LoggerEnum::CATALOG: {
+        return "catalog";
+    } break;
+    case LoggerConstants::LoggerEnum::STORAGE: {
+        return "storage";
+    } break;
+    case LoggerConstants::LoggerEnum::TRANSACTION_MANAGER: {
+        return "transaction_manager";
+    } break;
+    case LoggerConstants::LoggerEnum::WAL: {
+        return "wal";
+    } break;
+    default: {
+        assert(false);
+    }
+    }
+}
+
 std::vector<std::string> StringUtils::split(
     const std::string& input, const std::string& delimiter) {
     auto result = std::vector<std::string>();

diff --git a/src/include/common/constants.h b/src/include/common/constants.h
@@ -106,6 +106,20 @@ struct CopyConstants {
     static constexpr bool DEFAULT_CSV_HAS_HEADER = false;
 };
 
+struct LoggerConstants {
+    enum class LoggerEnum : uint8_t {
+        DATABASE = 0,
+        CSV_READER = 1,
+        LOADER = 2,
+        PROCESSOR = 3,
+        BUFFER_MANAGER = 4,
+        CATALOG = 5,
+        STORAGE = 6,
+        TRANSACTION_MANAGER = 7,
+        WAL = 8,
+    };
+};
+
 struct EnumeratorKnobs {
     static constexpr double PREDICATE_SELECTIVITY = 0.1;
     static constexpr double FLAT_PROBE_PENALTY = 10;

diff --git a/src/include/common/utils.h b/src/include/common/utils.h
@@ -9,6 +9,7 @@
 #include <thread>
 #include <vector>
 
+#include "common/constants.h"
 #include "exception.h"
 
 namespace spdlog {
@@ -21,7 +22,12 @@ namespace common {
 class LoggerUtils {
 public:
     // Note: create logger is not thread safe.
-    static std::shared_ptr<spdlog::logger> getOrCreateLogger(const std::string& loggerName);
+    static void createLogger(LoggerConstants::LoggerEnum loggerEnum);
+    static std::shared_ptr<spdlog::logger> getLogger(LoggerConstants::LoggerEnum loggerEnum);
+    static void dropLogger(LoggerConstants::LoggerEnum loggerEnum);
+
+private:
+    static std::string getLoggerName(LoggerConstants::LoggerEnum loggerEnum);
 };
 
 class StringUtils {

diff --git a/src/include/main/database.h b/src/include/main/database.h
@@ -63,7 +63,7 @@ class Database {
      * @param loggingLevel New logging level. (Supported logging levels are: "info", "debug",
      * "err").
      */
-    void setLoggingLevel(std::string loggingLevel);
+    static void setLoggingLevel(std::string loggingLevel);
 
     /**
      * @brief Resizes the buffer pool size of the database instance.
@@ -82,7 +82,8 @@ class Database {
         bool skipCheckpointForTestingRecovery = false);
 
     void initDBDirAndCoreFilesIfNecessary() const;
-    void initLoggers();
+    static void initLoggers();
+    static void dropLoggers();
 
     void checkpointAndClearWAL();
     void rollbackAndClearWAL();

diff --git a/src/include/storage/storage_structure/lists/lists_metadata.h b/src/include/storage/storage_structure/lists/lists_metadata.h
@@ -17,7 +17,9 @@ class BaseListsMetadata {
     static constexpr uint64_t LARGE_LIST_IDX_TO_PAGE_LIST_HEAD_IDX_MAP_HEADER_PAGE_IDX = 1;
     static constexpr uint64_t CHUNK_PAGE_LIST_HEADER_PAGE_IDX = 2;
 
-    explicit BaseListsMetadata() { logger = common::LoggerUtils::getOrCreateLogger("storage"); }
+    explicit BaseListsMetadata() {
+        logger = common::LoggerUtils::getLogger(common::LoggerConstants::LoggerEnum::STORAGE);
+    }
 
     inline static std::function<uint32_t(uint32_t)> getIdxInPageListToListPageIdxMapper(
         BaseInMemDiskArray<common::page_idx_t>* pageLists, uint32_t pageListsHead) {

diff --git a/src/include/storage/storage_structure/storage_structure.h b/src/include/storage/storage_structure/storage_structure.h
@@ -24,7 +24,7 @@ class StorageStructure {
 public:
     StorageStructure(const StorageStructureIDAndFName& storageStructureIDAndFName,
         BufferManager& bufferManager, WAL* wal)
-        : logger{common::LoggerUtils::getOrCreateLogger("storage")},
+        : logger{common::LoggerUtils::getLogger(common::LoggerConstants::LoggerEnum::STORAGE)},
           fileHandle{storageStructureIDAndFName, FileHandle::O_PERSISTENT_FILE_NO_CREATE},
           bufferManager{bufferManager}, wal{wal} {}
 

diff --git a/src/include/transaction/transaction_manager.h b/src/include/transaction/transaction_manager.h
@@ -17,8 +17,9 @@ class TransactionManager {
 
 public:
     explicit TransactionManager(storage::WAL& wal)
-        : logger{common::LoggerUtils::getOrCreateLogger("transaction_manager")}, wal{wal},
-          activeWriteTransactionID{INT64_MAX}, lastTransactionID{0}, lastCommitID{0} {};
+        : logger{common::LoggerUtils::getLogger(
+              common::LoggerConstants::LoggerEnum::TRANSACTION_MANAGER)},
+          wal{wal}, activeWriteTransactionID{INT64_MAX}, lastTransactionID{0}, lastCommitID{0} {};
     std::unique_ptr<Transaction> beginWriteTransaction();
     std::unique_ptr<Transaction> beginReadOnlyTransaction();
     void commit(Transaction* transaction);

diff --git a/src/main/database.cpp b/src/main/database.cpp
@@ -36,10 +36,10 @@ SystemConfig::SystemConfig(uint64_t bufferPoolSize) {
 Database::Database(std::string databasePath) : Database{std::move(databasePath), SystemConfig()} {}
 
 Database::Database(std::string databasePath, SystemConfig systemConfig)
-    : databasePath{std::move(databasePath)},
-      systemConfig{systemConfig}, logger{LoggerUtils::getOrCreateLogger("database")} {
+    : databasePath{std::move(databasePath)}, systemConfig{systemConfig} {
     initLoggers();
     initDBDirAndCoreFilesIfNecessary();
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::DATABASE);
     bufferManager = std::make_unique<BufferManager>(
         this->systemConfig.defaultPageBufferPoolSize, this->systemConfig.largePageBufferPoolSize);
     memoryManager = std::make_unique<MemoryManager>(bufferManager.get());
@@ -52,7 +52,9 @@ Database::Database(std::string databasePath, SystemConfig systemConfig)
     transactionManager = std::make_unique<transaction::TransactionManager>(*wal);
 }
 
-Database::~Database() = default;
+Database::~Database() {
+    dropLoggers();
+}
 
 void Database::initDBDirAndCoreFilesIfNecessary() const {
     if (!FileUtils::fileOrPathExists(databasePath)) {
@@ -75,17 +77,30 @@ void Database::initDBDirAndCoreFilesIfNecessary() const {
 void Database::initLoggers() {
     // To avoid multi-threading issue in creating logger, we create all loggers together with
     // database instance. All system components should get logger instead of creating.
-    LoggerUtils::getOrCreateLogger("csv_reader");
-    LoggerUtils::getOrCreateLogger("loader");
-    LoggerUtils::getOrCreateLogger("processor");
-    LoggerUtils::getOrCreateLogger("buffer_manager");
-    LoggerUtils::getOrCreateLogger("catalog");
-    LoggerUtils::getOrCreateLogger("storage");
-    LoggerUtils::getOrCreateLogger("transaction_manager");
-    LoggerUtils::getOrCreateLogger("wal");
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::DATABASE);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::CSV_READER);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::LOADER);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::PROCESSOR);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::BUFFER_MANAGER);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::CATALOG);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::STORAGE);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::TRANSACTION_MANAGER);
+    LoggerUtils::createLogger(LoggerConstants::LoggerEnum::WAL);
     spdlog::set_level(spdlog::level::err);
 }
 
+void Database::dropLoggers() {
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::DATABASE);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::CSV_READER);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::LOADER);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::PROCESSOR);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::BUFFER_MANAGER);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::CATALOG);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::STORAGE);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::TRANSACTION_MANAGER);
+    LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::WAL);
+}
+
 void Database::setLoggingLevel(std::string loggingLevel) {
     spdlog::set_level(LoggingLevelUtils::convertStrToLevelEnum(std::move(loggingLevel)));
 }

diff --git a/src/storage/buffer_manager/buffer_manager.cpp b/src/storage/buffer_manager/buffer_manager.cpp
@@ -11,17 +11,15 @@ namespace kuzu {
 namespace storage {
 
 BufferManager::BufferManager(uint64_t maxSizeForDefaultPagePool, uint64_t maxSizeForLargePagePool)
-    : logger{LoggerUtils::getOrCreateLogger("buffer_manager")},
+    : logger{LoggerUtils::getLogger(common::LoggerConstants::LoggerEnum::BUFFER_MANAGER)},
       bufferPoolDefaultPages(std::make_unique<BufferPool>(
           BufferPoolConstants::DEFAULT_PAGE_SIZE, maxSizeForDefaultPagePool)),
       bufferPoolLargePages(std::make_unique<BufferPool>(
           BufferPoolConstants::LARGE_PAGE_SIZE, maxSizeForLargePagePool)) {
     logger->info("Done Initializing Buffer Manager.");
 }
 
-BufferManager::~BufferManager() {
-    spdlog::drop("buffer_manager");
-}
+BufferManager::~BufferManager() = default;
 
 void BufferManager::resize(uint64_t newSizeForDefaultPagePool, uint64_t newSizeForLargePagePool) {
     bufferPoolDefaultPages->resize(newSizeForDefaultPagePool);

diff --git a/src/storage/buffer_manager/buffer_pool.cpp b/src/storage/buffer_manager/buffer_pool.cpp
@@ -53,7 +53,8 @@ void Frame::releaseBuffer() {
 }
 
 BufferPool::BufferPool(uint64_t pageSize, uint64_t maxSize)
-    : logger{LoggerUtils::getOrCreateLogger("buffer_manager")}, pageSize{pageSize}, clockHand{0},
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::BUFFER_MANAGER)},
+      pageSize{pageSize}, clockHand{0},
       numFrames((page_idx_t)(ceil((double)maxSize / (double)pageSize))) {
     assert(pageSize == BufferPoolConstants::DEFAULT_PAGE_SIZE ||
            pageSize == BufferPoolConstants::LARGE_PAGE_SIZE);

diff --git a/src/storage/buffer_manager/file_handle.cpp b/src/storage/buffer_manager/file_handle.cpp
@@ -10,7 +10,7 @@ namespace kuzu {
 namespace storage {
 
 FileHandle::FileHandle(const std::string& path, uint8_t flags)
-    : logger{LoggerUtils::getOrCreateLogger("storage")}, flags(flags) {
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::STORAGE)}, flags(flags) {
     logger->trace("FileHandle: Path {}", path);
     if (!isNewTmpFile()) {
         constructExistingFileHandle(path);

diff --git a/src/storage/copy_arrow/copy_structures_arrow.cpp b/src/storage/copy_arrow/copy_structures_arrow.cpp
@@ -11,8 +11,8 @@ namespace storage {
 
 CopyStructuresArrow::CopyStructuresArrow(CopyDescription& copyDescription,
     std::string outputDirectory, TaskScheduler& taskScheduler, Catalog& catalog)
-    : logger{LoggerUtils::getOrCreateLogger("loader")}, copyDescription{copyDescription},
-      outputDirectory{std::move(outputDirectory)}, numBlocks{0},
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::LOADER)},
+      copyDescription{copyDescription}, outputDirectory{std::move(outputDirectory)}, numBlocks{0},
       taskScheduler{taskScheduler}, catalog{catalog}, numRows{0} {}
 
 // Lists headers are created for only AdjLists, which store data in the page without NULL bits.

diff --git a/src/storage/storage_manager.cpp b/src/storage/storage_manager.cpp
@@ -13,7 +13,8 @@ namespace storage {
 
 StorageManager::StorageManager(
     catalog::Catalog& catalog, BufferManager& bufferManager, MemoryManager& memoryManager, WAL* wal)
-    : logger{LoggerUtils::getOrCreateLogger("storage")}, catalog{catalog}, wal{wal} {
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::STORAGE)}, catalog{catalog},
+      wal{wal} {
     logger->info("Initializing StorageManager from directory: " + wal->getDirectory());
     nodesStore = std::make_unique<NodesStore>(catalog, bufferManager, wal);
     relsStore = std::make_unique<RelsStore>(catalog, bufferManager, memoryManager, wal);

diff --git a/src/storage/storage_structure/lists/list_headers.cpp b/src/storage/storage_structure/lists/list_headers.cpp
@@ -9,7 +9,7 @@ namespace kuzu {
 namespace storage {
 
 BaseListHeaders::BaseListHeaders() {
-    logger = LoggerUtils::getOrCreateLogger("storage");
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::STORAGE);
 }
 
 ListHeadersBuilder::ListHeadersBuilder(const std::string& baseListFName, uint64_t numElements)

diff --git a/src/storage/store/table_statistics.cpp b/src/storage/store/table_statistics.cpp
@@ -8,7 +8,7 @@ namespace kuzu {
 namespace storage {
 
 TablesStatistics::TablesStatistics() {
-    logger = LoggerUtils::getOrCreateLogger("storage");
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::STORAGE);
     tablesStatisticsContentForReadOnlyTrx = std::make_unique<TablesStatisticsContent>();
 }
 

diff --git a/src/storage/wal/wal.cpp b/src/storage/wal/wal.cpp
@@ -10,7 +10,7 @@ namespace kuzu {
 namespace storage {
 
 WAL::WAL(const std::string& directory, BufferManager& bufferManager)
-    : logger{LoggerUtils::getOrCreateLogger("wal")}, directory{directory},
+    : logger{LoggerUtils::getLogger(LoggerConstants::LoggerEnum::WAL)}, directory{directory},
       bufferManager{bufferManager}, isLastLoggedRecordCommit_{false} {
     fileHandle = WAL::createWALFileHandle(directory);
     initCurrentPage();

diff --git a/src/storage/wal_replayer.cpp b/src/storage/wal_replayer.cpp
@@ -64,7 +64,7 @@ void WALReplayer::replay() {
 }
 
 void WALReplayer::init() {
-    logger = LoggerUtils::getOrCreateLogger("storage");
+    logger = LoggerUtils::getLogger(LoggerConstants::LoggerEnum::STORAGE);
     walFileHandle = WAL::createWALFileHandle(wal->getDirectory());
     pageBuffer = std::make_unique<uint8_t[]>(BufferPoolConstants::DEFAULT_PAGE_SIZE);
 }

diff --git a/test/catalog/CMakeLists.txt b/test/catalog/CMakeLists.txt
@@ -1,5 +1 @@
-add_executable(catalog_test catalog_test.cpp)
-
-target_link_libraries(catalog_test PUBLIC graph_test)
-
-add_test(kuzu_catalog_test catalog_test)
+add_kuzu_test(catalog_test catalog_test.cpp)
diff --git a/test/catalog/catalog_test.cpp b/test/catalog/catalog_test.cpp
@@ -11,11 +11,15 @@ class CatalogTest : public testing::Test {
 
     void SetUp() override {
         FileUtils::createDir(CATALOG_TEMP_DIRECTORY);
+        LoggerUtils::createLogger(LoggerConstants::LoggerEnum::CATALOG);
         catalog = std::make_unique<Catalog>();
         setupCatalog();
     }
 
-    void TearDown() override { FileUtils::removeDir(CATALOG_TEMP_DIRECTORY); }
+    void TearDown() override {
+        FileUtils::removeDir(CATALOG_TEMP_DIRECTORY);
+        LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::CATALOG);
+    }
 
     void setupCatalog() {
         std::vector<PropertyNameDataType> personProperties;

diff --git a/test/processor/order_by/key_block_merger_test.cpp b/test/processor/order_by/key_block_merger_test.cpp
@@ -18,6 +18,8 @@ class KeyBlockMergerTest : public Test {
 
 public:
     void SetUp() override {
+        LoggerUtils::createLogger(LoggerConstants::LoggerEnum::BUFFER_MANAGER);
+        LoggerUtils::createLogger(LoggerConstants::LoggerEnum::STORAGE);
         bufferManager =
             std::make_unique<BufferManager>(StorageConstants::DEFAULT_BUFFER_POOL_SIZE_FOR_TESTING *
                                                 StorageConstants::DEFAULT_PAGES_BUFFER_RATIO,
@@ -26,6 +28,11 @@ class KeyBlockMergerTest : public Test {
         memoryManager = std::make_unique<MemoryManager>(bufferManager.get());
     }
 
+    void TearDown() override {
+        LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::BUFFER_MANAGER);
+        LoggerUtils::dropLogger(LoggerConstants::LoggerEnum::STORAGE);
+    }
+
 public:
     std::unique_ptr<BufferManager> bufferManager;
     std::unique_ptr<MemoryManager> memoryManager;