From d35ad4174aed620c0a1c7ad9abb365b574438f05 Mon Sep 17 00:00:00 2001 From: t-horikawa Date: Tue, 21 May 2024 15:38:40 +0900 Subject: [PATCH] comply with the original design --- docs/internal/dbstats-design_ja.md | 4 +- .../tateyama/metrics/metrics_aggregation.h | 33 +++------- include/tateyama/metrics/metrics_store.h | 2 +- .../endpoint/ipc/metrics/ipc_metrics.h | 65 +++++++------------ src/tateyama/metrics/metrics_aggregation.cpp | 41 ++++++++++++ src/tateyama/metrics/metrics_store.cpp | 4 +- .../metrics/resource/metrics_store_impl.cpp | 14 ++-- .../metrics/resource/metrics_store_impl.h | 4 +- .../metrics/metrics_aggregation_test.cpp | 16 +---- .../metrics/resource/metrics_bridge_test.cpp | 21 +----- 10 files changed, 90 insertions(+), 114 deletions(-) create mode 100644 src/tateyama/metrics/metrics_aggregation.cpp diff --git a/docs/internal/dbstats-design_ja.md b/docs/internal/dbstats-design_ja.md index 110d2bc6..f78a17b0 100644 --- a/docs/internal/dbstats-design_ja.md +++ b/docs/internal/dbstats-design_ja.md @@ -175,7 +175,7 @@ public: * @brief returns a new aggregator of this aggregation. * @returns the aggregation operation type */ - virtual std::unique_ptr create_aggregator() const noexcept = 0; + std::unique_ptr create_aggregator() const noexcept; }; ``` @@ -258,7 +258,7 @@ public: * @param aggregation aggregation specification to register * @throws std::runtime_error if another aggregation with the same key is already in this store */ - void register_aggregation(std::unique_ptr aggregation); + void register_aggregation(const metrics_aggregation& aggregation); /** * @brief removes the previously registered metrics item or aggregation. diff --git a/include/tateyama/metrics/metrics_aggregation.h b/include/tateyama/metrics/metrics_aggregation.h index 838697c7..62361eff 100644 --- a/include/tateyama/metrics/metrics_aggregation.h +++ b/include/tateyama/metrics/metrics_aggregation.h @@ -17,6 +17,7 @@ #include #include +#include #include @@ -32,49 +33,35 @@ class metrics_aggregation { * @brief returns the metrics aggregation group key. * @returns the metrics aggregation group key string */ - [[nodiscard]] std::string_view group_key() const noexcept { - return group_key_; - } + [[nodiscard]] std::string_view group_key() const noexcept; /** * @brief returns the metrics item description. * @returns the metrics item description */ - [[nodiscard]] std::string_view description() const noexcept { - return description_; - } + [[nodiscard]] std::string_view description() const noexcept; /** * @brief returns a new aggregator of this aggregation. * @returns the aggregation operation type */ - [[nodiscard]] virtual std::unique_ptr create_aggregator() const noexcept = 0; + [[nodiscard]] std::unique_ptr create_aggregator() const; /** * @brief create a new object * @param group_key the metrics aggregation group key string * @param description the metrics item description + * @param factory a function to create the metrics_aggregator object */ metrics_aggregation( std::string_view group_key, - std::string_view description) noexcept : - group_key_(group_key), - description_(description) { - } - - /** - * @brief disposes this instance. - */ - virtual ~metrics_aggregation() = default; - - metrics_aggregation(metrics_aggregation const&) = delete; - metrics_aggregation(metrics_aggregation&&) = delete; - metrics_aggregation& operator = (metrics_aggregation const&) = delete; - metrics_aggregation& operator = (metrics_aggregation&&) = delete; + std::string_view description, + std::function()>&& factory) noexcept; private: - std::string group_key_{}; - std::string description_{}; + const std::string group_key_{}; + const std::string description_{}; + const std::function()> factory_; }; } diff --git a/include/tateyama/metrics/metrics_store.h b/include/tateyama/metrics/metrics_store.h index 961f8bd8..3dbb81f1 100644 --- a/include/tateyama/metrics/metrics_store.h +++ b/include/tateyama/metrics/metrics_store.h @@ -48,7 +48,7 @@ class metrics_store { * @param aggregation aggregation specification to register * @throws std::runtime_error if another aggregation with the same key is already in this store */ - void register_aggregation(std::unique_ptr aggregation); + void register_aggregation(const metrics_aggregation& aggregation); /** * @brief removes the previously registered metrics item or aggregation. diff --git a/src/tateyama/endpoint/ipc/metrics/ipc_metrics.h b/src/tateyama/endpoint/ipc/metrics/ipc_metrics.h index ee7aeb4d..0a7ef3e9 100644 --- a/src/tateyama/endpoint/ipc/metrics/ipc_metrics.h +++ b/src/tateyama/endpoint/ipc/metrics/ipc_metrics.h @@ -47,60 +47,35 @@ class ipc_metrics { private: double value_{}; }; - class session_count_aggregation : public tateyama::metrics::metrics_aggregation { - public: - session_count_aggregation( - const std::string& group_key, - const std::string& description) : metrics_aggregation(group_key, description) { - } - [[nodiscard]] std::unique_ptr create_aggregator() const noexcept override { - return std::make_unique(); - } - }; // for ipc_memory class ipc_memory_aggregator : public tateyama::metrics::metrics_aggregator { public: - ipc_memory_aggregator() = delete; - explicit ipc_memory_aggregator(std::size_t bytes) : memory_per_session_(bytes) { - } - void add(tateyama::metrics::metrics_metadata const&, double value) override { - value_ += value; + void add(tateyama::metrics::metrics_metadata const& metadata, double value) override { + if (metadata.key() == "ipc_session_count") { + session_count_ = value; + } else if (metadata.key() == "memory_usage_per_session") { + memory_per_session_ = value; + } } result_type aggregate() override { if(tateyama::metrics::service::ipc_correction) { - return (value_ - 1.0) * static_cast(memory_per_session_); + return (session_count_ - 1.0) * memory_per_session_; } - return value_ * static_cast(memory_per_session_); + return session_count_ * memory_per_session_; } private: - std::size_t memory_per_session_{}; - double value_{}; - }; - class ipc_memory_aggregation : public tateyama::metrics::metrics_aggregation { - public: - ipc_memory_aggregation( - const std::string& group_key, - const std::string& description) : metrics_aggregation(group_key, description) { - } - void set_memory_per_session_(std::size_t bytes) noexcept { - memory_per_session_ = bytes; - } - [[nodiscard]] std::unique_ptr create_aggregator() const noexcept override { - return std::make_unique(memory_per_session_); - } - private: - std::size_t memory_per_session_{}; + double memory_per_session_{}; + double session_count_{}; }; public: explicit ipc_metrics(tateyama::framework::environment& env) : metrics_store_(env.resource_repository().find<::tateyama::metrics::resource::bridge>()->metrics_store()), - session_count_(metrics_store_.register_item(session_count_metadata_)) + session_count_(metrics_store_.register_item(session_count_metadata_)), + ipc_memory_usage_(metrics_store_.register_item(ipc_memory_usage_metada_)) { - metrics_store_.register_aggregation(std::make_unique("session_count", "number of active sessions")); - auto memory_aggregation = std::make_unique("ipc_buffer_size", "allocated buffer size for all IPC sessions"); - ipc_memory_aggregation_ = memory_aggregation.get(); - metrics_store_.register_aggregation(std::move(memory_aggregation)); + metrics_store_.register_aggregation(tateyama::metrics::metrics_aggregation{"session_count", "number of active sessions", [](){return std::make_unique();}}); + metrics_store_.register_aggregation(tateyama::metrics::metrics_aggregation{"ipc_buffer_size", "allocated buffer size for all IPC sessions", [](){return std::make_unique();}}); } private: @@ -112,17 +87,21 @@ class ipc_metrics { std::vector {"session_count"s, "ipc_buffer_size"s}, false }; + tateyama::metrics::metrics_metadata ipc_memory_usage_metada_ { + "memory_usage_per_session"s, "memory usage per session"s, + std::vector> {}, + std::vector {"ipc_buffer_size"s}, + false + }; // have to be placed after corresponding metrics_metadata definition tateyama::metrics::metrics_item_slot& session_count_; - ipc_memory_aggregation *ipc_memory_aggregation_{}; + tateyama::metrics::metrics_item_slot& ipc_memory_usage_; std::atomic_long count_{}; void memory_usage(std::size_t bytes) noexcept { - if (ipc_memory_aggregation_) { - ipc_memory_aggregation_->set_memory_per_session_(bytes); - } + ipc_memory_usage_ = static_cast(bytes); } void increase() noexcept { count_++; diff --git a/src/tateyama/metrics/metrics_aggregation.cpp b/src/tateyama/metrics/metrics_aggregation.cpp new file mode 100644 index 00000000..27d2e128 --- /dev/null +++ b/src/tateyama/metrics/metrics_aggregation.cpp @@ -0,0 +1,41 @@ +/* + * Copyright 2018-2024 Project Tsurugi. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include + +namespace tateyama::metrics { + +metrics_aggregation::metrics_aggregation(std::string_view group_key, + std::string_view description, + std::function()>&& factory) noexcept : + group_key_(group_key), + description_(description), + factory_(factory) { +} + +std::string_view metrics_aggregation::group_key() const noexcept { + return group_key_; +} + +std::string_view metrics_aggregation::description() const noexcept { + return description_; +} + +std::unique_ptr metrics_aggregation::create_aggregator() const { + return factory_(); +} + +} diff --git a/src/tateyama/metrics/metrics_store.cpp b/src/tateyama/metrics/metrics_store.cpp index c098b2a6..52fa5280 100644 --- a/src/tateyama/metrics/metrics_store.cpp +++ b/src/tateyama/metrics/metrics_store.cpp @@ -25,8 +25,8 @@ metrics_item_slot& metrics_store::register_item(const metrics_metadata& metadata return body_->register_item(metadata); } -void metrics_store::register_aggregation(std::unique_ptr aggregation) { - return body_->register_aggregation(std::move(aggregation)); +void metrics_store::register_aggregation(const metrics_aggregation& aggregation) { + return body_->register_aggregation(aggregation); } bool metrics_store::unregister_element(std::string_view key) { diff --git a/src/tateyama/metrics/resource/metrics_store_impl.cpp b/src/tateyama/metrics/resource/metrics_store_impl.cpp index b5ce5ddb..a3a77c97 100644 --- a/src/tateyama/metrics/resource/metrics_store_impl.cpp +++ b/src/tateyama/metrics/resource/metrics_store_impl.cpp @@ -46,12 +46,12 @@ metrics_item_slot& metrics_store_impl::register_item(const metrics_metadata& met return (invisible_metrics_.find(key)->second->find(metadata)->second).first; } -void metrics_store_impl::register_aggregation(std::unique_ptr aggregation) { - const std::string key{aggregation->group_key()}; +void metrics_store_impl::register_aggregation(const metrics_aggregation& aggregation) { + const std::string key{aggregation.group_key()}; if (aggregations_.find(key) != aggregations_.end()) { throw std::runtime_error("aggregation is already registered"); } - aggregations_.emplace(key, std::move(aggregation)); + aggregations_.emplace(key, aggregation); } bool metrics_store_impl::unregister_element(std::string_view key) { @@ -90,7 +90,7 @@ void metrics_store_impl::enumerate_items(std::function const& acceptor) const { for (auto&& e: aggregations_) { - acceptor(*(e.second)); + acceptor(e.second); } } @@ -107,7 +107,7 @@ void metrics_store_impl::set_item_description(::tateyama::proto::metrics::respon for (auto&& a: aggregations_) { auto* item = information.add_items(); item->set_key(a.first); - item->set_description(std::string(a.second->description())); + item->set_description(std::string(a.second.description())); } } @@ -161,9 +161,9 @@ void metrics_store_impl::set_item_value(::tateyama::proto::metrics::response::Me for (auto&& a: aggregations_) { auto* item = information.add_items(); item->set_key(a.first); - item->set_description(std::string(a.second->description())); + item->set_description(std::string(a.second.description())); - auto aggregator = a.second->create_aggregator(); + auto aggregator = a.second.create_aggregator(); const auto aggregate_internal = [&aggregator, &a, item](std::map>& metrics) { for (auto&& fmap: metrics) { for (auto&& e: *fmap.second) { diff --git a/src/tateyama/metrics/resource/metrics_store_impl.h b/src/tateyama/metrics/resource/metrics_store_impl.h index 326a66e2..20fa1ee0 100644 --- a/src/tateyama/metrics/resource/metrics_store_impl.h +++ b/src/tateyama/metrics/resource/metrics_store_impl.h @@ -53,7 +53,7 @@ class metrics_store_impl { public: metrics_item_slot& register_item(const metrics_metadata& metadata); - void register_aggregation(std::unique_ptr aggregation); + void register_aggregation(const metrics_aggregation& aggregation); bool unregister_element(std::string_view key); @@ -64,7 +64,7 @@ class metrics_store_impl { private: std::map> metrics_{}; std::map> invisible_metrics_{}; - std::map> aggregations_{}; + std::map aggregations_{}; void set_item_description(::tateyama::proto::metrics::response::MetricsInformation& information); diff --git a/test/tateyama/metrics/metrics_aggregation_test.cpp b/test/tateyama/metrics/metrics_aggregation_test.cpp index c8e1a3fd..d409de30 100644 --- a/test/tateyama/metrics/metrics_aggregation_test.cpp +++ b/test/tateyama/metrics/metrics_aggregation_test.cpp @@ -40,20 +40,6 @@ class aggregator_for_aggregation_test : public metrics_aggregator { double value_{}; }; -// example aggregation -class aggregation_for_aggregation_test : public metrics_aggregation { -public: - aggregation_for_aggregation_test( - const std::string& group_key, - const std::string& description) : - metrics_aggregation(group_key, description) { - } - std::unique_ptr create_aggregator() const noexcept override { - return std::make_unique(); - } -}; - - class metrics_aggregation_test : public ::testing::Test, public test::test_utils @@ -103,7 +89,7 @@ TEST_F(metrics_aggregation_test, basic) { auto& slot_A1 = metrics_store_->register_item(metadata_table_A1_); auto& slot_A2 = metrics_store_->register_item(metadata_table_A2_); auto& slot_B = metrics_store_->register_item(metadata_table_B_); - metrics_store_->register_aggregation(std::make_unique("table_count", "number of user tables")); + metrics_store_->register_aggregation(tateyama::metrics::metrics_aggregation{"table_count", "number of user tables", [](){return std::make_unique();}}); slot_A1 = 65536; slot_A2 = 16777216; diff --git a/test/tateyama/metrics/resource/metrics_bridge_test.cpp b/test/tateyama/metrics/resource/metrics_bridge_test.cpp index c72b26d7..a3421334 100644 --- a/test/tateyama/metrics/resource/metrics_bridge_test.cpp +++ b/test/tateyama/metrics/resource/metrics_bridge_test.cpp @@ -30,8 +30,6 @@ using namespace std::literals::string_literals; // example aggregator class aggregator_for_bridge_test : public metrics_aggregator { public: - aggregator_for_bridge_test(std::string_view group_key) : group_key_(group_key) { - } void add(metrics_metadata const& metadata, double value) override { value_ += 1.0; } @@ -40,24 +38,9 @@ class aggregator_for_bridge_test : public metrics_aggregator { } private: - const std::string group_key_; double value_{}; }; -// example aggregation -class aggregation_for_bridge_test : public metrics_aggregation { -public: - aggregation_for_bridge_test( - const std::string& group_key, - const std::string& description) : - metrics_aggregation(group_key, description) { - } - - std::unique_ptr create_aggregator() const noexcept override { - return std::make_unique(group_key()); - } -}; - class metrics_bridge_test : public ::testing::Test, public test::test_utils { @@ -140,7 +123,7 @@ TEST_F(metrics_bridge_test, list) { auto& slot_A1 = metrics_store.register_item(metadata_table_A1_); auto& slot_A2 = metrics_store.register_item(metadata_table_A2_); auto& slot_B = metrics_store.register_item(metadata_table_B_); - metrics_store.register_aggregation(std::make_unique("table_count", "number of user tables")); + metrics_store.register_aggregation(tateyama::metrics::metrics_aggregation{"table_count", "number of user tables", [](){return std::make_unique();}}); session_count = 100; storage_log_size = 65535; @@ -188,7 +171,7 @@ TEST_F(metrics_bridge_test, show) { auto& slot_A1 = metrics_store.register_item(metadata_table_A1_); auto& slot_A2 = metrics_store.register_item(metadata_table_A2_); auto& slot_B = metrics_store.register_item(metadata_table_B_); - metrics_store.register_aggregation(std::make_unique("table_count", "number of user tables")); + metrics_store.register_aggregation(tateyama::metrics::metrics_aggregation{"table_count", "number of user tables", [](){return std::make_unique();}}); session_count = 100; storage_log_size = 65535;