> 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?
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.
> On Nov. 5, 2013, 12:58 a.m., Ben Mahler wrote:
> > src/zookeeper/group.cpp, line 350
> > <https://reviews.apache.org/r/15016/diff/3/?file=377511#file377511line350>
> >
> > Why is this CHECK safe?
> >
> > Is it safe because expired() is always called before reconnecting()?
It should be safe because the timer is only set in reconnecting(), which won't
be called twice consecutively without other timer resetting events (i.e.
connected, expired) in between.
> 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?
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.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15016/#review28161
-----------------------------------------------------------
On Nov. 5, 2013, 12:39 a.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, 12:39 a.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
>
>
