> On Nov. 5, 2013, 12:58 a.m., Ben Mahler wrote: > > src/zookeeper/group.cpp, line 373 > > <https://reviews.apache.org/r/15016/diff/3/?file=377511#file377511line373> > > > > If you fail to cancel it you'll have a pending call to timedout(), are > > you assuming that the Timer will not become SOME before the event for > > timedout() is processed? > > Jiang Yan Xu wrote: > It should be fine here too because in timedout(): > > if (timer.isSome() && > timer.get().timeout().expired() && > zk->getSessionId() == sessionId) { ... } > > > The sessionId will be different and thus the old timeout is ignored.
Sounds good. > On Nov. 5, 2013, 12:58 a.m., Ben Mahler wrote: > > src/zookeeper/group.cpp, line 325 > > <https://reviews.apache.org/r/15016/diff/3/?file=377511#file377511line325> > > > > What happens if cancel returns false? > > > > We'll set the timer to None() but there's an impending call to > > timedout() in the queue? Is that intended? > > Jiang Yan Xu wrote: > If timedout() dispatched -> connected() executed (timer = NONE) -> > timedout() executed: timer will be NONE in the "if (timer.isSome() && > zk->getSessionId() == sessionId)" check and the group will not abort. > If timedout() dispatched -> connected (timer = NONE) -> reconnecting (new > timer) -> previous enqueued timedout() executed: this is what I hadn't > thought about and we should not allow the timedout() method to abort > everything. > > So I changed the check condition in timedout() to be: > > if (timer.isSome() && > timer.get().timeout().expired() && > zk->getSessionId() == sessionId) { ... } > > So because the new timer has a pending timeout, the condition will > evaluate to false and the group will not abort. Ok great, thanks. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15016/#review28161 ----------------------------------------------------------- On Nov. 5, 2013, 10:12 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15016/ > ----------------------------------------------------------- > > (Updated Nov. 5, 2013, 10:12 p.m.) > > > Review request for mesos, Benjamin Hindman, Ben Mahler, and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > See summary > > > Diffs > ----- > > src/zookeeper/group.hpp 6b6c1557658df1bb7b7df3ded5c203e2dc0c8308 > src/zookeeper/group.cpp cd58d230a2715b9ed01002443ebd7b5366d31be6 > > Diff: https://reviews.apache.org/r/15016/diff/ > > > Testing > ------- > > make check. > > > Thanks, > > Jiang Yan Xu > >
