Address code review comments

azureplus · Nov 19, 2015 · 52a514a · 52a514a
1 parent 87f5166
commit 52a514a
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 25 deletions.
diff --git a/Makefile b/Makefile
diff --git a/test/cpp/interop/metrics_client.cc b/test/cpp/interop/metrics_client.cc
@@ -53,7 +53,7 @@ using grpc::testing::MetricsServiceImpl;
 void PrintMetrics(grpc::string& server_address) {
   gpr_log(GPR_INFO, "creating a channel to %s", server_address.c_str());
   std::shared_ptr<grpc::Channel> channel(
-      grpc::CreateChannel(server_address, grpc::InsecureCredentials()));
+      grpc::CreateChannel(server_address, grpc::InsecureChannelCredentials()));
 
   std::unique_ptr<MetricsService::Stub> stub(MetricsService::NewStub(channel));
 

diff --git a/test/cpp/interop/stress_test.cc b/test/cpp/interop/stress_test.cc
@@ -219,9 +219,9 @@ int main(int argc, char** argv) {
   int thread_idx = 0;
   for (auto it = server_addresses.begin(); it != server_addresses.end(); it++) {
     // TODO(sreek): This will change once we add support for other tests
-    // that won't work with InsecureCredentials()
+    // that won't work with InsecureChannelCredentials()
     std::shared_ptr<grpc::Channel> channel(
-        CreateChannel(*it, grpc::InsecureCredentials()));
+        grpc::CreateChannel(*it, grpc::InsecureChannelCredentials()));
 
     // Make multiple stubs (as defined by num_stubs_per_channel flag) to use the
     // same channel. This is to test calling multiple RPC calls in parallel on

diff --git a/test/cpp/util/metrics_server.cc b/test/cpp/util/metrics_server.cc
@@ -33,8 +33,6 @@
 
 #include "test/cpp/util/metrics_server.h"
 
-#include <vector>
-
 #include <grpc++/server_builder.h>
 
 #include "test/proto/metrics.grpc.pb.h"
@@ -43,15 +41,17 @@
 namespace grpc {
 namespace testing {
 
-using std::vector;
-
 Gauge::Gauge(long initial_val) : val_(initial_val) {}
 
 void Gauge::Set(long new_val) {
-  val_.store(new_val, std::memory_order_relaxed);
+  std::lock_guard<std::mutex> lock(val_mu_);
+  val_ = new_val;
 }
 
-long Gauge::Get() { return val_.load(std::memory_order_relaxed); }
+long Gauge::Get() {
+  std::lock_guard<std::mutex> lock(val_mu_);
+  return val_;
+}
 
 grpc::Status MetricsServiceImpl::GetAllGauges(
     ServerContext* context, const EmptyMessage* request,
@@ -74,7 +74,7 @@ grpc::Status MetricsServiceImpl::GetGauge(ServerContext* context,
                                           GaugeResponse* response) {
   std::lock_guard<std::mutex> lock(mu_);
 
-  auto it = gauges_.find(request->name());
+  const auto it = gauges_.find(request->name());
   if (it != gauges_.end()) {
     response->set_name(it->first);
     response->set_long_value(it->second->Get());
@@ -88,16 +88,17 @@ std::shared_ptr<Gauge> MetricsServiceImpl::CreateGauge(const grpc::string& name,
   std::lock_guard<std::mutex> lock(mu_);
 
   std::shared_ptr<Gauge> gauge(new Gauge(0));
-  auto p = gauges_.emplace(name, gauge);
+  const auto p = gauges_.emplace(name, gauge);
 
   // 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
+  // is a boolean which is set to 'true' if the Gauge is inserted in the guages_
+  // map and 'false' if it is already present in the map
   *already_present = !p.second;
   return p.first->second;
 }
 
-// Starts the metrics server and returns the grpc::Server instance. Call
-// wait() on the returned server instance.
+// Starts the metrics server and returns the grpc::Server instance. Call Wait()
+// on the returned server instance.
 std::unique_ptr<grpc::Server> MetricsServiceImpl::StartServer(int port) {
   gpr_log(GPR_INFO, "Building metrics server..");
 

diff --git a/test/cpp/util/metrics_server.h b/test/cpp/util/metrics_server.h
@@ -33,7 +33,6 @@
 #ifndef GRPC_TEST_CPP_METRICS_SERVER_H
 #define GRPC_TEST_CPP_METRICS_SERVER_H
 
-#include <atomic>
 #include <map>
 #include <mutex>
 
@@ -69,7 +68,8 @@ class Gauge {
   long Get();
 
  private:
-  std::atomic_long val_;
+  long val_;
+  std::mutex val_mu_;
 };
 
 class MetricsServiceImpl GRPC_FINAL : public MetricsService::Service {