diff --git a/src/limestone/compaction_options.h b/src/limestone/compaction_options.h index d8901de..0a1468e 100644 --- a/src/limestone/compaction_options.h +++ b/src/limestone/compaction_options.h @@ -26,48 +26,47 @@ namespace limestone::internal { class compaction_options { - public: + public: // Constructor: Pre-compaction phase where to_dir is not provided. // File set is provided and GC is disabled. // In this constructor, "/not_exists_dir" is set for to_dir to indicate an error if used. compaction_options( - const boost::filesystem::path& from, + boost::filesystem::path from, int workers, std::set file_names ) - : from_dir_(from), + : from_dir_(std::move(from)), to_dir_("/not_exists_dir"), num_worker_(workers), - file_names_(file_names), + file_names_(std::move(file_names)), has_file_set_(true), gc_snapshot_(std::nullopt) {} // Constructor: to_dir provided, GC disabled, no file set. compaction_options( - const boost::filesystem::path& from, - const boost::filesystem::path& to, + boost::filesystem::path from, + boost::filesystem::path to, int workers ) - : from_dir_(from), - to_dir_(to), + : from_dir_(std::move(from)), + to_dir_(std::move(to)), num_worker_(workers), - file_names_(), has_file_set_(false), gc_snapshot_(std::nullopt) {} // Constructor: to_dir provided, file set available, GC disabled. compaction_options( - const boost::filesystem::path& from, - const boost::filesystem::path& to, + boost::filesystem::path from, + boost::filesystem::path to, int workers, std::set file_names ) - : from_dir_(from), - to_dir_(to), + : from_dir_(std::move(from)), + to_dir_(std::move(to)), num_worker_(workers), - file_names_(file_names), + file_names_(std::move(file_names)), has_file_set_(true), gc_snapshot_(std::nullopt) {} @@ -75,49 +74,49 @@ // Constructor: to_dir provided, file set available, GC enabled. // Note: The blob_file_gc_snapshot is provided externally by reference. compaction_options( - const boost::filesystem::path& from, - const boost::filesystem::path& to, + boost::filesystem::path from, + boost::filesystem::path to, int workers, std::set file_names, blob_file_gc_snapshot& gc_snapshot ) - : from_dir_(from), - to_dir_(to), + : from_dir_(std::move(from)), + to_dir_(std::move(to)), num_worker_(workers), - file_names_(file_names), + file_names_(std::move(file_names)), has_file_set_(true), gc_snapshot_(std::ref(gc_snapshot)) {} - // Getter for from_dir - const boost::filesystem::path& get_from_dir() const { return from_dir_; } + // Getter for from_dir. + [[nodiscard]] const boost::filesystem::path& get_from_dir() const { return from_dir_; } // Getter for to_dir. - const boost::filesystem::path& get_to_dir() const { return to_dir_; } + [[nodiscard]] const boost::filesystem::path& get_to_dir() const { return to_dir_; } // Getter for num_worker. - int get_num_worker() const { return num_worker_; } + [[nodiscard]] int get_num_worker() const { return num_worker_; } // Getter for file_names. - const std::set& get_file_names() const { return file_names_; } + [[nodiscard]] const std::set& get_file_names() const { return file_names_; } // Returns true if a file set is configured. - bool has_file_set() const { return has_file_set_; } + [[nodiscard]] bool has_file_set() const { return has_file_set_; } // Check if GC is enabled. - bool is_gc_enabled() const { return gc_snapshot_.has_value(); } + [[nodiscard]] bool is_gc_enabled() const { return gc_snapshot_.has_value(); } // Getter for gc_snapshot. // It is caller's responsibility to ensure GC is enabled before calling. - blob_file_gc_snapshot& get_gc_snapshot() const { return gc_snapshot_.value().get(); } + [[nodiscard]] blob_file_gc_snapshot& get_gc_snapshot() const { return gc_snapshot_.value().get(); } - private: + private: // Basic compaction settings. boost::filesystem::path from_dir_; boost::filesystem::path to_dir_; int num_worker_; - // File set for compaction + // File set for compaction. std::set file_names_; bool has_file_set_; diff --git a/src/limestone/datastore_snapshot.cpp b/src/limestone/datastore_snapshot.cpp index 644768e..002e5bd 100644 --- a/src/limestone/datastore_snapshot.cpp +++ b/src/limestone/datastore_snapshot.cpp @@ -109,7 +109,7 @@ static void insert_twisted_entry(sortdb_wrapper* sortdb, const log_entry& e) { sortdb->put(db_key, db_value); } -static std::pair create_sorted_from_wals(compaction_options options) { +static std::pair create_sorted_from_wals(compaction_options &options) { auto from_dir = options.get_from_dir(); auto file_names = options.get_file_names(); auto num_worker = options.get_num_worker(); @@ -300,11 +300,11 @@ static void sortdb_foreach( #endif } -blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options options) { +blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options &options) { auto [max_appeared_epoch, sctx] = create_sorted_from_wals(options); boost::system::error_code error; - auto to_dir = options.get_to_dir(); + const auto &to_dir = options.get_to_dir(); const bool result_check = boost::filesystem::exists(to_dir, error); if (!result_check || error) { const bool result_mkdir = boost::filesystem::create_directory(to_dir, error); diff --git a/src/limestone/dblog_scan.h b/src/limestone/dblog_scan.h index 518df7a..69a52d4 100644 --- a/src/limestone/dblog_scan.h +++ b/src/limestone/dblog_scan.h @@ -82,8 +82,10 @@ class dblog_scan { * * @param logdir The path to the directory containing the files to be processed. */ + explicit dblog_scan(boost::filesystem::path logdir) : dblogdir_(std::move(logdir)) { + rescan_directory_paths(); + } - explicit dblog_scan(const boost::filesystem::path& logdir) : dblogdir_(logdir) { rescan_directory_paths(); } /** * @brief Constructor that initializes the dblog_scan with the specified log directory and file names. @@ -95,8 +97,8 @@ class dblog_scan { * @param logdir The path to the directory containing the files to be processed. * @param file_names The set of file names within `logdir` to be processed. */ - explicit dblog_scan(const boost::filesystem::path& logdir, compaction_options options) : dblogdir_(logdir) { - for (const auto& file_name : options.get_file_names()) { + explicit dblog_scan(boost::filesystem::path logdir, const compaction_options &options) : dblogdir_(std::move(logdir)) { + for (const auto &file_name : options.get_file_names()) { path_list_.emplace_back(dblogdir_ / file_name); } } diff --git a/src/limestone/internal.h b/src/limestone/internal.h index d402283..cee273f 100644 --- a/src/limestone/internal.h +++ b/src/limestone/internal.h @@ -95,7 +95,7 @@ status purge_dir(const boost::filesystem::path& dir); * number of workers, file set, and garbage collection settings. * @return The maximum blob ID found during the compaction process. */ -limestone::api::blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options options); +limestone::api::blob_id_type create_compact_pwal_and_get_max_blob_id(compaction_options &options); std::set filter_epoch_files(const boost::filesystem::path& directory); diff --git a/test/limestone/compaction/compaction_options_test.cpp b/test/limestone/compaction/compaction_options_test.cpp index 5a2873a..c1a1095 100644 --- a/test/limestone/compaction/compaction_options_test.cpp +++ b/test/limestone/compaction/compaction_options_test.cpp @@ -70,7 +70,7 @@ compaction_options options(from_dir_, to_dir_, num_workers_); EXPECT_FALSE(options.is_gc_enabled()); - EXPECT_THROW(options.get_gc_snapshot(), std::bad_optional_access); + EXPECT_THROW((void)options.get_gc_snapshot(), std::bad_optional_access); } // New test for the constructor without to_dir (pre-compaction phase)