This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit df9e3a655467a3036ac7540c13266b6725654572 Author: Benjamin Mahler <[email protected]> AuthorDate: Tue Jan 28 13:42:36 2020 -0500 Improved performance of v1 agent operator API GET_METRICS call. This follow the same approach used for the master's GET_METRICS call: https://github.com/apache/mesos/commit/469f2ebaf65b1642d1eb4a1df81a https://github.com/apache/mesos/commit/3dda3622f5ed01e8c132dc5ca594 That is, serializing directly to protobuf or json from the in-memory v0 state. Review: https://reviews.apache.org/r/72056 --- src/common/http.hpp | 110 ++++++++++++++++++++++++++++++++++++++++++++++++++++ src/master/http.cpp | 108 ++------------------------------------------------- src/slave/http.cpp | 76 +++++++++++++++++++++++++++++------- 3 files changed, 176 insertions(+), 118 deletions(-) diff --git a/src/common/http.hpp b/src/common/http.hpp index 5fc19fd..4a0f4a8 100644 --- a/src/common/http.hpp +++ b/src/common/http.hpp @@ -214,6 +214,116 @@ JSON::Object model(const FileInfo& fileInfo); void json(JSON::ObjectWriter* writer, const Task& task); + +// NOTE: The `metrics` object provided as an argument must outlive +// the returned function, since the function captures `metrics` +// by reference to avoid a really expensive map copy. This is a +// rather unsafe approach, but in typical jsonify usage this is +// not an issue. +// +// TODO(bmahler): Use std::enable_if with std::is_same to check +// that T is either the master or agent GetMetrics. +template <typename T> +std::function<void(JSON::ObjectWriter*)> jsonifyGetMetrics( + const std::map<std::string, double>& metrics) +{ + // Serialize the following message: + // + // mesos::master::Response::GetMetrics getMetrics; + // // or: mesos::agent::Response::GetMetrics getMetrics; + // + // foreachpair (const string& key, double value, metrics) { + // Metric* metric = getMetrics->add_metrics(); + // metric->set_name(key); + // metric->set_value(value); + // } + + return [&](JSON::ObjectWriter* writer) { + const google::protobuf::Descriptor* descriptor = T::descriptor(); + + int field = T::kMetricsFieldNumber; + + writer->field( + descriptor->FindFieldByNumber(field)->name(), + [&](JSON::ArrayWriter* writer) { + foreachpair (const std::string& key, double value, metrics) { + writer->element([&](JSON::ObjectWriter* writer) { + const google::protobuf::Descriptor* descriptor = + v1::Metric::descriptor(); + + int field; + + field = v1::Metric::kNameFieldNumber; + writer->field( + descriptor->FindFieldByNumber(field)->name(), key); + + field = v1::Metric::kValueFieldNumber; + writer->field( + descriptor->FindFieldByNumber(field)->name(), value); + }); + } + }); + }; +} + + +// TODO(bmahler): Use std::enable_if with std::is_same to check +// that T is either the master or agent GetMetrics. +template <typename T> +std::string serializeGetMetrics( + const std::map<std::string, double>& metrics) +{ + // Serialize the following message: + // + // v1::master::Response::GetMetrics getMetrics; + // // or: v1::agent::Response::GetMetrics getMetrics; + // + // foreachpair (const string& key, double value, metrics) { + // Metric* metric = getMetrics->add_metrics(); + // metric->set_name(key); + // metric->set_value(value); + // } + + auto serializeMetric = [](const std::string& key, double value) { + std::string output; + google::protobuf::io::StringOutputStream stream(&output); + google::protobuf::io::CodedOutputStream writer(&stream); + + google::protobuf::internal::WireFormatLite::WriteString( + v1::Metric::kNameFieldNumber, key, &writer); + google::protobuf::internal::WireFormatLite::WriteDouble( + v1::Metric::kValueFieldNumber, value, &writer); + + // While an explicit Trim() isn't necessary (since the coded + // output stream is destructed before the string is returned), + // it's a quite tricky bug to diagnose if Trim() is missed, so + // we always do it explicitly to signal the reader about this + // subtlety. + writer.Trim(); + return output; + }; + + std::string output; + google::protobuf::io::StringOutputStream stream(&output); + google::protobuf::io::CodedOutputStream writer(&stream); + + foreachpair (const std::string& key, double value, metrics) { + google::protobuf::internal::WireFormatLite::WriteBytes( + T::kMetricsFieldNumber, + serializeMetric(key, value), + &writer); + } + + // While an explicit Trim() isn't necessary (since the coded + // output stream is destructed before the string is returned), + // it's a quite tricky bug to diagnose if Trim() is missed, so + // we always do it explicitly to signal the reader about this + // subtlety. + writer.Trim(); + return output; +} + + } // namespace internal { void json(JSON::ObjectWriter* writer, const Attributes& attributes); diff --git a/src/master/http.cpp b/src/master/http.cpp index 81ab26a..bf04395 100644 --- a/src/master/http.cpp +++ b/src/master/http.cpp @@ -1670,108 +1670,6 @@ Future<Response> Master::Http::getVersion( } -// NOTE: The `metrics` object provided as an argument must outlive -// the returned function, since the function captures `metrics` -// by reference to avoid a really expensive map copy. This is a -// rather unsafe approach, but in typical jsonify usage this is -// not an issue. -function<void(JSON::ObjectWriter*)> jsonifyGetMetrics( - const map<string, double>& metrics) -{ - // Serialize the following message: - // - // mesos::master::Response::GetMetrics getMetrics; - // - // foreachpair (const string& key, double value, metrics) { - // Metric* metric = getMetrics->add_metrics(); - // metric->set_name(key); - // metric->set_value(value); - // } - - return [&](JSON::ObjectWriter* writer) { - const google::protobuf::Descriptor* descriptor = - v1::master::Response::GetMetrics::descriptor(); - - int field; - - field = v1::master::Response::GetMetrics::kMetricsFieldNumber; - writer->field( - descriptor->FindFieldByNumber(field)->name(), - [&](JSON::ArrayWriter* writer) { - foreachpair (const string& key, double value, metrics) { - writer->element([&](JSON::ObjectWriter* writer) { - const google::protobuf::Descriptor* descriptor = - v1::Metric::descriptor(); - - int field; - - field = v1::Metric::kNameFieldNumber; - writer->field( - descriptor->FindFieldByNumber(field)->name(), key); - - field = v1::Metric::kValueFieldNumber; - writer->field( - descriptor->FindFieldByNumber(field)->name(), value); - }); - } - }); - }; -} - - -string serializeGetMetrics(const map<string, double>& metrics) -{ - // Serialize the following message: - // - // mesos::master::Response::GetMetrics getMetrics; - // - // foreachpair (const string& key, double value, metrics) { - // Metric* metric = getMetrics->add_metrics(); - // metric->set_name(key); - // metric->set_value(value); - // } - - auto serializeMetric = [](const string& key, double value) { - string output; - google::protobuf::io::StringOutputStream stream(&output); - google::protobuf::io::CodedOutputStream writer(&stream); - - WireFormatLite::WriteString( - mesos::v1::Metric::kNameFieldNumber, key, &writer); - - WireFormatLite::WriteDouble( - mesos::v1::Metric::kValueFieldNumber, value, &writer); - - // While an explicit Trim() isn't necessary (since the coded - // output stream is destructed before the string is returned), - // it's a quite tricky bug to diagnose if Trim() is missed, so - // we always do it explicitly to signal the reader about this - // subtlety. - writer.Trim(); - return output; - }; - - string output; - google::protobuf::io::StringOutputStream stream(&output); - google::protobuf::io::CodedOutputStream writer(&stream); - - foreachpair (const string& key, double value, metrics) { - WireFormatLite::WriteBytes( - mesos::v1::master::Response::GetMetrics::kMetricsFieldNumber, - serializeMetric(key, value), - &writer); - } - - // While an explicit Trim() isn't necessary (since the coded - // output stream is destructed before the string is returned), - // it's a quite tricky bug to diagnose if Trim() is missed, so - // we always do it explicitly to signal the reader about this - // subtlety. - writer.Trim(); - return output; -} - - Future<Response> Master::Http::getMetrics( const mesos::master::Call& call, const Option<Principal>& principal, @@ -1806,7 +1704,8 @@ Future<Response> Master::Http::getMetrics( WireFormatLite::WriteBytes( mesos::v1::master::Response::kGetMetricsFieldNumber, - serializeGetMetrics(metrics), + serializeGetMetrics<mesos::v1::master::Response::GetMetrics>( + metrics), &writer); // We must manually trim the unused buffer space since @@ -1833,7 +1732,8 @@ Future<Response> Master::Http::getMetrics( field = v1::master::Response::kGetMetricsFieldNumber; writer->field( descriptor->FindFieldByNumber(field)->name(), - jsonifyGetMetrics(metrics)); + jsonifyGetMetrics<mesos::v1::master::Response::GetMetrics>( + metrics)); }); // TODO(bmahler): Pass jsonp query parameter through here. diff --git a/src/slave/http.cpp b/src/slave/http.cpp index 04ad0d8..095e787 100644 --- a/src/slave/http.cpp +++ b/src/slave/http.cpp @@ -82,6 +82,8 @@ #include "version/version.hpp" +using google::protobuf::internal::WireFormatLite; + using mesos::agent::ProcessIO; using mesos::authorization::createSubject; @@ -109,6 +111,8 @@ using mesos::authorization::MODIFY_RESOURCE_PROVIDER_CONFIG; using mesos::authorization::MARK_RESOURCE_PROVIDER_GONE; using mesos::authorization::PRUNE_IMAGES; +using mesos::internal::protobuf::WireFormatLite2; + using mesos::internal::recordio::Reader; using mesos::slave::ContainerClass; @@ -151,6 +155,7 @@ using process::metrics::internal::MetricsProcess; using ::recordio::Decoder; +using std::function; using std::list; using std::map; using std::string; @@ -1049,7 +1054,7 @@ Future<Response> Http::getVersion( Future<Response> Http::getMetrics( const mesos::agent::Call& call, - ContentType acceptType, + ContentType contentType, const Option<Principal>& principal) const { CHECK_EQ(mesos::agent::Call::GET_METRICS, call.type()); @@ -1063,21 +1068,64 @@ Future<Response> Http::getMetrics( } return process::metrics::snapshot(timeout) - .then([acceptType](const map<string, double>& metrics) -> Response { - mesos::agent::Response response; - response.set_type(mesos::agent::Response::GET_METRICS); - mesos::agent::Response::GetMetrics* _getMetrics = - response.mutable_get_metrics(); - - foreachpair (const string& key, double value, metrics) { - Metric* metric = _getMetrics->add_metrics(); - metric->set_name(key); - metric->set_value(value); + .then([contentType](const map<string, double>& metrics) -> Response { + // Serialize the following message: + // + // v1::agent::Response response; + // response.set_type(v1::agent::Response::GET_METRICS); + // v1::agent::Response::GetMetrics* getMetrics = ...; + + switch (contentType) { + case ContentType::PROTOBUF: { + string output; + google::protobuf::io::StringOutputStream stream(&output); + google::protobuf::io::CodedOutputStream writer(&stream); + + WireFormatLite::WriteEnum( + v1::agent::Response::kTypeFieldNumber, + v1::agent::Response::GET_METRICS, + &writer); + + WireFormatLite::WriteBytes( + v1::agent::Response::kGetMetricsFieldNumber, + serializeGetMetrics<v1::agent::Response::GetMetrics>(metrics), + &writer); + + // We must manually trim the unused buffer space since + // we use the string before the coded output stream is + // destructed. + writer.Trim(); + + return OK(std::move(output), stringify(contentType)); } - return OK(serialize(acceptType, evolve(response)), - stringify(acceptType)); - }); + case ContentType::JSON: { + string body = jsonify([&](JSON::ObjectWriter* writer) { + const google::protobuf::Descriptor* descriptor = + v1::agent::Response::descriptor(); + + int field; + + field = v1::agent::Response::kTypeFieldNumber; + writer->field( + descriptor->FindFieldByNumber(field)->name(), + v1::agent::Response::Type_Name( + v1::agent::Response::GET_METRICS)); + + field = v1::agent::Response::kGetMetricsFieldNumber; + writer->field( + descriptor->FindFieldByNumber(field)->name(), + jsonifyGetMetrics<v1::agent::Response::GetMetrics>(metrics)); + }); + + // TODO(bmahler): Pass jsonp query parameter through here. + return OK(std::move(body), stringify(contentType)); + } + + default: + return NotAcceptable("Request must accept json or protobuf"); + } + }); }
