mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Hindman <b...@berkeley.edu>
Subject Re: Review Request 54839: Updated Mesos process::loop uses with process::ControlFlow.
Date Sun, 18 Dec 2016 01:29:31 GMT


> On Dec. 17, 2016, 9:53 p.m., Kevin Klues wrote:
> > src/slave/containerizer/mesos/io/switchboard.cpp, lines 1571-1593
> > <https://reviews.apache.org/r/54839/diff/1/?file=1588551#file1588551line1571>
> >
> >     Is there a specific reason to reorganize things in this way with `.then()`,
`.repair()` and `.recover()`. I'm mostly curious because using `.onAny()` tends to be simpler
for me to reason about and I don't quite follow what's going on with your change (plus we
lose the discarded case).

The reason is to simplify the logic around not having to create a `Promise`. While creating
the promise is not that big of a deal, the bigger problem is actually linking the promise
so as to properly handle discards. In the original code you don't propagate any discards into
the `process::io::write` call which means that even if the code around you was discarded this
code won't be. Setting up the discard chaining is tedious and complex. When you chain futures
with `.then`, `.repair`, etc, then you don't have to worry about it. And note that in your
original code you should have never had the discard case because there wasn't any chaining!
That being said, adding `.recover` is meant to capture the chaining. Going forward, we want
to introduce something like `.match` that let's you chain correctly regardless of the result.


- Benjamin


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


On Dec. 17, 2016, 9:40 p.m., Benjamin Hindman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54839/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2016, 9:40 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar, Kevin Klues, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Updated Mesos process::loop uses with process::ControlFlow.
> 
> 
> Diffs
> -----
> 
>   src/common/recordio.hpp 5a22d066bd2bcd40ede5ebf6476b9ceb972dd584 
>   src/slave/containerizer/mesos/io/switchboard.cpp c80627986f729255e3b3ad1fc9cfa6213e19c521

>   src/slave/http.cpp e1cea46cb17ea2f24fe457bb06a0a1a55c2a2bc4 
> 
> Diff: https://reviews.apache.org/r/54839/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Benjamin Hindman
> 
>


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