Repository: mesos Updated Branches: refs/heads/master 42971bd95 -> 30d1bb7c3
Integer Precision for JSON <-> Protobuf conversions. * Add TODO's for refactoring some JSON parsing in the docker code (See MESOS-3409). Update how the JSON::Number is used. * Tweak some tests to match changes to JSON::Number. * Address a TODO on one test, which used a workaround for double-precision comparison. Review: https://reviews.apache.org/r/38077 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/30d1bb7c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/30d1bb7c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/30d1bb7c Branch: refs/heads/master Commit: 30d1bb7c378a23e3c72782d2a0e89a6681eb9a8b Parents: 1116ad5 Author: Joseph Wu <[email protected]> Authored: Fri Sep 18 12:11:45 2015 -0400 Committer: Joris Van Remoortere <[email protected]> Committed: Fri Sep 18 12:37:18 2015 -0400 ---------------------------------------------------------------------- src/docker/docker.cpp | 3 +- .../provisioner/docker/token_manager.cpp | 3 +- src/tests/fault_tolerance_tests.cpp | 2 +- src/tests/master_tests.cpp | 2 +- src/tests/monitor_tests.cpp | 50 +++------ src/tests/rate_limiting_tests.cpp | 106 +++++++++++++------ src/tests/slave_tests.cpp | 2 +- 7 files changed, 93 insertions(+), 75 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/docker/docker.cpp ---------------------------------------------------------------------- diff --git a/src/docker/docker.cpp b/src/docker/docker.cpp index c4c37cb..afcedf1 100755 --- a/src/docker/docker.cpp +++ b/src/docker/docker.cpp @@ -236,6 +236,7 @@ Try<Nothing> Docker::validateVersion(const Version& minVersion) const } +// TODO(josephw): Parse this string with a protobuf. Try<Docker::Container> Docker::Container::create(const string& output) { Try<JSON::Array> parse = JSON::parse<JSON::Array>(output); @@ -286,7 +287,7 @@ Try<Docker::Container> Docker::Container::create(const string& output) return Error("Error finding Pid in State: " + pidValue.error()); } - pid_t pid = pid_t(pidValue.get().value); + pid_t pid = pid_t(pidValue.get().as<int64_t>()); Option<pid_t> optionalPid; if (pid != 0) { http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/slave/containerizer/provisioner/docker/token_manager.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/provisioner/docker/token_manager.cpp b/src/slave/containerizer/provisioner/docker/token_manager.cpp index cf52626..18b29c3 100644 --- a/src/slave/containerizer/provisioner/docker/token_manager.cpp +++ b/src/slave/containerizer/provisioner/docker/token_manager.cpp @@ -122,6 +122,7 @@ Token::Token( notBefore(_notBefore) {} +// TODO(josephw): Parse this string with some protobufs. Try<Token> Token::create(const string& raw) { auto decode = []( @@ -196,7 +197,7 @@ Result<Time> Token::getTimeValue(const JSON::Object& object, const string& key) // If expiration is provided, we will process it for future validations. if (jsonValue.isSome()) { - Try<Time> time = Time::create(jsonValue.get().value); + Try<Time> time = Time::create(jsonValue.get().as<double>()); if (time.isError()) { return Error("Failed to decode time: " + time.error()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/tests/fault_tolerance_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/fault_tolerance_tests.cpp b/src/tests/fault_tolerance_tests.cpp index 061e099..c97bc46 100644 --- a/src/tests/fault_tolerance_tests.cpp +++ b/src/tests/fault_tolerance_tests.cpp @@ -1918,7 +1918,7 @@ TEST_F(FaultToleranceTest, UpdateFrameworkInfoOnSchedulerFailover) EXPECT_EQ(1u, framework.values.count("failover_timeout")); JSON::Number failoverTimeout = framework.values["failover_timeout"].as<JSON::Number>(); - EXPECT_EQ(finfo2.failover_timeout(), failoverTimeout.value); + EXPECT_EQ(finfo2.failover_timeout(), failoverTimeout.as<double>()); EXPECT_EQ(1u, framework.values.count("hostname")); JSON::String hostname = framework.values["hostname"].as<JSON::String>(); http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/tests/master_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/master_tests.cpp b/src/tests/master_tests.cpp index 06d74c3..a477794 100644 --- a/src/tests/master_tests.cpp +++ b/src/tests/master_tests.cpp @@ -2826,7 +2826,7 @@ TEST_F(MasterTest, StateEndpoint) ASSERT_TRUE(state.values["start_time"].is<JSON::Number>()); EXPECT_EQ( static_cast<int>(Clock::now().secs()), - static_cast<int>(state.values["start_time"].as<JSON::Number>().value)); + state.values["start_time"].as<JSON::Number>().as<int>()); ASSERT_TRUE(state.values["id"].is<JSON::String>()); EXPECT_NE("", state.values["id"].as<JSON::String>().value); http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/tests/monitor_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/monitor_tests.cpp b/src/tests/monitor_tests.cpp index f404955..583e711 100644 --- a/src/tests/monitor_tests.cpp +++ b/src/tests/monitor_tests.cpp @@ -106,44 +106,18 @@ TEST(MonitorTest, Statistics) "Content-Type", response); - // TODO(bmahler): Use JSON equality instead to avoid having to use - // numeric limits for double precision. - AWAIT_EXPECT_RESPONSE_BODY_EQ( - strings::format( - "[{" - "\"executor_id\":\"executor\"," - "\"executor_name\":\"name\"," - "\"framework_id\":\"framework\"," - "\"source\":\"source\"," - "\"statistics\":{" - "\"cpus_limit\":%g," - "\"cpus_nr_periods\":%d," - "\"cpus_nr_throttled\":%d," - "\"cpus_system_time_secs\":%g," - "\"cpus_throttled_time_secs\":%g," - "\"cpus_user_time_secs\":%g," - "\"mem_anon_bytes\":%lu," - "\"mem_file_bytes\":%lu," - "\"mem_limit_bytes\":%lu," - "\"mem_mapped_file_bytes\":%lu," - "\"mem_rss_bytes\":%lu," - "\"timestamp\":" - "%." + stringify(numeric_limits<double>::digits10) + "g" - "}" - "}]", - statistics.cpus_limit(), - statistics.cpus_nr_periods(), - statistics.cpus_nr_throttled(), - statistics.cpus_system_time_secs(), - statistics.cpus_throttled_time_secs(), - statistics.cpus_user_time_secs(), - statistics.mem_anon_bytes(), - statistics.mem_file_bytes(), - statistics.mem_limit_bytes(), - statistics.mem_mapped_file_bytes(), - statistics.mem_rss_bytes(), - statistics.timestamp()).get(), - response); + JSON::Array expected; + JSON::Object usage; + usage.values["executor_id"] = "executor"; + usage.values["executor_name"] = "name"; + usage.values["framework_id"] = "framework"; + usage.values["source"] = "source"; + usage.values["statistics"] = JSON::Protobuf(statistics); + expected.values.push_back(usage); + + Try<JSON::Array> result = JSON::parse<JSON::Array>(response.get().body); + ASSERT_SOME(result); + ASSERT_EQ(expected, result.get()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/tests/rate_limiting_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/rate_limiting_tests.cpp b/src/tests/rate_limiting_tests.cpp index f3aedde..e512aa6 100644 --- a/src/tests/rate_limiting_tests.cpp +++ b/src/tests/rate_limiting_tests.cpp @@ -166,12 +166,16 @@ TEST_F(RateLimitingTest, NoRateLimiting) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(1, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } Future<Nothing> removeFramework = @@ -270,12 +274,16 @@ TEST_F(RateLimitingTest, RateLimitingEnabled) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(1, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // The 2nd message is throttled for a second. @@ -305,8 +313,12 @@ TEST_F(RateLimitingTest, RateLimitingEnabled) // The 2nd message is received and but not processed after half // a second because of throttling. - EXPECT_EQ(2, metrics.values[messages_received].as<JSON::Number>().value); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 2, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); EXPECT_TRUE(duplicateFrameworkRegisteredMessage.isPending()); } @@ -324,8 +336,10 @@ TEST_F(RateLimitingTest, RateLimitingEnabled) "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(2, metrics.values[messages_received].as<JSON::Number>().value); - EXPECT_EQ(2, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 2, metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); + EXPECT_EQ( + 2, metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); EXPECT_EQ(DRIVER_STOPPED, driver.stop()); EXPECT_EQ(DRIVER_STOPPED, driver.join()); @@ -481,19 +495,19 @@ TEST_F(RateLimitingTest, DifferentPrincipalFrameworks) EXPECT_EQ( 2, metrics.values["frameworks/framework1/messages_received"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 2, metrics.values["frameworks/framework2/messages_received"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 1, metrics.values["frameworks/framework1/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 1, metrics.values["frameworks/framework2/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); } // Advance for a second so the message from framework1 (1qps) @@ -508,11 +522,11 @@ TEST_F(RateLimitingTest, DifferentPrincipalFrameworks) EXPECT_EQ( 2, metrics.values["frameworks/framework1/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 1, metrics.values["frameworks/framework2/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); // After another half a second framework2 (0.2qps)'s message is // processed as well. @@ -535,19 +549,19 @@ TEST_F(RateLimitingTest, DifferentPrincipalFrameworks) EXPECT_EQ( 2, metrics.values["frameworks/framework1/messages_received"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 2, metrics.values["frameworks/framework2/messages_received"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 2, metrics.values["frameworks/framework1/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); EXPECT_EQ( 2, metrics.values["frameworks/framework2/messages_processed"] - .as<JSON::Number>().value); + .as<JSON::Number>().as<int64_t>()); } // 3. Remove a framework and its message counters are deleted while @@ -705,12 +719,16 @@ TEST_F(RateLimitingTest, SamePrincipalFrameworks) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(2, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 2, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // Advance for another half a second to make sure throttled @@ -828,12 +846,16 @@ TEST_F(RateLimitingTest, SchedulerFailover) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(1, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // 2. Now launch the second (i.e., failover) scheduler using the @@ -898,12 +920,16 @@ TEST_F(RateLimitingTest, SchedulerFailover) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(2, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 2, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // Need another half a second to have it processed. @@ -922,12 +948,16 @@ TEST_F(RateLimitingTest, SchedulerFailover) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(2, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 2, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(2, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 2, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } EXPECT_EQ(DRIVER_STOPPED, driver2.stop()); @@ -1012,12 +1042,16 @@ TEST_F(RateLimitingTest, CapacityReached) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(1, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // The subsequent messages are going to be throttled. @@ -1064,12 +1098,16 @@ TEST_F(RateLimitingTest, CapacityReached) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(5, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 5, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); // Four messages not processed, two in the queue and two dropped. - EXPECT_EQ(1, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 1, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); } // Advance three times for the two pending messages and the exited @@ -1086,12 +1124,16 @@ TEST_F(RateLimitingTest, CapacityReached) const string& messages_received = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_received"; EXPECT_EQ(1u, metrics.values.count(messages_received)); - EXPECT_EQ(5, metrics.values[messages_received].as<JSON::Number>().value); + EXPECT_EQ( + 5, + metrics.values[messages_received].as<JSON::Number>().as<int64_t>()); const string& messages_processed = "frameworks/" + DEFAULT_CREDENTIAL.principal() + "/messages_processed"; EXPECT_EQ(1u, metrics.values.count(messages_processed)); // Two messages are dropped. - EXPECT_EQ(3, metrics.values[messages_processed].as<JSON::Number>().value); + EXPECT_EQ( + 3, + metrics.values[messages_processed].as<JSON::Number>().as<int64_t>()); Shutdown(); } http://git-wip-us.apache.org/repos/asf/mesos/blob/30d1bb7c/src/tests/slave_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/slave_tests.cpp b/src/tests/slave_tests.cpp index d158ccc..dccdbb0 100644 --- a/src/tests/slave_tests.cpp +++ b/src/tests/slave_tests.cpp @@ -1081,7 +1081,7 @@ TEST_F(SlaveTest, StateEndpoint) ASSERT_TRUE(state.values["start_time"].is<JSON::Number>()); EXPECT_EQ( static_cast<int>(Clock::now().secs()), - static_cast<int>(state.values["start_time"].as<JSON::Number>().value)); + state.values["start_time"].as<JSON::Number>().as<int>()); // TODO(bmahler): The slave must register for the 'id' // to be non-empty.
