----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18341/#review35536 -----------------------------------------------------------
Ship it! src/master/registrar.cpp <https://reviews.apache.org/r/18341/#comment66132> It looks like 'applied' is true iff 'operator ()' returns Some or None. Can we simplify the semantics and not require setting 'applied'? What about another wrapper that calls 'operator ()' and sets 'applied' based on the result of 'operator ()'. Also, do you perceive any cases of where Error won't turn into an 'applied = false' case? Finally, since maybe s/applied/success/ since some operations aren't technically "applied" as discussed in Vinod's previous review. src/master/registrar.cpp <https://reviews.apache.org/r/18341/#comment66130> Looks like mis-alignment here. src/master/registrar.cpp <https://reviews.apache.org/r/18341/#comment66129> Minor nit: why ERROR instead of WARNING? This isn't an error, this is an expected case! Also, when first reading this I was sort of surprised that we were just printing this out and not completing the promises/operations here. Perhaps a brief comment explaining that we set this first and do the same for the operations below? - Benjamin Hindman On Feb. 26, 2014, 2:19 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/18341/ > ----------------------------------------------------------- > > (Updated Feb. 26, 2014, 2:19 a.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Bugs: MESOS-764 > https://issues.apache.org/jira/browse/MESOS-764 > > > Repository: mesos-git > > > Description > ------- > > We previously were setting the Promises in some cases _before_ writing to > storage. While this approach was correct, it requires some time and > explanation to convince oneself of this. > > This is why this patch simplifies the semantics by only setting the Promises > _after_ we've written to storage. > > Also, 'Mutation' has now become 'Operation' to better reflect the fact that > some operations do not mutate state when applied. > > > Diffs > ----- > > src/master/registrar.cpp ee16121035db21d966ee151483dd23cbc4a495c2 > > Diff: https://reviews.apache.org/r/18341/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
