You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2018/01/27 01:40:21 UTC
Review Request 65368: Updated maintenance schedule logic.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65368/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-7882
https://issues.apache.org/jira/browse/MESOS-7882
Repository: mesos
Description
-------
This moves most of the conditional logic of maintenance schedules
into the master's `updateUnavailability` method, while the HTTP
handler will only loop through all MachineIDs once.
The `updateUnavailability` method will now check if the given
Unavailability has changed before it updates the allocator. This will
prevent a maintenance schedule from rescinding offers unrelated to the
maintenance schedule.
Diffs
-----
src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990
src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7
Diff: https://reviews.apache.org/r/65368/diff/1/
Testing
-------
make check
Thanks,
Joseph Wu
Re: Review Request 65368: Updated maintenance schedule logic.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65368/#review196404
-----------------------------------------------------------
It would be helpful for posterity if the commit message is tailored towards the behavioral change we made, e.g.:
```
Only rescind offers from affected agents in maintenance API.
```
I held off on a ship it because I was left confused about when agents' machines are stored in `machines`
src/master/http.cpp
Line 4460 (original), 4473 (patched)
<https://reviews.apache.org/r/65368/#comment276024>
Not yours, but seems odd to copy the whole map rather than just the keys?
src/master/http.cpp
Lines 4502-4504 (original), 4481-4483 (patched)
<https://reviews.apache.org/r/65368/#comment276025>
Not yours, but the mutation of master variables directly from the http handler seems a little odd, I would have expected the master to have a function for replacing the maintenance schedule, so that the http handler can just call it.
src/master/master.cpp
Lines 7749-7750 (patched)
<https://reviews.apache.org/r/65368/#comment276026>
Not yours, but the `machines` variable and registry field could use some documentation, the invariant here seems to be that the master only stores non-UP machines? I'm still left confused about this after reading the code.
src/master/master.cpp
Lines 7761-7763 (patched)
<https://reviews.apache.org/r/65368/#comment276027>
Hm.. the invariant is that each agent will always have a corresponding machine stored within `machines`? Is that the same invariant as the registry `machines`? A little puzzling
src/master/master.cpp
Lines 7769 (patched)
<https://reviews.apache.org/r/65368/#comment276028>
Could you use .at where you're not intending to insert?
- Benjamin Mahler
On Jan. 27, 2018, 1:40 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65368/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2018, 1:40 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7882
> https://issues.apache.org/jira/browse/MESOS-7882
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This moves most of the conditional logic of maintenance schedules
> into the master's `updateUnavailability` method, while the HTTP
> handler will only loop through all MachineIDs once.
>
> The `updateUnavailability` method will now check if the given
> Unavailability has changed before it updates the allocator. This will
> prevent a maintenance schedule from rescinding offers unrelated to the
> maintenance schedule.
>
>
> Diffs
> -----
>
> src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990
> src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7
>
>
> Diff: https://reviews.apache.org/r/65368/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 65368: Updated maintenance schedule logic.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65368/#review196386
-----------------------------------------------------------
PASS: Mesos patch 65368 was successfully built and tested.
Reviews applied: `['65366', '65367', '65368']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65368
- Mesos Reviewbot Windows
On Jan. 27, 2018, 1:40 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65368/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2018, 1:40 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7882
> https://issues.apache.org/jira/browse/MESOS-7882
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This moves most of the conditional logic of maintenance schedules
> into the master's `updateUnavailability` method, while the HTTP
> handler will only loop through all MachineIDs once.
>
> The `updateUnavailability` method will now check if the given
> Unavailability has changed before it updates the allocator. This will
> prevent a maintenance schedule from rescinding offers unrelated to the
> maintenance schedule.
>
>
> Diffs
> -----
>
> src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990
> src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7
>
>
> Diff: https://reviews.apache.org/r/65368/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 65368: Updated maintenance schedule logic.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/65368/#review196384
-----------------------------------------------------------
PASS: Mesos patch 65368 was successfully built and tested.
Reviews applied: `['65366', '65367', '65368']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/65368
- Mesos Reviewbot Windows
On Jan. 27, 2018, 1:40 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65368/
> -----------------------------------------------------------
>
> (Updated Jan. 27, 2018, 1:40 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-7882
> https://issues.apache.org/jira/browse/MESOS-7882
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This moves most of the conditional logic of maintenance schedules
> into the master's `updateUnavailability` method, while the HTTP
> handler will only loop through all MachineIDs once.
>
> The `updateUnavailability` method will now check if the given
> Unavailability has changed before it updates the allocator. This will
> prevent a maintenance schedule from rescinding offers unrelated to the
> maintenance schedule.
>
>
> Diffs
> -----
>
> src/master/http.cpp c489b6f525f157811549b2cc84addd9d85e87990
> src/master/master.cpp b97ebae6ebfd8ae0f73e617d0c55e140b9c3fce7
>
>
> Diff: https://reviews.apache.org/r/65368/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Joseph Wu
>
>