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 67777: Added a helper to match agent-framework capabilities in the allocator.
Date Thu, 05 Jul 2018 21:27:58 GMT


> On July 2, 2018, 10:42 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Line 1756 (original), 1755 (patched)
> > <https://reviews.apache.org/r/67777/diff/2/?file=2047194#file2047194line1756>
> >
> >     How about a member function of the framework struct?
> >     
> >     ```
> >     if (!framework.isCapableOnAgent(slave)) {
> >       ...
> >     }
> >     ```
> 
> Meng Zhu wrote:
>     That would need to move nested Slave class forward or outside. Currently, all the
helper functions for slave/framework are member functions of the outer class. I was trying
to be consistent. If you think it is necessary, I can go ahead and overhaul the structure.
> 
> Meng Zhu wrote:
>     I could forward declare Slave in the outer class. Yet it would still depend on `filterGpuResources`
of the outer class. Need to either cache a reference or pass in the flag. Feel cleaner this
way.

Which functions are you referring to? I only see isRemoteSlave and isWhitelisted that look
like they should be members of Slave?

Yeah I think the ideal would be to pull these structs out of being nested so that the logic
can be more clearly divided between them and the allocator, like we did in master.hpp for
Slave and Framework structs. We can just put the TODOs in this patch and follow up separately
for that, can you mark as fixed by adding TODOs?


- Benjamin


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


On July 4, 2018, 12:19 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67777/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 12:19 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> `isFrameworkCapableReceivingAgent` checks if a framework
> is capable of receiving resources on the agent based on
> the framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26

>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b

> 
> 
> Diff: https://reviews.apache.org/r/67777/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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