That was my assessment too, but the amount of time to get a LOST event seems incorrect to me. It takes 4/3 session timeout seconds, where I think it should take session timeout seconds On 13 Jun 2016 12:02 PM, "Jordan Zimmerman" <[email protected]> wrote:
> Sorry it’s taken me so long to get to this… > > I don’t see what the problem is with testSessionLossWithLongTimeout(). The > session timeout is being set to timing.forWaiting().milliseconds(). The > test on line 116 only waits timing.forWaiting().milliseconds() for timeout > and will almost always fail. If I change this line to: > > timing.multiple(2).forWaiting().milliseconds() > > The test succeeds. This seems correct to me. > > -Jordan > > > On Jun 6, 2016, at 7:41 PM, Cameron McKenzie <[email protected]> > wrote: > > > > Seems like I have uncovered another problem on the 3.0 branch. > > > > It looks like the new (ish) connection handling stuff doesn't seem to be > > working correctly for long session timeouts. Specifically, the test for > > CURATOR-335 fails on the 3.0 branch when run with the new connection > > handling, but works with the 'classic' connection handling. > > > > It fails when asserting that the LOST event occurs after the server is > > stopped. > > > > I'm not going to have time to do much more digging for at least today, > but > > I have made a more targeted test case: > > > > TestFramework:testSessionLossWithLongTimeout on > > the long_session_timeout_issue branch. > > > > if anyone has time to look before I do. > > > > I think that this needs to be resolved before 3.0 can be released. > > cheers > > > > > > On Mon, Jun 6, 2016 at 9:49 AM, Jordan Zimmerman < > [email protected] > >> wrote: > > > >> :D > >> > >>> Is it worth holding up the build to merge CURATOR-331? > >> No, let’s go with what we have. > >> > >>> On Jun 5, 2016, at 6:48 PM, Cameron McKenzie <[email protected]> > >> wrote: > >>> > >>> Ah, must still be recovering, I'm sure I saw it was being applied to > the > >>> 3.0 branch. > >>> > >>> I will merge it into master and 3.0. > >>> > >>> Is it worth holding up the build to merge CURATOR-331? I have asked > Scott > >>> what his opinion is since its the TreeCache stuff. It looks ok to me > >> though. > >>> cheers > >>> > >>> On Mon, Jun 6, 2016 at 9:44 AM, Jordan Zimmerman < > >> [email protected] > >>>> wrote: > >>> > >>>> Yes, that’s correct. It’s a patch against master. I’ll do the merge if > >>>> you’re OK with it. > >>>> > >>>> -Jordan > >>>> > >>>>> On Jun 5, 2016, at 6:42 PM, Cameron McKenzie <[email protected] > > > >>>> wrote: > >>>>> > >>>>> hey Jordan, > >>>>> The fix for CURATOR-335 looks good to me, but I'm wondering if it > >> should > >>>>> actually be applied against master and then merged into 3.0? > >>>>> > >>>>> On Fri, Jun 3, 2016 at 12:21 PM, Jordan Zimmerman < > >>>>> [email protected]> wrote: > >>>>> > >>>>>> no worries - get well. > >>>>>> > >>>>>>> On Jun 2, 2016, at 9:20 PM, Cameron McKenzie < > [email protected] > >>> > >>>>>> wrote: > >>>>>>> > >>>>>>> Thanks for sorting this out Jordan. I'm pretty sick today so won't > >> get > >>>>>>> around to looking at it, but I will try over the weekend or really > >> next > >>>>>> week > >>>>>>> On 3 Jun 2016 7:05 AM, "Jordan Zimmerman" < > >> [email protected]> > >>>>>>> wrote: > >>>>>>> > >>>>>>>>> It sounds like curator is using a different algorithm since it > has > >>>>>>>>> nodes sorting their position to determine if they have a lease or > >>>> not. > >>>>>>>> > >>>>>>>> No - I just added that as I thought there was a bug. But, now I > >>>> realize > >>>>>>>> I’m wrong. So, it was correct all along. Thanks Ben. > >>>>>>>> > >>>>>>>> -Jordan > >>>>>> > >>>>>> > >>>> > >>>> > >> > >> > >
