mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 65368: Updated maintenance schedule logic.
Date Mon, 29 Jan 2018 05:22:16 GMT

-----------------------------------------------------------
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
> 
>


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