Cleaned up weights handling code.

Removed a redundant statement, fixed log message style, cleaned up
comments.

Review: https://reviews.apache.org/r/56804/


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/eef4340e
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/eef4340e
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/eef4340e

Branch: refs/heads/master
Commit: eef4340ef26289d7dd6cc6b0cc56139ce5f0059e
Parents: 190c9e0
Author: Neil Conway <[email protected]>
Authored: Mon Feb 20 20:18:48 2017 -0800
Committer: Adam B <[email protected]>
Committed: Mon Feb 20 20:18:48 2017 -0800

----------------------------------------------------------------------
 src/master/master.cpp               |  6 ++--
 src/master/weights_handler.cpp      |  2 +-
 src/tests/dynamic_weights_tests.cpp | 50 ++++++++++++++++----------------
 3 files changed, 28 insertions(+), 30 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/eef4340e/src/master/master.cpp
----------------------------------------------------------------------
diff --git a/src/master/master.cpp b/src/master/master.cpp
index 0f9fc39..2ef8365 100644
--- a/src/master/master.cpp
+++ b/src/master/master.cpp
@@ -1720,8 +1720,8 @@ Future<Nothing> Master::_recover(const Registry& registry)
     // for weights, so the `--weights` flag can be deprecated and this check
     // can eventually be removed.
     if (!weights.empty()) {
-      LOG(WARNING) << "Ignoring the --weights flag '" << flags.weights.get()
-                   << "', and recovering the weights from registry.";
+      LOG(WARNING) << "Ignoring --weights flag '" << flags.weights.get()
+                   << "' and recovering the weights from registry";
 
       // Before recovering weights from the registry, the allocator was already
       // initialized with `--weights`, so here we need to reset (to 1.0)
@@ -1734,8 +1734,6 @@ Future<Nothing> Master::_recover(const Registry& registry)
           weightInfos.push_back(weightInfo);
         }
       }
-      // Clear weights specified by `--weights` flag.
-      weights.clear();
     }
 
     // Recover `weights` with `registry_weights`.

http://git-wip-us.apache.org/repos/asf/mesos/blob/eef4340e/src/master/weights_handler.cpp
----------------------------------------------------------------------
diff --git a/src/master/weights_handler.cpp b/src/master/weights_handler.cpp
index da0b995..4f6a4cd 100644
--- a/src/master/weights_handler.cpp
+++ b/src/master/weights_handler.cpp
@@ -57,7 +57,7 @@ Future<http::Response> Master::WeightsHandler::get(
     const http::Request& request,
     const Option<string>& principal) const
 {
-  VLOG(1) << "Handling get weights request.";
+  VLOG(1) << "Handling get weights request";
 
   // Check that the request type is GET which is guaranteed by the master.
   CHECK_EQ("GET", request.method);

http://git-wip-us.apache.org/repos/asf/mesos/blob/eef4340e/src/tests/dynamic_weights_tests.cpp
----------------------------------------------------------------------
diff --git a/src/tests/dynamic_weights_tests.cpp 
b/src/tests/dynamic_weights_tests.cpp
index ce577ce..5571ab7 100644
--- a/src/tests/dynamic_weights_tests.cpp
+++ b/src/tests/dynamic_weights_tests.cpp
@@ -145,7 +145,7 @@ protected:
 
 
 // Update weights requests with invalid JSON structure
-// should return a '400 Bad Request'.
+// should return '400 Bad Request'.
 TEST_F(DynamicWeightsTest, PutInvalidRequest)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
@@ -196,14 +196,14 @@ TEST_F(DynamicWeightsTest, PutInvalidRequest)
 }
 
 
-// A update weights request with zero value
-// should return a '400 Bad Request'.
+// An update weights request with zero value
+// should return '400 Bad Request'.
 TEST_F(DynamicWeightsTest, ZeroWeight)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  // Send a weight update request to update the weight of 'role1' to 0.
+  // Send a request to update the weight of 'role1' to 0.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos("role1=0");
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -221,14 +221,14 @@ TEST_F(DynamicWeightsTest, ZeroWeight)
 }
 
 
-// A update weights request with negative value
-// should return a '400 Bad Request'.
+// An update weights request with negative value
+// should return '400 Bad Request'.
 TEST_F(DynamicWeightsTest, NegativeWeight)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  // Send a weight update request to update the weight of 'role1' to -2.0.
+  // Send a request to update the weight of 'role1' to -2.0.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos("role1=-2.0");
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -246,14 +246,14 @@ TEST_F(DynamicWeightsTest, NegativeWeight)
 }
 
 
-// A update weights request with non-numeric value
-// should return a '400 Bad Request'.
+// An update weights request with non-numeric value
+// should return '400 Bad Request'.
 TEST_F(DynamicWeightsTest, NonNumericWeight)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
   ASSERT_SOME(master);
 
-  // Send a weight update request to update the weight of 'role1' to 'two'.
+  // Send a request to update the weight of 'role1' to 'two'.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos("role1=two");
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -272,7 +272,7 @@ TEST_F(DynamicWeightsTest, NonNumericWeight)
 
 
 // Updates must be explicit about what role they are updating,
-// and a missing role request should return a '400 Bad Request'.
+// and a missing role request should return '400 Bad Request'.
 TEST_F(DynamicWeightsTest, MissingRole)
 {
   Try<Owned<cluster::Master>> master = StartMaster();
@@ -312,7 +312,7 @@ TEST_F(DynamicWeightsTest, MissingRole)
 
 
 // Verifies that an update request for an unknown role (not specified
-// in --roles flag) is rejected when using the explicit roles (--roles flag
+// in --roles flag) is rejected when using explicit roles (--roles flag
 // is specified when master is started).
 TEST_F(DynamicWeightsTest, UnknownRole)
 {
@@ -341,8 +341,8 @@ TEST_F(DynamicWeightsTest, UnknownRole)
 
 
 // Verifies that a weights update request for a whitelisted role
-// (contained in --roles flag) succeeds when using the explicit
-// roles (--roles flag is specified when master is started).
+// (contained in --roles flag) succeeds when using explicit roles
+// (--roles flag is specified when master is started).
 TEST_F(DynamicWeightsTest, UpdateWeightsWithExplictRoles)
 {
   // Specify --roles whitelist when starting master.
@@ -353,7 +353,7 @@ TEST_F(DynamicWeightsTest, UpdateWeightsWithExplictRoles)
 
   checkWithGetRequest(master.get()->pid, DEFAULT_CREDENTIAL);
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -384,7 +384,7 @@ TEST_F(DynamicWeightsTest, 
UnauthenticatedUpdateWeightRequest)
   credential.set_principal("unknown-principal");
   credential.set_secret("test-secret");
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response1 = process::http::request(
       process::http::createRequest(
@@ -473,7 +473,7 @@ TEST_F(DynamicWeightsTest, AuthorizedGetWeightsRequest)
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -517,7 +517,7 @@ TEST_F(DynamicWeightsTest, AuthorizedWeightUpdateRequest)
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -562,7 +562,7 @@ TEST_F(DynamicWeightsTest, 
AuthorizedUpdateWeightRequestWithoutPrincipal)
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -594,7 +594,7 @@ TEST_F(DynamicWeightsTest, UnauthorizedWeightUpdateRequest)
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
-  // Send a weight update request for the specified roles in UPDATED_WEIGHTS1.
+  // Send a weight update request for the roles in UPDATED_WEIGHTS1.
   RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS1);
   Future<Response> response = process::http::request(
       process::http::createRequest(
@@ -618,13 +618,13 @@ TEST_F(DynamicWeightsTest, RecoveredWeightsFromRegistry)
   // Start a master with `--weights` flag.
   master::Flags masterFlags = CreateMasterFlags();
   masterFlags.registry = "replicated_log";
-
   masterFlags.weights = UPDATED_WEIGHTS1;
+
   Try<Owned<cluster::Master>> master = StartMaster(masterFlags);
   ASSERT_SOME(master);
 
-  // Tests whether the weights replicated log is initialized with the
-  // `--weights` flag when bootstrapping the cluster.
+  // Tests whether the weights stored in the registry are initialized
+  // using the `--weights` flag when the cluster is bootstrapped.
   {
     checkWithGetRequest(
         master.get()->pid,
@@ -645,8 +645,8 @@ TEST_F(DynamicWeightsTest, RecoveredWeightsFromRegistry)
         UPDATED_WEIGHTS1);
   }
 
-  // Tests whether the weights replicated log can be updated with
-  // `/weights` endpoint.
+  // Tests whether the weights stored in the registry are updated
+  // successfully using the `/weights` endpoint.
   {
     // Send a weights update request for the specified roles.
     RepeatedPtrField<WeightInfo> infos = createWeightInfos(UPDATED_WEIGHTS2);

Reply via email to