You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Bannier <bb...@apache.org> on 2017/09/28 06:42:22 UTC
Review Request 62655: Made 'UpdateSlaveMessage' a union of possible
updates.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62655/
-----------------------------------------------------------
Review request for mesos, Jie Yu and Jan Schlicht.
Repository: mesos
Description
-------
The 'UpdateSlaveMessage' could either transport an update to an
agent's total, or to its oversubscribed resources. In certain
scenarios this required the agent to send two messages where before
one was sufficient. When talking to a master which did not understand
the 'type' field this would have let the master to rescind all offers
for revocable resources from that agent.
This patch changes the message to be a union of possible updates,
i.e., it is now possible to send updates to both the total and the
oversubscribed resources simultaneously on agent registration and
reregistration without the master rescinding offers for revocable
resources on the agent.
Diffs
-----
src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20
src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd
src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50
src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca
Diff: https://reviews.apache.org/r/62655/diff/1/
Testing
-------
`make check`; also tested as part of https://reviews.apache.org/r/61183/.
Thanks,
Benjamin Bannier
Re: Review Request 62655: Made 'UpdateSlaveMessage' a union of
possible updates.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62655/#review186600
-----------------------------------------------------------
Fix it, then Ship it!
LGTM!
src/master/master.cpp
Lines 6765 (patched)
<https://reviews.apache.org/r/62655/#comment263288>
Add one line above.
- Jie Yu
On Sept. 28, 2017, 6:42 a.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62655/
> -----------------------------------------------------------
>
> (Updated Sept. 28, 2017, 6:42 a.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The 'UpdateSlaveMessage' could either transport an update to an
> agent's total, or to its oversubscribed resources. In certain
> scenarios this required the agent to send two messages where before
> one was sufficient. When talking to a master which did not understand
> the 'type' field this would have let the master to rescind all offers
> for revocable resources from that agent.
>
> This patch changes the message to be a union of possible updates,
> i.e., it is now possible to send updates to both the total and the
> oversubscribed resources simultaneously on agent registration and
> reregistration without the master rescinding offers for revocable
> resources on the agent.
>
>
> Diffs
> -----
>
> src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20
> src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd
> src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50
> src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca
>
>
> Diff: https://reviews.apache.org/r/62655/diff/1/
>
>
> Testing
> -------
>
> `make check`; also tested as part of https://reviews.apache.org/r/61183/.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 62655: Made 'UpdateSlaveMessage' a union of
possible updates.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62655/#review186982
-----------------------------------------------------------
Fix it, then Ship it!
src/master/master.cpp
Lines 6841-6843 (patched)
<https://reviews.apache.org/r/62655/#comment263934>
IIUC, this assumes that `total_resources` in the `UpdateSlaveMessage` only contains non revocable resources? If that's the case, can you add a comment to the message protobuf?
- Jie Yu
On Oct. 2, 2017, 4:53 p.m., Benjamin Bannier wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62655/
> -----------------------------------------------------------
>
> (Updated Oct. 2, 2017, 4:53 p.m.)
>
>
> Review request for mesos, Jie Yu and Jan Schlicht.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The 'UpdateSlaveMessage' could either transport an update to an
> agent's total, or to its oversubscribed resources. In certain
> scenarios this required the agent to send two messages where before
> one was sufficient. When talking to a master which did not understand
> the 'type' field this would have let the master to rescind all offers
> for revocable resources from that agent.
>
> This patch changes the message to be a union of possible updates,
> i.e., it is now possible to send updates to both the total and the
> oversubscribed resources simultaneously on agent registration and
> reregistration without the master rescinding offers for revocable
> resources on the agent.
>
>
> Diffs
> -----
>
> src/master/master.cpp b40621ee3870b1968ab1b05dd0f41a489f4db00e
> src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd
> src/slave/slave.cpp bf85baf8b7f7ebf52afa25214e71278f18dc9b30
> src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca
>
>
> Diff: https://reviews.apache.org/r/62655/diff/3/
>
>
> Testing
> -------
>
> `make check`; also tested as part of https://reviews.apache.org/r/61183/.
>
>
> Thanks,
>
> Benjamin Bannier
>
>
Re: Review Request 62655: Made 'UpdateSlaveMessage' a union of
possible updates.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62655/
-----------------------------------------------------------
(Updated Oct. 2, 2017, 6:53 p.m.)
Review request for mesos, Jie Yu and Jan Schlicht.
Changes
-------
Changed handling of updates.
If only a `total` is sent we now preserve the previous `oversubscribed`.
Repository: mesos
Description
-------
The 'UpdateSlaveMessage' could either transport an update to an
agent's total, or to its oversubscribed resources. In certain
scenarios this required the agent to send two messages where before
one was sufficient. When talking to a master which did not understand
the 'type' field this would have let the master to rescind all offers
for revocable resources from that agent.
This patch changes the message to be a union of possible updates,
i.e., it is now possible to send updates to both the total and the
oversubscribed resources simultaneously on agent registration and
reregistration without the master rescinding offers for revocable
resources on the agent.
Diffs (updated)
-----
src/master/master.cpp b40621ee3870b1968ab1b05dd0f41a489f4db00e
src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd
src/slave/slave.cpp bf85baf8b7f7ebf52afa25214e71278f18dc9b30
src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca
Diff: https://reviews.apache.org/r/62655/diff/3/
Changes: https://reviews.apache.org/r/62655/diff/2-3/
Testing
-------
`make check`; also tested as part of https://reviews.apache.org/r/61183/.
Thanks,
Benjamin Bannier
Re: Review Request 62655: Made 'UpdateSlaveMessage' a union of
possible updates.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62655/
-----------------------------------------------------------
(Updated Sept. 29, 2017, 11:14 a.m.)
Review request for mesos, Jie Yu and Jan Schlicht.
Changes
-------
Added an empty line.
Repository: mesos
Description
-------
The 'UpdateSlaveMessage' could either transport an update to an
agent's total, or to its oversubscribed resources. In certain
scenarios this required the agent to send two messages where before
one was sufficient. When talking to a master which did not understand
the 'type' field this would have let the master to rescind all offers
for revocable resources from that agent.
This patch changes the message to be a union of possible updates,
i.e., it is now possible to send updates to both the total and the
oversubscribed resources simultaneously on agent registration and
reregistration without the master rescinding offers for revocable
resources on the agent.
Diffs (updated)
-----
src/master/master.cpp 6d84a26bff970b842b58dfb69dbf232ba5c16a20
src/messages/messages.proto dc4e19c08d671743f08223dcdf1dbc336b3940bd
src/slave/slave.cpp 75e2e25c438637e2b1731f9fd01b6fe297687c50
src/tests/oversubscription_tests.cpp 02b10d6689dd6f01510cd2d5db2bb76b4b190eca
Diff: https://reviews.apache.org/r/62655/diff/2/
Changes: https://reviews.apache.org/r/62655/diff/1-2/
Testing
-------
`make check`; also tested as part of https://reviews.apache.org/r/61183/.
Thanks,
Benjamin Bannier