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 59677: Added '--filter_gpu_resources' flag to the mesos master.
Date Wed, 31 May 2017 18:58:18 GMT

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


Fix it, then Ship it!





docs/configuration.md
Lines 637-643 (patched)
<https://reviews.apache.org/r/59677/#comment249868>

    We should clarify that it's (offers (from agents with GPU resources)) rather than (offers
with GPU resources).
    
    E.g. to filter offers from GPU agents



docs/configuration.md
Lines 644 (patched)
<https://reviews.apache.org/r/59677/#comment249870>

    Do we also have the ticket to point to?



src/master/flags.cpp
Lines 465-474 (patched)
<https://reviews.apache.org/r/59677/#comment249869>

    Ditto here about this filtering all offers on GPU agents rather than only those offers
with the GPUs.



src/master/flags.cpp
Lines 474 (patched)
<https://reviews.apache.org/r/59677/#comment249871>

    Ditto here w.r.t the ticket.



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
Lines 365-367 (patched)
<https://reviews.apache.org/r/59677/#comment249876>

    Do we have already have a test for the positive case where we do expect filtering?
    
    If not, would be great to have one.
    
    Also, if you want easier cherry-picking, you can pull this test into a separate patch,
and have yet another one for the positive filter test case.



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
Lines 365-366 (patched)
<https://reviews.apache.org/r/59677/#comment249879>

    How about: This test ensures that a scheduler can be allocated offers from GPU agents
without the GPU_RESOURCES framework capability, when the ...



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
Lines 368 (patched)
<https://reviews.apache.org/r/59677/#comment249877>

    Maybe just s/Verify// since verifying something is implicitly what each test is doing?



src/tests/containerizer/nvidia_gpu_isolator_tests.cpp
Lines 390-393 (patched)
<https://reviews.apache.org/r/59677/#comment249878>

    You may want to assert that the capability isn't there rather than assuming so.


- Benjamin Mahler


On May 31, 2017, 2:47 a.m., Kevin Klues wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59677/
> -----------------------------------------------------------
> 
> (Updated May 31, 2017, 2:47 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7576
>     https://issues.apache.org/jira/browse/MESOS-7576
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When set to true, this flag will cause the mesos master to filter
> offers containing GPU resources by only sending them to frameworks
> that opt into the 'GPU_RESOURCES' framework capability. When set to
> false, this flag will cause the master to not filter offers containing
> GPU resources, and indiscriminately send them to all frameworks
> whether they set the 'GPU_RESOURCES' capability or not.  This flag is
> meant as a temporary workaround towards the eventual deprecation of
> the 'GPU_RESOURCES' capability.
> 
> Please see the following link for more information:
> https://www.mail-archive.com/dev@mesos.apache.org/msg37571.html
> 
> 
> Diffs
> -----
> 
>   docs/configuration.md 8c3be230b789d0a56f74c0e015f73492efb65603 
>   include/mesos/allocator/allocator.hpp dc34a1b6f0c2bdf0d653bd53394364cfa9873ca7 
>   src/master/allocator/mesos/allocator.hpp 119b461136123229f85ed3c3cfd41137974a6b9b 
>   src/master/allocator/mesos/hierarchical.hpp 82fea40ad09c95f0096934c4210a39a6f0638ed9

>   src/master/allocator/mesos/hierarchical.cpp ca368bb5ef0dcfe83672004d58a4de6f85d6b4bc

>   src/master/flags.hpp 93ca9b9348732a9219440dd9196c73d93c181120 
>   src/master/flags.cpp b1c0886977292afe938a4eb36193b224931b0eb2 
>   src/master/master.cpp 14007e08f509446005423e223d5dd76a70744e27 
>   src/tests/allocator.hpp 4ea722491f63f5ceda5a47228aafddc020f643b0 
>   src/tests/api_tests.cpp 97a8cc9714f2ee64c1398a9961d96cd40edc3a75 
>   src/tests/containerizer/nvidia_gpu_isolator_tests.cpp 9a78ae65c1cd414b5093b54ff51724e31e31c9d3

>   src/tests/master_allocator_tests.cpp b9e04226a1688499847bc25c7c220c0b82ba8db7 
>   src/tests/master_quota_tests.cpp 2c7ec5ac9e585c1ab52bc3771e215e5ee30abb3a 
>   src/tests/persistent_volume_endpoints_tests.cpp 21136b29d7eeee24959527b889f52611baee0668

>   src/tests/reservation_endpoints_tests.cpp 8c195eb7d788fbca8722d66d81c77d399702160a

>   src/tests/reservation_tests.cpp 47ccf7f6f6ee48236f6794ce3084ce4f425c93c5 
>   src/tests/resource_offers_tests.cpp f0bca1d9e03013ce35215b0ffa6b50b38972dc0c 
>   src/tests/slave_recovery_tests.cpp df0c5c88786190be06df7ef3602834aa8985cefe 
> 
> 
> Diff: https://reviews.apache.org/r/59677/diff/1/
> 
> 
> Testing
> -------
> 
> $ GTEST_FILTER="" make -j check
> $ sudo GTEST_FILTER="*GPUFilterDisabled*" src/mesos-tests
> ```
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from NvidiaGpuTest
> [ RUN      ] NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyGPUFilterDisabled
> Executing pre-exec command '{"arguments":["mesos-containerizer","mount","--help=false","--operation=make-rslave","--path=\/"],"shell":false,"value":"\/home\/klueska\/projects\/mesos\/build\/src\/mesos-containerizer"}'
> I0530 19:45:29.213250 33200 exec.cpp:162] Version: 1.4.0
> I0530 19:45:29.228093 33183 exec.cpp:237] Executor registered on agent 5c94c185-8f83-4e70-af3e-2bebcb49a310-S0
> Received SUBSCRIBED event
> Subscribed executor on core-dev
> Received LAUNCH event
> Starting task dabc46f6-2866-474b-a1f5-3419a7b5927d
> Running '/home/klueska/projects/mesos/build/src/mesos-containerizer launch <POSSIBLY-SENSITIVE-DATA>'
> Forked command at 33228
> Command exited with status 0 (pid: 33228)
> I0530 19:45:29.424254 33195 exec.cpp:435] Executor asked to shutdown
> Received SHUTDOWN event
> Shutting down
> [       OK ] NvidiaGpuTest.ROOT_CGROUPS_NVIDIA_GPU_VerifyGPUFilterDisabled (9111 ms)
> [----------] 1 test from NvidiaGpuTest (9113 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (9146 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> 
> Thanks,
> 
> Kevin Klues
> 
>


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