> On Dec. 18, 2013, 7:50 p.m., Ben Mahler wrote: > > src/zookeeper/contender.cpp, line 148 > > <https://reviews.apache.org/r/16290/diff/3/?file=399313#file399313line148> > > > > Is this also a Failure to withdraw?
This is "impossible not happen" because Group doesn't discard futures unless it's destructing - which shouldn't happen either here because Group outlives the contender. > On Dec. 18, 2013, 7:50 p.m., Ben Mahler wrote: > > src/zookeeper/contender.cpp, line 123 > > <https://reviews.apache.org/r/16290/diff/3/?file=399313#file399313line123> > > > > Looks like this is a failure to contend rather than a consistency > > issue, so it seems Failure is more appropriate here. Here it's a CHECK because this usage violates the instructions in contender.hpp: " // Note that the contender is NOT reusable, which means its methods // are supposed to be called once and the client needs to create a // new instance to contend again. " It was suggested in Vinod's review. Sound alright? > On Dec. 18, 2013, 7:50 p.m., Ben Mahler wrote: > > src/zookeeper/contender.cpp, lines 138-139 > > <https://reviews.apache.org/r/16290/diff/3/?file=399313#file399313line138> > > > > I'm not sure this should be a CHECK failure since it can be handled > > gracefully. Can this return a failure instead? Same as above. - Jiang Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/16290/#review30628 ----------------------------------------------------------- On Dec. 18, 2013, 6 a.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/16290/ > ----------------------------------------------------------- > > (Updated Dec. 18, 2013, 6 a.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Bugs: MESOS-883 > https://issues.apache.org/jira/browse/MESOS-883 > > > Repository: mesos-git > > > Description > ------- > > With this patch the group only returns failed futures when the error is > non-retryable and client (master, slave, sched) should always terminate. > > > Diffs > ----- > > src/master/master.hpp 6c168a2cdbd8343516cb47adceaff70c3d46690b > src/master/master.cpp dd6111946289b82008a66e46df8ef5e538de70ea > src/sched/sched.cpp c46535643ce2ea456bd939571fe78358c1d3871b > src/slave/slave.cpp 6e6107e3551f29566ff233dad47dac8c29b4fab5 > src/tests/master_contender_detector_tests.cpp > 76464eab479461e6e3cb8b5afe85860e60428cf5 > src/tests/zookeeper_tests.cpp a0660cbfb1b7073654b05b82bffd69fbb04d0165 > src/zookeeper/contender.hpp d0b386eeec485f7c8e6b5e60936a84d2f0bba92d > src/zookeeper/contender.cpp bb7e25555b2635398e9d56a808993bcd0b0e47f6 > src/zookeeper/group.hpp 088363a80108472ffeb14121fcb4da02d6e9acf4 > src/zookeeper/group.cpp f3ba86fec3584a83fd55d6388656ebde5249824a > > Diff: https://reviews.apache.org/r/16290/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jiang Yan Xu > >
