allow several <graphite> targets #603
Conversation
| namespace DB | ||
| { | ||
| /// get all internal key names for given key | ||
| std::vector<std::string> config_keys_multi(Poco::Util::AbstractConfiguration & config, const std::string & root, const std::string & name); |
There was a problem hiding this comment.
Function name is not in style.
| auto & config = Poco::Util::Application::instance().config(); | ||
| auto interval = config.getInt(config_name + ".interval", 60); | ||
|
|
||
| setThreadName(("MetricsTransmit " + std::to_string(interval) + "s").c_str()); |
There was a problem hiding this comment.
Thread name could be maximum 15 bytes long.
This code is misleading.
| std::chrono::time_point_cast<std::chrono::minutes, std::chrono::system_clock>( | ||
| std::chrono::system_clock::now() + std::chrono::minutes(seconds / 60))); | ||
| return std::chrono::time_point_cast<std::chrono::seconds, std::chrono::system_clock>( | ||
| std::chrono::system_clock::now() + std::chrono::seconds(seconds)); |
There was a problem hiding this comment.
I think, you should also align any intervals, which length is divisor of 60. Or all intervals.
For example, 15-second intervals should be aligned to 15-second boundaries, to avoid time drift.
dbms/src/Server/MetricsTransmitter.h
Outdated
| @@ -22,14 +23,15 @@ class AsynchronousMetrics; | |||
| class MetricsTransmitter | |||
| { | |||
| public: | |||
| MetricsTransmitter(const AsynchronousMetrics & async_metrics_) : async_metrics(async_metrics_) {} | |||
| MetricsTransmitter(const AsynchronousMetrics & async_metrics, const std::string & config_name) : async_metrics{async_metrics}, config_name{config_name} {} | |||
| namespace DB | ||
| { | ||
| /// get all internal key names for given key | ||
| std::vector<std::string> config_keys_multi(Poco::Util::AbstractConfiguration & config, const std::string & root, const std::string & name); |
There was a problem hiding this comment.
Probably you could use more meaningful name, like getMultipleKeysFromConfig.
dbms/src/Server/Server.cpp
Outdated
| @@ -558,8 +556,12 @@ int Server::main(const std::vector<std::string> & args) | |||
|
|
|||
| attach_system_tables_async(system_database, async_metrics); | |||
There was a problem hiding this comment.
Please correct function names according to style.
dbms/src/Server/config.xml
Outdated
| <graphite> | ||
| <host>localhost</host> | ||
| <port>42000</port> | ||
| <root_path>one_min</root_path> | ||
| <timeout>0.1</timeout> | ||
| <interval>60</interval> |
There was a problem hiding this comment.
You should add comment here.
dbms/src/Server/config.xml
Outdated
| <graphite> | ||
| <host>localhost</host> | ||
| <port>42000</port> | ||
| <root_path>one_min</root_path> | ||
| <timeout>0.1</timeout> | ||
| <interval>60</interval> | ||
|
|
||
| <metrics>true<metrics> |
| @@ -85,5 +85,29 @@ std::ostream & operator<<(std::ostream & stream, const std::pair<K, V> & what) | |||
| return stream; | |||
| } | |||
|
|
|||
| #include <ratio> | |||
|
|
|||
| template < std::intmax_t Num, std::intmax_t Denom> | |||
There was a problem hiding this comment.
Inconsistent whitespace. Double whitespace.
Num with capital letter is not in style, lowercase will be Ok..
libs/libdaemon/src/BaseDaemon.cpp
Outdated
| graphite_writer.reset(new GraphiteWriter("graphite")); | ||
| for (const auto & key : DB::config_keys_multi(config(), "", "graphite")) | ||
| { | ||
| graphite_writers.emplace(std::make_pair(key, new GraphiteWriter(key))); |
There was a problem hiding this comment.
Please avoid operator new, use make_unique instead.
You could construct std::pair inplace, no need for std::make_pair.
No description provided.