-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53840/#review175817
-----------------------------------------------------------
Ship it!
I'll commit with these adjustments.
docs/monitoring.md
Lines 1053 (patched)
<https://reviews.apache.org/r/53840/#comment249208>
s/algorithm/batch/
docs/monitoring.md
Lines 1053 (patched)
<https://reviews.apache.org/r/53840/#comment249210>
s/algorithm/batch/
docs/monitoring.md
Lines 1058 (patched)
<https://reviews.apache.org/r/53840/#comment249209>
Having all these lines for each timer feels like too much repetition but alas, it's the
pattern right now and let's just maintain consistency and clean up later.
src/tests/hierarchical_allocator_tests.cpp
Lines 3578-3581 (original), 3578-3581 (patched)
<https://reviews.apache.org/r/53840/#comment249149>
I kept the comment but do see the need to move the assignment.
src/tests/hierarchical_allocator_tests.cpp
Lines 3673 (patched)
<https://reviews.apache.org/r/53840/#comment249205>
I simplified the test a bit. Until an abstraction is provided, tests tend to be copied
so let's keep things as simple as possible.
```
TEST_F_TEMP_DISABLED_ON_WINDOWS(
HierarchicalAllocatorTest,
AllocationRunLatencyMetrics)
{
Clock::pause();
initialize();
// These time series statistics will be generated
// once at least 2 allocation runs occur.
auto statistics = {
"allocator/mesos/allocation_run_latency_ms/count",
"allocator/mesos/allocation_run_latency_ms/min",
"allocator/mesos/allocation_run_latency_ms/max",
"allocator/mesos/allocation_run_latency_ms/p50",
"allocator/mesos/allocation_run_latency_ms/p95",
"allocator/mesos/allocation_run_latency_ms/p99",
"allocator/mesos/allocation_run_latency_ms/p999",
"allocator/mesos/allocation_run_latency_ms/p9999",
};
JSON::Object metrics = Metrics();
map<string, JSON::Value> values = metrics.values;
EXPECT_EQ(0u, values.count("allocator/mesos/allocation_run_latency_ms"));
// No allocation latency statistics should appear.
foreach (const string& statistic, statistics) {
EXPECT_EQ(0u, values.count(statistic))
<< "Expected " << statistic << " to be absent";
}
// Allow the allocation timer to measure time.
Clock::resume();
// Trigger at least two calls to allocate to generate the window statistics.
SlaveInfo agent = createSlaveInfo("cpus:2;mem:1024;disk:0");
allocator->addSlave(
agent.id(),
agent,
AGENT_CAPABILITIES(),
None(),
agent.resources(),
{});
// Wait for the allocation triggered by `addSlave()` to complete.
Clock::pause();
Clock::settle();
Clock::resume();
FrameworkInfo framework = createFrameworkInfo({"role1"});
allocator->addFramework(framework.id(), framework, {}, true);
// Wait for the allocation triggered by `addFramework()` to complete.
AWAIT_READY(allocations.get());
metrics = Metrics();
values = metrics.values;
// A non-zero measurement should be present.
EXPECT_EQ(1u, values.count("allocator/mesos/allocation_run_latency_ms"));
JSON::Value value =
metrics.values["allocator/mesos/allocation_run_latency_ms"];
ASSERT_TRUE(value.is<JSON::Number>()) << value.which();
JSON::Number timing = value.as<JSON::Number>();
ASSERT_EQ(JSON::Number::FLOATING, timing.type);
EXPECT_GE(timing.as<double>(), 0.0);
// The statistics should be generated.
foreach (const string& statistic, statistics) {
EXPECT_EQ(1u, values.count(statistic))
<< "Expected " << statistic << " to be present";
}
}
```
- Jiang Yan Xu
On May 23, 2017, 9:30 a.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53840/
> -----------------------------------------------------------
>
> (Updated May 23, 2017, 9:30 a.m.)
>
>
> Review request for mesos, James Peach and Jiang Yan Xu.
>
>
> Bugs: MESOS-6579
> https://issues.apache.org/jira/browse/MESOS-6579
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added a metric 'allocator/mesos/allocation_run_interval_ms' which
> tracks the latency between scheduling an allocation to an actual
> allocation run.
>
>
> Diffs
> -----
>
> docs/monitoring.md a027f4905a0e6e41ff4e1348d34fd7aa5f1cbe61
> src/master/allocator/mesos/hierarchical.cpp b75ed9a20a9a42f958cebbacd91e5e15b0d21394
> src/master/allocator/mesos/metrics.hpp ce486eb079dc44bf0bbfad8668426c7ae87c97b3
> src/master/allocator/mesos/metrics.cpp ec987fe0ce8605bf9e7c0ccbe3ac384deaf53be1
> src/tests/hierarchical_allocator_tests.cpp 6dee2296d5a14185dbf7eee17968b20148839bfd
>
>
> Diff: https://reviews.apache.org/r/53840/diff/7/
>
>
> Testing
> -------
>
> All tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
|