> On Feb. 26, 2014, 6:04 p.m., Benjamin Hindman wrote: > > src/master/registrar.cpp, lines 79-83 > > <https://reviews.apache.org/r/18341/diff/3/?file=504150#file504150line79> > > > > 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.
I've made some updates here. Now Operation is still a functor, but the () operator wraps a call to a protected 'apply' function. This is done in order to set 'success' based on the result of the 'apply' call. To answer your question, 'success' is true iff !apply().isError() and I think this will generalize to all operations. Now each Operation overrides the pure virtual apply() call and does not have to deal with setting the 'success' boolean, which is in private scope anyway and is thus hidden from the subclasses of Operation. Let me know how it looks, it's a bit unfortunate that we've introduced a copy across () and apply(). - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18341/#review35536 ----------------------------------------------------------- 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 > >
