mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 69942: Added a test for MESOS-9554.
Date Tue, 12 Feb 2019 19:23:28 GMT


> On Feb. 12, 2019, 6:34 p.m., Benjamin Bannier wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 2924-2929 (patched)
> > <https://reviews.apache.org/r/69942/diff/1/?file=2124739#file2124739line2924>
> >
> >     This test actually is more than a regression test for MESOS-9554 as it validates
to some extent that framework allocation decisions are independent (but for possibly decreasing
the available resource pool).
> >     
> >     Maybe rephrase along the lines of
> >     ```
> >     This test validates with one case that framework allocation decisions are independent
(but for possibly decreasing the available resource pool). In particular, we validate that
the decision not to allocate resources of an agent to a particular framework does not influence
to allocate the same resources to another agent. In this test we make agent resources not
allocatable to a framework by refining the resource reservations while the framework which
should be allocated these resources based on DRF share is not capable of receiving them (it
lacks the `RESERVATION_REFINEMENT` capability).
> >     
> >     This is a regression test for MESOS-9554.
> >     ```
> >     
> >     We could also rename the test, e.g., to `HierarchicalAllocatorTest.AllocationIndependent`
(doesn't read to well, either). This tests models a whole class of scenarios we currently
don't validate.

Thanks for the suggestion. I will have to think on this, and so I'll leave it open for now


> On Feb. 12, 2019, 6:34 p.m., Benjamin Bannier wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 2984-2990 (patched)
> > <https://reviews.apache.org/r/69942/diff/1/?file=2124739#file2124739line2984>
> >
> >     I don't think using a single statement here reads much worse,
> >     ```
> >     Resources resources =?
> >       CHECK_NOTERROR(Resources::parse("cpus:1;mem:1;disk:1"))?
> >         .pushReservation(createDynamicReservationInfo("parent"))?
> >         .pushReservation(createDynamicReservationInfo("parent/child"));?
> >     ```

Looks good, thanks


- Benjamin


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


On Feb. 11, 2019, 6:21 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69942/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2019, 6:21 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Meng Zhu.
> 
> 
> Bugs: MESOS-9554
>     https://issues.apache.org/jira/browse/MESOS-9554
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This test ensures that the framework allocation loop does not
> incorrectly break in the case that a framework is incapable
> of receiving the resources on an agent. In this case, the loop
> should continue as other frameworks may be capable of receiving
> the resources.
> 
> 
> Diffs
> -----
> 
>   src/tests/hierarchical_allocator_tests.cpp cc88afbad1b4e6bf707cb13b50c964aa01f9a3ee

> 
> 
> Diff: https://reviews.apache.org/r/69942/diff/1/
> 
> 
> Testing
> -------
> 
> Ensured it fails on master, succeeds after fix to MESOS-9554.
> 
> Ran in repetition.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>


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