diff options
author | Sree Kuchibhotla <sreek@google.com> | 2015-11-16 11:52:54 -0800 |
---|---|---|
committer | Sree Kuchibhotla <sreek@google.com> | 2015-11-16 14:41:04 -0800 |
commit | b047c0fc9b455e6cbfd1d6b080e9bb427282b670 (patch) | |
tree | 6c8bd8598bfe51607fda1f7c92e5e04ee9de096b /test | |
parent | c36b480ce9354daaaf829de88a0a04e76873df7a (diff) |
Address code review comments
Diffstat (limited to 'test')
-rw-r--r-- | test/cpp/interop/metrics_client.cc | 12 | ||||
-rw-r--r-- | test/cpp/interop/stress_interop_client.h | 2 | ||||
-rw-r--r-- | test/cpp/interop/stress_test.cc | 26 | ||||
-rw-r--r-- | test/cpp/util/metrics_server.cc | 12 | ||||
-rw-r--r-- | test/cpp/util/metrics_server.h | 9 | ||||
-rw-r--r-- | test/proto/metrics.proto | 6 |
6 files changed, 35 insertions, 32 deletions
diff --git a/test/cpp/interop/metrics_client.cc b/test/cpp/interop/metrics_client.cc index a1def7299a..d68bf6d667 100644 --- a/test/cpp/interop/metrics_client.cc +++ b/test/cpp/interop/metrics_client.cc @@ -67,9 +67,13 @@ void PrintMetrics(grpc::string& server_address) { long overall_qps = 0; int idx = 0; while (reader->Read(&gauge_response)) { - gpr_log(GPR_INFO, "Gauge: %d (%s: %ld)", ++idx, - gauge_response.name().c_str(), gauge_response.value()); - overall_qps += gauge_response.value(); + if (gauge_response.value_case() == GaugeResponse::kLongValue) { + gpr_log(GPR_INFO, "Gauge: %d (%s: %ld)", ++idx, + gauge_response.name().c_str(), gauge_response.long_value()); + overall_qps += gauge_response.long_value(); + } else { + gpr_log(GPR_INFO, "Gauge %s is not a long value", gauge_response.name().c_str()); + } } gpr_log(GPR_INFO, "OVERALL: %ld", overall_qps); @@ -84,7 +88,7 @@ int main(int argc, char** argv) { grpc::testing::InitTest(&argc, &argv, true); // Make sure server_addresses flag is not empty - if (FLAGS_metrics_server_address.length() == 0) { + if (FLAGS_metrics_server_address.empty()) { gpr_log( GPR_ERROR, "Cannot connect to the Metrics server. Please pass the address of the" diff --git a/test/cpp/interop/stress_interop_client.h b/test/cpp/interop/stress_interop_client.h index 567c1b0a6d..6fd303d6b7 100644 --- a/test/cpp/interop/stress_interop_client.h +++ b/test/cpp/interop/stress_interop_client.h @@ -90,7 +90,7 @@ class StressTestInteropClient { long test_duration_secs, long sleep_duration_ms, long metrics_collection_interval_secs); - // The main funciton. Use this as the thread entry point. + // The main function. Use this as the thread entry point. // qps_gauge is the Gauge to record the requests per second metric void MainLoop(std::shared_ptr<Gauge> qps_gauge); diff --git a/test/cpp/interop/stress_test.cc b/test/cpp/interop/stress_test.cc index fffa3dd3de..2999d6aae2 100644 --- a/test/cpp/interop/stress_test.cc +++ b/test/cpp/interop/stress_test.cc @@ -41,6 +41,7 @@ #include <grpc/support/time.h> #include <grpc++/create_channel.h> #include <grpc++/grpc++.h> +#include <grpc++/impl/thd.h> #include "test/cpp/interop/interop_client.h" #include "test/cpp/interop/stress_interop_client.h" @@ -91,11 +92,6 @@ DEFINE_string(test_cases, "", " 'large_unary', 10% of the time and 'empty_stream' the remaining" " 70% of the time"); -using std::make_pair; -using std::pair; -using std::thread; -using std::vector; - using grpc::testing::kTestCaseList; using grpc::testing::MetricsService; using grpc::testing::MetricsServiceImpl; @@ -119,7 +115,7 @@ TestCaseType GetTestTypeFromName(const grpc::string& test_name) { // Converts a string of comma delimited tokens to a vector of tokens bool ParseCommaDelimitedString(const grpc::string& comma_delimited_str, - vector<grpc::string>& tokens) { + std::vector<grpc::string>& tokens) { size_t bpos = 0; size_t epos = grpc::string::npos; @@ -137,10 +133,10 @@ bool ParseCommaDelimitedString(const grpc::string& comma_delimited_str, // - Whether parsing was successful (return value) // - Vector of (test_type_enum, weight) pairs returned via 'tests' parameter bool ParseTestCasesString(const grpc::string& test_cases, - vector<pair<TestCaseType, int>>& tests) { + std::vector<std::pair<TestCaseType, int>>& tests) { bool is_success = true; - vector<grpc::string> tokens; + std::vector<grpc::string> tokens; ParseCommaDelimitedString(test_cases, tokens); for (auto it = tokens.begin(); it != tokens.end(); it++) { @@ -168,8 +164,8 @@ bool ParseTestCasesString(const grpc::string& test_cases, } // For debugging purposes -void LogParameterInfo(const vector<grpc::string>& addresses, - const vector<pair<TestCaseType, int>>& tests) { +void LogParameterInfo(const std::vector<grpc::string>& addresses, + const std::vector<std::pair<TestCaseType, int>>& tests) { gpr_log(GPR_INFO, "server_addresses: %s", FLAGS_server_addresses.c_str()); gpr_log(GPR_INFO, "test_cases : %s", FLAGS_test_cases.c_str()); gpr_log(GPR_INFO, "sleep_duration_ms: %d", FLAGS_sleep_duration_ms); @@ -195,7 +191,7 @@ int main(int argc, char** argv) { srand(time(NULL)); // Parse the server addresses - vector<grpc::string> server_addresses; + std::vector<grpc::string> server_addresses; ParseCommaDelimitedString(FLAGS_server_addresses, server_addresses); // Parse test cases and weights @@ -204,7 +200,7 @@ int main(int argc, char** argv) { return 1; } - vector<pair<TestCaseType, int>> tests; + std::vector<std::pair<TestCaseType, int>> tests; if (!ParseTestCasesString(FLAGS_test_cases, tests)) { gpr_log(GPR_ERROR, "Error in parsing test cases string %s ", FLAGS_test_cases.c_str()); @@ -218,7 +214,7 @@ int main(int argc, char** argv) { gpr_log(GPR_INFO, "Starting test(s).."); - vector<thread> test_threads; + std::vector<grpc::thread> test_threads; int thread_idx = 0; for (auto it = server_addresses.begin(); it != server_addresses.end(); it++) { @@ -239,8 +235,8 @@ int main(int argc, char** argv) { grpc::string metricName = "/stress_test/qps/thread/" + std::to_string(thread_idx); test_threads.emplace_back( - thread(&StressTestInteropClient::MainLoop, client, - metrics_service.CreateGauge(metricName, is_already_created))); + grpc::thread(&StressTestInteropClient::MainLoop, client, + metrics_service.CreateGauge(metricName, &is_already_created))); // The Gauge should not have been already created GPR_ASSERT(!is_already_created); diff --git a/test/cpp/util/metrics_server.cc b/test/cpp/util/metrics_server.cc index eac29f3093..5bb87e53f1 100644 --- a/test/cpp/util/metrics_server.cc +++ b/test/cpp/util/metrics_server.cc @@ -61,8 +61,8 @@ grpc::Status MetricsServiceImpl::GetAllGauges( std::lock_guard<std::mutex> lock(mu_); for (auto it = gauges_.begin(); it != gauges_.end(); it++) { GaugeResponse resp; - resp.set_name(it->first); // Gauge name - resp.set_value(it->second->Get()); // Gauge value + resp.set_name(it->first); // Gauge name + resp.set_long_value(it->second->Get()); // Gauge value writer->Write(resp); } @@ -77,14 +77,14 @@ grpc::Status MetricsServiceImpl::GetGauge(ServerContext* context, auto it = gauges_.find(request->name()); if (it != gauges_.end()) { response->set_name(it->first); - response->set_value(it->second->Get()); + response->set_long_value(it->second->Get()); } return Status::OK; } -std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(string name, - bool& already_present) { +std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(const grpc::string& name, + bool* already_present) { std::lock_guard<std::mutex> lock(mu_); std::shared_ptr<Gauge> gauge(new Gauge(0)); @@ -92,7 +92,7 @@ std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(string name, // p.first is an iterator pointing to <name, shared_ptr<Gauge>> pair. p.second // is a boolean indicating if the Gauge is already present in the map - already_present = !p.second; + *already_present = !p.second; return p.first->second; } diff --git a/test/cpp/util/metrics_server.h b/test/cpp/util/metrics_server.h index 19c3a0c32e..36529095f9 100644 --- a/test/cpp/util/metrics_server.h +++ b/test/cpp/util/metrics_server.h @@ -36,7 +36,6 @@ #include <atomic> #include <map> #include <mutex> -#include <vector> #include "test/proto/metrics.grpc.pb.h" #include "test/proto/metrics.pb.h" @@ -61,9 +60,8 @@ namespace grpc { namespace testing { -using std::map; -using std::vector; - +// TODO(sreek): Add support for other types of Gauges like Double, String in +// future class Gauge { public: Gauge(long initial_val); @@ -86,7 +84,8 @@ class MetricsServiceImpl GRPC_FINAL : public MetricsService::Service { // is already present in the map. // NOTE: CreateGauge can be called anytime (i.e before or after calling // StartServer). - std::shared_ptr<Gauge> CreateGauge(string name, bool& is_present); + std::shared_ptr<Gauge> CreateGauge(const grpc::string& name, + bool* already_present); std::unique_ptr<grpc::Server> StartServer(int port); diff --git a/test/proto/metrics.proto b/test/proto/metrics.proto index f434c95390..14740eb65d 100644 --- a/test/proto/metrics.proto +++ b/test/proto/metrics.proto @@ -36,7 +36,11 @@ package grpc.testing; message GaugeResponse { string name = 1; - int64 value = 2; + oneof value { + int64 long_value = 2; + double double_vale = 3; + string string_value = 4; + } } message GaugeRequest { |