mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrei Sekretenko <asekrete...@mesosphere.io>
Subject Re: Review Request 71436: Replaced removeOffer + recoverResources pairs with specialized helpers.
Date Fri, 06 Sep 2019 17:03:47 GMT


> On Sept. 4, 2019, 11:53 p.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Line 3357 (original), 3352 (patched)
> > <https://reviews.apache.org/r/71436/diff/1/?file=2163351#file2163351line3357>
> >
> >     Looking at the call site, this `rescind` parameter is redundant, we could just
look at framework->active(), this saves one parameter and also saves me from wondering
what makes a `resind` true.

Looks like we can't :(

rescind = true:
https://github.com/apache/mesos/blob/6c2a94ca0eca90e6d3517e4400f4529ddce0b84c/src/master/master.cpp#L3335
rescind = false:
https://github.com/apache/mesos/blob/6c2a94ca0eca90e6d3517e4400f4529ddce0b84c/src/master/master.cpp#L11228

Both are calling `deactivate()` for an active framework only.


> On Sept. 4, 2019, 11:53 p.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Line 8849 (original), 8811 (patched)
> > <https://reviews.apache.org/r/71436/diff/1/?file=2163351#file2163351line8864>
> >
> >     LOG should be "Rescinding"?

yeah, and several other similar places


> On Sept. 4, 2019, 11:53 p.m., Meng Zhu wrote:
> > src/master/master.cpp
> > Line 8910 (original), 8860 (patched)
> > <https://reviews.apache.org/r/71436/diff/1/?file=2163351#file2163351line8925>
> >
> >     s/Remove and rescind/Rescind/

Thanks for noticing!

Realized that we have many more comments of this kind. 
Rewrote some of them, removed many others (which are redundant now).


- Andrei


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


On Sept. 6, 2019, 4:56 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71436/
> -----------------------------------------------------------
> 
> (Updated Sept. 6, 2019, 4:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Meng Zhu.
> 
> 
> Bugs: MESOS-1452 and MESOS-9949
>     https://issues.apache.org/jira/browse/MESOS-1452
>     https://issues.apache.org/jira/browse/MESOS-9949
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds helper methods `Master::rescindOffer()` /
> `Master::discardOffer()` that recover offer's resources in the allocator
> and remove the offer, and replaces paired calls of `removeOffer()` +
> `allocator->recoverResources()` with these helpers.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp 0987d933c03b059c70f0b7c6231df73b8b4d228f 
>   src/master/master.hpp 19c17821d2b8111ebe8e1429a5b4a8423357fdd6 
>   src/master/master.cpp f00906ef2d33920f23127a74ed141fff9d32865b 
>   src/master/quota_handler.cpp f28eb27964afd2c8bfa0bf7a3ab8246442812f2a 
>   src/master/weights_handler.cpp dfb6f06449f85e7acb156cac0ff79cadcc712255 
> 
> 
> Diff: https://reviews.apache.org/r/71436/diff/2/
> 
> 
> Testing
> -------
> 
> make check, 10 times
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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