mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Kevin Klues <klue...@gmail.com>
Subject Re: Review Request 48914: Added GPU_RESOURCES capability to FrameworkInfo.
Date Sun, 19 Jun 2016 02:49:13 GMT


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1274
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423480#file1423480line1274>
> >
> >     s/slaves/agents

I thought about this as I was writing it, but I left it as slave because all of the other
comments in the file use slave. I figured it was better to stay consistent and wait for a
full pass to change it than have one coment refering to agent and the next referring to slave.


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 261-263
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423480#file1423480line261>
> >
> >     Here should be `if` but not `else if` as one framework can have multiple capabilities.
> >     
> >     if (capability.type() == FrameworkInfo::Capability::GPU_RESOURCES) {
> >       frameworks[frameworkId].gpuCapable = true;
> >     }

I took a look at it again, but I still think it should be `else if`. We loop through each
capability, and each of them can only have a single type. They can only match one type or
another type.


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 403-405
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423480#file1423480line403>
> >
> >     ditto

ditto


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1277-1279
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423480#file1423480line1277>
> >
> >     This logic seems also allow agents without gpu resources offered to the framework
with gpuCapble, is it expected behavior?

Yes, that was my intended behaviour as I wrote it. If we think there is some problem with
this, we should discuss it further.


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/tests/containerizer/nvidia_gpu_isolator_tests.cpp, lines 94-97
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423481#file1423481line94>
> >
> >     Only updating here may be not enough, I think that we should have at least two
agents for such test case, one is with gpu and the other without gpi resources.
> >     Or else you may want to add a new test case with a gpu host but a framework
without gpuCapble, and the framework will not get resources.

I am still puttting together a full set of tests for this. They will be submitted in a follow-on
patch. This was just a fix to the tests currently in the code base so they would continue
to pass.


> On June 19, 2016, 2:18 a.m., Guangya Liu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, line 1412
> > <https://reviews.apache.org/r/48914/diff/1/?file=1423480#file1423480line1412>
> >
> >     s/slaves/agents

Again, I left is as slave for consistency with the other comments in this file.


- Kevin


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


On June 18, 2016, 10:07 p.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48914/
> -----------------------------------------------------------
> 
> (Updated June 18, 2016, 10:07 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-5634
>     https://issues.apache.org/jira/browse/MESOS-5634
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Due to the scarce resource problem described in MESOS-5377, we are
> introducing a GPU_RESOURCES Framework capability. This capability
> allows the Mesos allocator to make better decisions about which
> frameworks should receive resources from GPU capable machines. In
> essence, the allocator ONLY allocate resources from GPU capable
> machines to frameworks that have this capability. This is necessary to
> prevent non-GPU workloads from filling up the GPU machines and
> preventing GPU workloads to run.
> 
> 
> Diffs
> -----
> 
>   include/mesos/mesos.proto e4c5bd31cf035707036eb509336fe051119b4e78 
>   include/mesos/v1/mesos.proto 9be22f02861f1eb89ab547d88530faf90ebee7ab 
>   src/master/allocator/mesos/hierarchical.hpp 9c6b23abe2b0cb16412f1ed90165f8d0c14552fa

>   src/master/allocator/mesos/hierarchical.cpp 8b7b3afb5770c617918ec4864faaf8d8a7a81ef2

>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp e06d107f2dcdb9b470e330c8ceee66a54220d41b

> 
> Diff: https://reviews.apache.org/r/48914/diff/
> 
> 
> Testing
> -------
> 
> $ make -j check; sudo GTEST_FILTER="*NVIDIA*" src/mesos-tests
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


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