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




src/slave/slave.cpp
Line 3737 (original), 3737 (patched)
<https://reviews.apache.org/r/63798/#comment268716>

    Can you update `needCheckpointing` to just look at agent default resources?



src/slave/slave.cpp
Lines 3737-3749 (original), 3737-3742 (patched)
<https://reviews.apache.org/r/63798/#comment268720>

    As I mentioned earlier, `chckpointResources` will overwrite 
`totalResources`, which is wrong with resource provider resources.



src/tests/resource_provider_manager_tests.cpp
Lines 810-811 (original), 833-834 (patched)
<https://reviews.apache.org/r/63798/#comment268723>

    This does not sound like a valid sequence. I'm surprised that we allow 
CREATE on a RAW disk resources. Can you add some validation to disallow that?
    
    Another suggestion is that we should add some default action to 
MockResourceProvider so that we can reduce the size of this test. The default 
behavior for handling an operation is to just return the converted resources 
(for new operations), and success for old operations. One can override the 
default behavior. Take a look at `TestAllocator`


- Jie Yu


On Nov. 15, 2017, 11:35 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63798/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2017, 11:35 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8218
>     https://issues.apache.org/jira/browse/MESOS-8218
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The `RESERVE`/`UNRESERVE`/`CREATE`/`DESTROY` operations for
> reservations and persistent volumes have been enabled for resource
> provider resources. Similar to agent resources, they are speculatively
> applied to allow offer pipelining.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp d8edc5e6bbfa265bca4d19bbaa7db3063949dbc0 
>   src/tests/resource_provider_manager_tests.cpp 
> ecfe2b4c0952838d6312df603f8eb2f458725175 
> 
> 
> Diff: https://reviews.apache.org/r/63798/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>

Reply via email to