mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jiang Yan Xu <...@jxu.me>
Subject Re: Review Request 53840: Metric in the allocator to track latency in running allocations.
Date Tue, 23 May 2017 23:56:43 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message