From dec751cfbe5723a00896f9b8320111507b26abff Mon Sep 17 00:00:00 2001 From: Benny Halevy Date: Thu, 16 Jul 2020 13:29:17 +0300 Subject: [PATCH] compaction: report_(start|finish): just return description Rather than logging the message in the virtual callee method just return a string description and make the logger call in the common caller. 1. There is no need to do the logger call in the callee, it is simpler to format the log message in the the caller and just retrieve the per-compaction-type description. 2. Prepare to centrally print the compaction uuid. Signed-off-by: Benny Halevy --- sstables/compaction.cc | 59 +++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 30 deletions(-) diff --git a/sstables/compaction.cc b/sstables/compaction.cc index fa75f0e6a2..c7bb96092b 100644 --- a/sstables/compaction.cc +++ b/sstables/compaction.cc @@ -515,7 +515,7 @@ private: requires CompactedFragmentsConsumer future<> setup(GCConsumer gc_consumer) { auto ssts = make_lw_shared(_cf.get_compaction_strategy().make_sstable_set(_schema)); - sstring formatted_msg = "["; + sstring formatted_msg = "{} ["; auto fully_expired = get_fully_expired_sstables(_cf, _sstables, gc_clock::now() - _schema->gc_grace_seconds()); min_max_tracker timestamp_tracker; @@ -555,7 +555,7 @@ private: _info->sstables = _sstables.size(); _info->ks_name = _schema->ks_name(); _info->cf_name = _schema->cf_name(); - report_start(formatted_msg); + clogger.info(formatted_msg.c_str(), report_start_desc()); _compacting = std::move(ssts); @@ -603,12 +603,11 @@ private: // - add support to merge summary (message: Partition merge counts were {%s}.). // - there is no easy way, currently, to know the exact number of total partitions. // By the time being, using estimated key count. - sstring formatted_msg = fmt::format("{} sstables to [{}]. {} to {} (~{}% of original) in {}ms = {}. " \ - "~{} total partitions merged to {}.", - _info->sstables, new_sstables_msg, pretty_printed_data_size(_info->start_size), pretty_printed_data_size(_info->end_size), int(ratio * 100), - std::chrono::duration_cast(duration).count(), pretty_printed_throughput(_info->end_size, duration), - _info->total_partitions, _info->total_keys_written); - report_finish(formatted_msg, ended_at); + clogger.info("{} {} sstables to [{}]. {} to {} (~{}% of original) in {}ms = {}. ~{} total partitions merged to {}.", + report_finish_desc(), + _info->sstables, new_sstables_msg, pretty_printed_data_size(_info->start_size), pretty_printed_data_size(_info->end_size), int(ratio * 100), + std::chrono::duration_cast(duration).count(), pretty_printed_throughput(_info->end_size, duration), + _info->total_partitions, _info->total_keys_written); backlog_tracker_adjust_charges(); @@ -617,8 +616,8 @@ private: return std::move(*info); } - virtual void report_start(const sstring& formatted_msg) const = 0; - virtual void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const = 0; + virtual std::string_view report_start_desc() const = 0; + virtual std::string_view report_finish_desc() const = 0; virtual void backlog_tracker_adjust_charges() { }; std::function max_purgeable_func() { @@ -760,12 +759,12 @@ public: default_read_monitor_generator()); } - void report_start(const sstring& formatted_msg) const override { - clogger.info("Reshaping {}", formatted_msg); + std::string_view report_start_desc() const override { + return "Reshaping"; } - void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const override { - clogger.info("Reshaped {}", formatted_msg); + std::string_view report_finish_desc() const override { + return "Reshaped"; } virtual compaction_writer create_compaction_writer(const dht::decorated_key& dk) override { @@ -810,12 +809,12 @@ public: _monitor_generator); } - void report_start(const sstring& formatted_msg) const override { - clogger.info("Compacting {}", formatted_msg); + std::string_view report_start_desc() const override { + return "Compacting"; } - void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const override { - clogger.info("Compacted {}", formatted_msg); + std::string_view report_finish_desc() const override { + return "Compacted"; } void backlog_tracker_adjust_charges() override { @@ -993,12 +992,12 @@ public: { } - void report_start(const sstring& formatted_msg) const override { - clogger.info("Cleaning {}", formatted_msg); + std::string_view report_start_desc() const override { + return "Cleaning"; } - void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const override { - clogger.info("Cleaned {}", formatted_msg); + std::string_view report_finish_desc() const override { + return "Cleaned"; } flat_mutation_reader::filter make_partition_filter() const override { @@ -1196,12 +1195,12 @@ public: , _options(options) { } - void report_start(const sstring& formatted_msg) const override { - clogger.info("Scrubbing {}", formatted_msg); + std::string_view report_start_desc() const override { + return "Scrubbing"; } - void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const override { - clogger.info("Finished scrubbing {}", formatted_msg); + std::string_view report_finish_desc() const override { + return "Finished scrubbing"; } flat_mutation_reader make_sstable_reader() const override { @@ -1289,12 +1288,12 @@ public: return true; } - void report_start(const sstring& formatted_msg) const override { - clogger.info("Resharding {}", formatted_msg); + std::string_view report_start_desc() const override { + return "Resharding"; } - void report_finish(const sstring& formatted_msg, std::chrono::time_point ended_at) const override { - clogger.info("Resharded {}", formatted_msg); + std::string_view report_finish_desc() const override { + return "Resharded"; } void backlog_tracker_adjust_charges() override { }