> 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 > >
