mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 64494: Sent resource version uuid only for agent default resources.
Date Mon, 11 Dec 2017 12:38:22 GMT

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


Fix it, then Ship it!





src/master/master.cpp
Lines 6300 (patched)
<https://reviews.apache.org/r/64494/#comment271937>

    Let's init this with `None()` for consistency.



src/master/master.cpp
Lines 6974 (patched)
<https://reviews.apache.org/r/64494/#comment271938>

    Keep breaking on assignment like before?



src/messages/messages.proto
Line 517 (original), 517 (patched)
<https://reviews.apache.org/r/64494/#comment271939>

    `s/uuid/UUID/`



src/messages/messages.proto
Line 521 (original), 522 (patched)
<https://reviews.apache.org/r/64494/#comment271940>

    `s/  / /`



src/messages/messages.proto
Line 575 (original), 576 (patched)
<https://reviews.apache.org/r/64494/#comment271941>

    `s/Resoruce/Resource/`
    `s/uuid/UIID/`



src/messages/messages.proto
Line 579 (original), 581 (patched)
<https://reviews.apache.org/r/64494/#comment271942>

    `s/  / /`



src/slave/slave.cpp
Lines 1546 (patched)
<https://reviews.apache.org/r/64494/#comment271943>

    Could this be unconditional?



src/slave/slave.cpp
Lines 1562 (patched)
<https://reviews.apache.org/r/64494/#comment271944>

    Ditto.



src/tests/slave_tests.cpp
Lines 8967 (patched)
<https://reviews.apache.org/r/64494/#comment271945>

    This is not needed if we always send the `optional` agent resource version.


- Benjamin Bannier


On Dec. 11, 2017, 5:59 a.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64494/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2017, 5:59 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Greg Mann.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> It's not correct to send resource version uuids for local resources
> providers during agent re(registration) because the total resources from
> those local resource providers are not sent in the same message.
> 
> Consider the following sequence of events:
> (1) Agent disconnects
> (2) Speculative operation fails in an RP, the RP bumps the version uuid
> (3) Agent updates the RP’s resource version uuid
> (4) Agent reregisters
> (5) Master is informed about the new resource version uuid of that RP
> (6) Master still has the old total of the RP
> (7) Framework launch an operation assuming the old total, but with the
>     new resource version uuid
> 
> This patch updated the `RegisterSlaveMessage` and
> `ReregisterSlaveMessage` to only send resource version uuids for the
> agent default resources.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 5c26f2066aae31223ffdd76ed058d5a4e63a2603 
>   src/master/master.cpp b3e074cfe86600793310deb87932fa145e95055d 
>   src/messages/messages.proto a13a6410891a45876b4458369c282aa4d502db08 
>   src/slave/slave.cpp 373e393ca1e7c0c30c3474cc9e630e25ad92f235 
>   src/tests/slave_tests.cpp 0fb2a63a5c9e7bd353c4f1b8f8f32e58e3e12e94 
> 
> 
> Diff: https://reviews.apache.org/r/64494/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


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