mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 71639: Added an benchmark for `allocator->UpdateAllocation()`.
Date Wed, 30 Oct 2019 16:55:20 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71639/#review218460
-----------------------------------------------------------


Fix it, then Ship it!





src/tests/hierarchical_allocator_tests.cpp
Lines 8384-8393 (patched)
<https://reviews.apache.org/r/71639/#comment306191>

    Not sure that this is a right place for this comment, especially the part describing what
each reservation contains. After all, things stated in this comment are not a responsibility
of the `ResourceParam` struct. Rather, this is how `UpdateAllocation` behcnmark uses these
parameters (which is described in the comments below).



src/tests/hierarchical_allocator_tests.cpp
Lines 8408 (patched)
<https://reviews.apache.org/r/71639/#comment306189>

    Shouldn't that be "exactly", not "more than"?
    
    Also, "unique"/"distinct"/"random" would probably be a better word than "various" (IMO,
"various" does not indicate that all the individual `Resource`s have different metadata to
make them non-addable).



src/tests/hierarchical_allocator_tests.cpp
Lines 8425-8427 (patched)
<https://reviews.apache.org/r/71639/#comment306190>

    I would suggest slightly rewording this: "Each set of reservations contains some cpu,
memory, disk and port (number of ranges is controlled by `portRangeCount`) with a random label,
and is allocated to a framework."



src/tests/hierarchical_allocator_tests.cpp
Lines 8429 (patched)
<https://reviews.apache.org/r/71639/#comment306188>

    s/a "loop" framework/one of the frameworks/ ?



src/tests/hierarchical_allocator_tests.cpp
Lines 8510 (patched)
<https://reviews.apache.org/r/71639/#comment306187>

    Outdated comment? (There is no dedicated loop role now.)



src/tests/hierarchical_allocator_tests.cpp
Lines 8569 (patched)
<https://reviews.apache.org/r/71639/#comment306186>

    If we want to print agent resources size, shouldn't that be `param.reservationCount *
param.roleCount * RESOURCE_NAMES.size()` or `agentResources.size()`?
    
    Alternatively, words `Agent resources size` can be replaced by more precise `Total number
of reservations`.


- Andrei Sekretenko


On Oct. 29, 2019, 3:43 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71639/
> -----------------------------------------------------------
> 
> (Updated Oct. 29, 2019, 3:43 a.m.)
> 
> 
> Review request for mesos, Andrei Sekretenko and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10016
>     https://issues.apache.org/jira/browse/MESOS-10016
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This benchmark evaluates the performance of
> `allocator->UpdateAllocation()` where the agent has various
> sizes of resource reservations.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp 38fd19cee6409e4daa5bb0ab523e8e464cdcc9a5

> 
> 
> Diff: https://reviews.apache.org/r/71639/diff/3/
> 
> 
> Testing
> -------
> 
> On the master branch optimized build:
> 
> ```
> [ RUN      ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/0
> Agent resources size: 50 (50 roles, 1 reservations per role, 1 ranges per port resource)
> 20 RESERVE operations took 1.968569746secs, each takes 98.428487ms
> 20 UNRESERVE operations took 1.978470742secs, each takes 98.923537ms
> [       OK ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/0
(3996 ms)
> [ RUN      ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/1
> Agent resources size: 100 (100 roles, 1 reservations per role, 1 ranges per port resource)
> 20 RESERVE operations took 10.094084426secs, each takes 504.704221ms
> 20 UNRESERVE operations took 10.293614069secs, each takes 514.680703ms
> [       OK ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/1
(20584 ms)
> [ RUN      ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/2
> Agent resources size: 200 (200 roles, 1 reservations per role, 1 ranges per port resource)
> 20 RESERVE operations took 1.16018705933333mins, each takes 3.480561178secs
> 20 UNRESERVE operations took 1.17209781845mins, each takes 3.516293455secs
> [       OK ] ResourceParam/HierarchicalAllocator__BENCHMARK_WithResourceParam.UpdateAllocation/2
(141363 ms)
> ```
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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