> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> >

Thanks!


> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 667-676
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line667>
> >
> >     We have a bunch of places where this condition needs to hold true I 
> > think. Is this location more special than others for some reason?
> >     
> >     If we're not going to pull this out to a validation of some kind, let's 
> > at least leave a `TODO` here.

I'll go with this:

```
  hashmap<string, Resources> allocations = offeredResources.allocations();

  CHECK_EQ(1u, allocations.size());

  string role = allocations.begin()->first;
```

And put a TODO that it is inducing unnecessary copying of Resources.


> On Jan. 31, 2017, 5:25 a.m., Michael Park wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 436
> > <https://reviews.apache.org/r/55870/diff/1/?file=1615405#file1615405line436>
> >
> >     Does `CHECK_EQ` not work here?

No unfortunately not, since we only have a `stringify()` definition for 
`std::set`. We don't have an `operator<<` defined.


- Benjamin


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


On Jan. 25, 2017, 9:55 p.m., Benjamin Mahler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55870/
> -----------------------------------------------------------
> 
> (Updated Jan. 25, 2017, 9:55 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Jay Guo, Guangya Liu, and Michael 
> Park.
> 
> 
> Bugs: MESOS-6635
>     https://issues.apache.org/jira/browse/MESOS-6635
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The allocator now sets `Resource.AllocationInfo` when performing
> an allocation, and the interface is updated to expose allocations
> for each role within a framework.
> 
> The allocator also assumes that `Resource.AllocationInfo` is set
> for inbound resources that are allocated (e.g. when adding an agent,
> when adding a framework, when recovering resources). Note however,
> that the necessary changes to the master and agent to enforce this
> will be done via separate patches.
> 
> 
> Diffs
> -----
> 
>   include/mesos/allocator/allocator.hpp 
> 558594983beb6ff49c1fddf875ba29c1983b1288 
>   src/master/allocator/mesos/allocator.hpp 
> 8e0f37a5eedd4d71d765991c0039b49c96f0ca53 
>   src/master/allocator/mesos/hierarchical.hpp 
> 9b66c23f26b37c02ed850c06c4518ea99078b02d 
>   src/master/allocator/mesos/hierarchical.cpp 
> c2211be7458755aeb91ef078e4bfe92ac474044a 
>   src/master/master.hpp 8e8a9037af11cf95961b6498540a0fd486ed091b 
>   src/master/master.cpp 73159328ce3fd838e02eba0e6a30cf69efc319ba 
>   src/tests/allocator.hpp 1f9261d1e7afa027bebc07915ba3e871a58e6558 
> 
> Diff: https://reviews.apache.org/r/55870/diff/
> 
> 
> Testing
> -------
> 
> Adjustments to existing tests are split out into a subsequent patch.
> 
> New tests for frameworks having multiple roles will be added as a follow up.
> 
> 
> Thanks,
> 
> Benjamin Mahler
> 
>

Reply via email to