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


Fix it, then Ship it!




This implements the same functionality as https://reviews.apache.org/r/64445/. 
Let's just update this one and drop the other.


src/master/master.hpp
Line 502 (original), 502 (patched)
<https://reviews.apache.org/r/64561/#comment272277>

    Let's not do this in this patch as it is an unrelated change. Instead we 
should clean up all handler at once.



src/master/master.cpp
Line 7221 (original), 7221 (patched)
<https://reviews.apache.org/r/64561/#comment272278>

    Ditto.



src/master/master.cpp
Line 7249 (original), 7249 (patched)
<https://reviews.apache.org/r/64561/#comment272279>

    If the agent did not specify *the* 'update_overs...



src/messages/messages.proto
Line 734 (original), 734 (patched)
<https://reviews.apache.org/r/64561/#comment272280>

    It looks like we introduced resource categories in `a88469bfe8` which is 
currently still only contained in `master` and was not part of a release.
    
    We can recycle field number 5.



src/slave/slave.hpp
Line 561 (original), 561 (patched)
<https://reviews.apache.org/r/64561/#comment272267>

    Not yours, but could you `s/resoruce/resource/`?



src/slave/slave.cpp
Line 6992 (original)
<https://reviews.apache.org/r/64561/#comment272281>

    One motivation for such helper functions can be that it becomes clearer 
than `UpdateSlaveMessage` should not be constructed manually due to possible 
complexity. By removing one of them this is not as apparent anymore.
    
    I'd suggest to keep this helper and revert changes at the callers so an 
up-to-date `oversubscribedResources` is found.


- Benjamin Bannier


On Dec. 13, 2017, 1:56 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64561/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2017, 1:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Given that now we use `UpdateSlaveMessage` to send resource provider
> information directly, having resource categories in the message is
> unnecessary and misleading.
> 
> Instead, this patch introduced a single optional boolean to indicate if
> oversubscribed resources need to be updated or not.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 232cc3758f240db626c4fdaf852163fa48af4dd7 
>   src/master/master.cpp efe8b8f1704b314e6e6a4d5632718cab2854e38f 
>   src/messages/messages.proto e680cd5e4d5a93c3c77309f327844f55fbb239a1 
>   src/slave/slave.hpp 7c40fc71b49057fea0cfd85290931fbd0f6a9d62 
>   src/slave/slave.cpp 5869e73ca1c14c99e580da9d7375181da2073ec5 
>   src/tests/oversubscription_tests.cpp 
> 3f57ce105e24e9f9cd681d8d984dbe242aa51f75 
> 
> 
> Diff: https://reviews.apache.org/r/64561/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>

Reply via email to