Mailing-List: contact dev-help@curator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@curator.apache.org
Content-Type: multipart/alternative;
 boundary="Apple-Mail=_D5B2C75C-2B33-4C7E-85A3-859D53374687"
Mime-Version: 1.0 (Mac OS X Mail 9.2 \(3112\))
Subject: Re: PLEASE REVIEW - Major re-work of Watcher wrappers
From: Jordan Zimmerman <jordan@jordanzimmerman.com>
In-Reply-To: 
 <CALuNCpiKMxJurXZwKPwHfCiRkGsZUcqsh5V0dqnRvw+6w5PmdQ@mail.gmail.com>
Date: Fri, 5 Feb 2016 12:25:57 -0500
Cc: "dev@curator.apache.org" <dev@curator.apache.org>
Message-Id: <8E4199BD-6975-4CF3-90BB-16F05FB71342@jordanzimmerman.com>
References: <B5085594-84DF-42B2-94B5-68262EF4A6B8@jordanzimmerman.com>
 <12BFA12E-B07C-4CDD-AD15-7019C8102DB5@jordanzimmerman.com>
 <CALuNCpi-yqKvS_62FWVK9Njg9-fBijgDFUccPwWJ+=vjjZMM1w@mail.gmail.com>
 <389A2290-081C-4B21-8B4F-DE32DA01249A@jordanzimmerman.com>
 <CALuNCphQsQPUGoOe1NOvTysJsfGJy+RD2k4anFd1=UdOCqxJ=A@mail.gmail.com>
 <CALuNCpj8dnqfN2sBpME=6=1p=_gd8nmcAMS5Ot9LcmqqivfLXg@mail.gmail.com>
 <CALuNCpiKMxJurXZwKPwHfCiRkGsZUcqsh5V0dqnRvw+6w5PmdQ@mail.gmail.com>
To: Scott Blum <dragonsinth@gmail.com>

--Apple-Mail=_D5B2C75C-2B33-4C7E-85A3-859D53374687
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

OK - please create a new Issue in Jira for this.

-Jordan

> On Feb 5, 2016, at 12:24 PM, Scott Blum <dragonsinth@gmail.com> wrote:
>=20
> BTW: this is broken on CURATOR-3.0 as well, so it appears to have been =
broken for a while.  Maybe I'll have to git bisect...
>=20
> On Fri, Feb 5, 2016 at 12:22 PM, Scott Blum <dragonsinth@gmail.com =
<mailto:dragonsinth@gmail.com>> wrote:
> Okay, so I looked into this for a bit, and I hit kind of a wall.  I =
think there is a legit bug/race in TreeCache, and the following patch =
*should* remedy:
>=20
> diff --git =
a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache=
/TreeCache.java =
b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache=
/TreeCache.java
> index df4403c..a4a022b 100644
> --- =
a/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache=
/TreeCache.java
> +++ =
b/curator-recipes/src/main/java/org/apache/curator/framework/recipes/cache=
/TreeCache.java
> @@ -303,7 +303,6 @@ public class TreeCache implements Closeable
>          void wasDeleted() throws Exception
>          {
>              ChildData oldChildData =3D childData.getAndSet(null);
> -            =
client.watches().remove(this).ofType(WatcherType.Any).locally().inBackgrou=
nd().forPath(path);
>              ConcurrentMap<String, TreeNode> childMap =3D =
children.getAndSet(null);
>              if ( childMap !=3D null )
>              {
> @@ -807,8 +806,16 @@ public class TreeCache implements Closeable
>          case RECONNECTED:
>              try
>              {
> +                outstandingOps.incrementAndGet();
>                  root.wasReconnected();
>                  =
publishEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED);
> +                if ( outstandingOps.decrementAndGet() =3D=3D 0 )
> +                {
> +                    if ( isInitialized.compareAndSet(false, true) )
> +                    {
> +                        =
publishEvent(TreeCacheEvent.Type.INITIALIZED);
> +                    }
> +                }
>              }
>              catch ( Exception e )
>              {
>=20
> That should guarantee that the initialized event gets deferred until =
all outstanding refreshes finish.. but it's not.  Something seems to =
have changed under the hood in how background events are getting sent to =
TreeCache, and I don't really understand it yet.  And running the =
debugger seems to affect the timing, like something racy is going on. :(
>=20
>=20
> On Fri, Feb 5, 2016 at 11:57 AM, Scott Blum <dragonsinth@gmail.com =
<mailto:dragonsinth@gmail.com>> wrote:
> Ok, that is kind of weird.  I'll take a look.
>=20
> On Fri, Feb 5, 2016 at 4:58 AM, Jordan Zimmerman =
<jordan@jordanzimmerman.com <mailto:jordan@jordanzimmerman.com>> wrote:
> No, sorry. The last few lines of the test currently are:
>=20
> assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", =
"data".getBytes());
> assertEvent(TreeCacheEvent.Type.INITIALIZED);
>=20
> This fails. But, if I switch them it works:
>=20
> assertEvent(TreeCacheEvent.Type.INITIALIZED);
> assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", =
"data".getBytes());
>=20
>> On Feb 5, 2016, at 2:57 AM, Scott Blum <dragonsinth@gmail.com =
<mailto:dragonsinth@gmail.com>> wrote:
>>=20
>> So you end up with 2 initialized events?
>>=20
>> You mean this?
>>=20
>>          assertEvent(TreeCacheEvent.Type.CONNECTION_RECONNECTED);
>> +        assertEvent(TreeCacheEvent.Type.INITIALIZED);
>>          assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", =
"data".getBytes());
>>          assertEvent(TreeCacheEvent.Type.INITIALIZED);
>>=20
>> Seems weird if there are two, but I can help look.
>>=20
>> On Thu, Feb 4, 2016 at 10:48 PM, Jordan Zimmerman =
<jordan@jordanzimmerman.com <mailto:jordan@jordanzimmerman.com>> wrote:
>> Hey Scott,
>>=20
>> In this branch, TestTreeCache.testKilledSession() is failing at:
>>=20
>>         assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", =
"data".getBytes());
>>=20
>> However, if I change the two asserts to:
>>=20
>>         assertEvent(TreeCacheEvent.Type.INITIALIZED);
>>         assertEvent(TreeCacheEvent.Type.NODE_REMOVED, "/test/me", =
"data".getBytes());
>>=20
>> it works. Does that make any sense?
>>=20
>> -Jordan
>>=20
>> > On Feb 4, 2016, at 9:23 PM, Jordan Zimmerman =
<jordan@jordanzimmerman.com <mailto:jordan@jordanzimmerman.com>> wrote:
>> >
>> > Devs,
>> >
>> > In trying to fix the bad log message "Failed to find watcher=E2=80=9D=
 (which turns out to be a ZK client issue), I realize that the =
NamespaceWatcher and WatcherWrapper stuff could be improved. I=E2=80=99m =
still working on getting all tests to pass but I=E2=80=99d appreciate =
more sets of eyes on this change. Please review carefully if you can.
>> >
>> > https://github.com/apache/curator/pull/131 =
<https://github.com/apache/curator/pull/131>
>> >
>> > -Jordan
>>=20
>>=20
>=20
>=20
>=20
>=20


--Apple-Mail=_D5B2C75C-2B33-4C7E-85A3-859D53374687--