mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jie Yu <yujie....@gmail.com>
Subject Re: Review Request 64494: Sent resource version uuid only for agent default resources.
Date Mon, 11 Dec 2017 20:09:22 GMT


> On Dec. 11, 2017, 12:38 p.m., Benjamin Bannier wrote:
> > src/master/master.cpp
> > Lines 6300 (patched)
> > <https://reviews.apache.org/r/64494/diff/1/?file=1912395#file1912395line6300>
> >
> >     Let's init this with `None()` for consistency.

oh? I never initialize an Option to None() explicitly in other places as the default executor
will initialize it to None.


> On Dec. 11, 2017, 12:38 p.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Line 521 (original), 522 (patched)
> > <https://reviews.apache.org/r/64494/diff/1/?file=1912396#file1912396line522>
> >
> >     `s/  / /`

What's this?


> On Dec. 11, 2017, 12:38 p.m., Benjamin Bannier wrote:
> > src/slave/slave.cpp
> > Lines 1546 (patched)
> > <https://reviews.apache.org/r/64494/diff/1/?file=1912397#file1912397line1555>
> >
> >     Could this be unconditional?

Yeah, this is probably fine. The reason I did that is because i want to mimic the old agent
behavior when the capability is not set so that we can validate in the master that if resource
version uuid is set, it has resource provider capability. But if we don't have that check,
it's ok to always set it.


- Jie


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


On Dec. 11, 2017, 4: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, 4: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