I'm still seeing 6 failed tests that seem related to the same stuff after merging your fix:
Failed tests: org.apache.curator.framework.recipes.cache.TestPathChildrenCache.testBasics(org.apache.curator.framework.recipes.cache.TestPathChildrenCache) Run 1: TestPathChildrenCache.testBasics:863 One or more child watchers are still registered: [/test] Run 2: TestPathChildrenCache.testBasics:863 One or more child watchers are still registered: [/test] org.apache.curator.framework.recipes.cache.TestPathChildrenCache.testBasicsOnTwoCachesWithSameExecutor(org.apache.curator.framework.recipes.cache.TestPathChildrenCache) Run 1: TestPathChildrenCache.testBasicsOnTwoCachesWithSameExecutor:934 One or more child watchers are still registered: [/test] Run 2: TestPathChildrenCache.testBasicsOnTwoCachesWithSameExecutor:934 One or more child watchers are still registered: [/test] org.apache.curator.framework.recipes.cache.TestPathChildrenCache.testEnsurePath(org.apache.curator.framework.recipes.cache.TestPathChildrenCache) Run 1: TestPathChildrenCache.testEnsurePath:363 One or more child watchers are still registered: [/one/two/three] Run 2: TestPathChildrenCache.testEnsurePath:363 One or more child watchers are still registered: [/one/two/three] TestInterProcessSemaphoreCluster.testCluster:294 expected [true] but found [false] org.apache.curator.framework.recipes.shared.TestSharedCount.testMultiClientVersioned(org.apache.curator.framework.recipes.shared.TestSharedCount) Run 1: PASS Run 2: TestSharedCount.testMultiClientVersioned:256 One or more data watchers are still registered: [/count] org.apache.curator.framework.recipes.shared.TestSharedCount.testSimple(org.apache.curator.framework.recipes.shared.TestSharedCount) Run 1: TestSharedCount.testSimple:174 One or more data watchers are still registered: [/count] Run 2: TestSharedCount.testSimple:174 One or more data watchers are still registered: [/count] Tests run: 491, Failures: 6, Errors: 0, Skipped: 0 On Fri, May 27, 2016 at 3:30 AM, Jordan Zimmerman < [email protected]> wrote: > I see the problem. The fix is not simple though so I’ll spend some time on > it. The TL;DR is that exists watchers are still supposed to get set when > there is a KeeperException.NoNode and the code isn’t handling it. But, > while I was looking at the code I realized there are some significant > additional problems. Curator, here, is trying to mirror what ZooKeeper does > internally which is insanely complicated. In hindsight, the whole ZK > watcher mechanism should’ve been decoupled from the mutator APIs. But, of > course, that’s easy for me to say now. > > -Jordan > > > On May 26, 2016, at 1:10 AM, Cameron McKenzie <[email protected]> > wrote: > > > > Thanks Scott, > > Those tests are now passing for me. > > > > Jordan, testNodeCache:testBasics() is failing consistently on the 3.0 > > branch. It appears that this is actually potentially a bug in the > > NodeCache. It ends up leaking a Watcher reference. I've had a quick look > > through, but I haven't dived in in any detail. It's the > > WatcherRemovalManager stuff I think. If you've got time, can you have a > > look? If not, let me know and I'll do some more digging. > > > > cheers > > > > On Thu, May 26, 2016 at 11:47 AM, Cameron McKenzie < > [email protected]> > > wrote: > > > >> Thanks Scott. > >> > >> Push the fix to master and merge it into 3.0. > >> > >> Then I guess, I'll push new versions of 2.11 and 3.2 onto Nexus. > >> cheers > >> > >> On Thu, May 26, 2016 at 11:44 AM, Scott Blum <[email protected]> > >> wrote: > >> > >>> Alright, I have a fix, but it wants to be applied to both master and > 3.0. > >>> Where should I push the fix? > >>> > >>> On Wed, May 25, 2016 at 6:10 PM, Cameron McKenzie < > [email protected] > >>>> > >>> wrote: > >>> > >>>> Thanks Scott, > >>>> If you just checkout the CURATOR-3.0 branch, they are failing there. > >>>> cheers > >>>> > >>>> On Thu, May 26, 2016 at 2:06 AM, Scott Blum <[email protected]> > >>> wrote: > >>>> > >>>>> Sure, what SHA are they failing at Cam? > >>>>> > >>>>> On Wed, May 25, 2016 at 9:36 AM, Jordan Zimmerman < > >>>>> [email protected]> wrote: > >>>>> > >>>>>> Scott can you take a look? > >>>>>> > >>>>>> -Jordan > >>>>>> > >>>>>>> On May 25, 2016, at 4:35 AM, Cameron McKenzie < > >>>> [email protected]> > >>>>>> wrote: > >>>>>>> > >>>>>>> Tree cache tests are still failing. I've tried a few times but no > >>>> love: > >>>>>>> > >>>>>>> TestTreeCacheEventOrdering>TestEventOrdering.testEventOrdering:151 > >>>>>> actual 6 > >>>>>>> expected -31: > >>>>>>> > >>>>>>> I will have a look into what's going on in the morning. Given that > >>>>> these > >>>>>>> may take some messing about to fix up, do we just want to vote on > >>>>> 2.11.0 > >>>>>>> separately, as that is all ready to go? > >>>>>>> cheers > >>>>>>> > >>>>>>> On Wed, May 25, 2016 at 5:34 PM, Jordan Zimmerman < > >>>>>>> [email protected]> wrote: > >>>>>>> > >>>>>>>> Great news. Thanks. > >>>>>>>> > >>>>>>>> ==================== > >>>>>>>> Jordan Zimmerman > >>>>>>>> > >>>>>>>>> On May 25, 2016, at 12:37 AM, Cameron McKenzie < > >>>>> [email protected] > >>>>>>> > >>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>> I have fixed up the test case, all good now. > >>>>>>>>> > >>>>>>>>> On Wed, May 25, 2016 at 1:45 PM, Cameron McKenzie < > >>>>>>>> [email protected]> > >>>>>>>>> wrote: > >>>>>>>>> > >>>>>>>>>> Looks like it was introduced with the schema validation stuff. > >>> It > >>>>> now > >>>>>>>> does > >>>>>>>>>> an ACL check prior to the backgrounding call. Because the unit > >>>> test > >>>>>>>> uses a > >>>>>>>>>> bogus ACL provider it just throws an exception > >>>>>>>>>> > >>>>>>>>>> final String adjustedPath = > >>>>>>>>>> adjustPath(client.fixForNamespace(givenPath, > >>>>>>>> createMode.isSequential())); > >>>>>>>>>> List<ACL> aclList = acling.getAclList(adjustedPath); > >>>>>>>>>> > >>>>>>>>>> > >>>>> client.getSchemaSet().getSchema(givenPath).validateCreate(createMode, > >>>>>>>> data, > >>>>>>>>>> aclList); > >>>>>>>>>> > >>>>>>>>>> String returnPath = null; > >>>>>>>>>> if ( backgrounding.inBackground() ) > >>>>>>>>>> { > >>>>>>>>>> pathInBackground(adjustedPath, data, givenPath); > >>>>>>>>>> > >>>>>>>>>> So, I guess the answer is to get the test to force a failure > >>> in a > >>>>>>>>>> different way. With the UnhandledErrorListener, the > >>> expectation is > >>>>>> that > >>>>>>>>>> this only gets called on backgrounding operations? > >>>>>>>>>> cheers > >>>>>>>>>> > >>>>>>>>>> On Wed, May 25, 2016 at 1:39 PM, Cameron McKenzie < > >>>>>>>> [email protected]> > >>>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Same on the master branch, but it passes there, so maybe > >>>> something > >>>>>> has > >>>>>>>>>>> legitimately broken the test. Will let you know if I get > >>> stuck. > >>>>>>>>>>> cheers > >>>>>>>>>>> > >>>>>>>>>>> On Wed, May 25, 2016 at 1:36 PM, Jordan Zimmerman < > >>>>>>>>>>> [email protected]> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Let me know if you need help. > >>>>>>>>>>>> > >>>>>>>>>>>> It might be a bad merge. Have you compared it to the master > >>>>> branch? > >>>>>>>>>>>> > >>>>>>>>>>>> -JZ > >>>>>>>>>>>> > >>>>>>>>>>>>>> On May 24, 2016, at 10:31 PM, Cameron McKenzie < > >>>>>>>> [email protected]> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>> Guys, > >>>>>>>>>>>>> There's a test TestFrameworkBackground:testErrorListener > >>> that > >>>> is > >>>>>>>>>>>> failing > >>>>>>>>>>>>> consistently on the CURATOR-3.0 branch. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I can't see how it has ever worked. It seems to try and > >>> provoke > >>>>> an > >>>>>>>>>>>> error > >>>>>>>>>>>>> via a bad ACL provider. > >>>>>>>>>>>>> > >>>>>>>>>>>>> But this ACL provider is called by the CreateBuilderImpl > >>> prior > >>>> to > >>>>>> the > >>>>>>>>>>>>> backgrounding call, which means that the exception that it > >>>> throws > >>>>>>>>>>>> happens > >>>>>>>>>>>>> in the main Thread of the unit test. So, it just throws an > >>>>>>>>>>>>> UnsupportedOperationException which is propogated up the > >>> stack > >>>> at > >>>>>>>> which > >>>>>>>>>>>>> point the unit test fails. > >>>>>>>>>>>>> > >>>>>>>>>>>>> So, I will look at fixing this, but I just don't understand > >>> how > >>>>> it > >>>>>>>> ever > >>>>>>>>>>>>> worked? > >>>>>>>>>>>>> cheers > >>>>>>>>>> > >>>>>>>> > >>>>>> > >>>>>> > >>>>> > >>>> > >>> > >> > >> > >
