Mailing-List: contact dev-help@accumulo.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@accumulo.apache.org
Received-SPF: softfail (athena.apache.org: transitioning domain of
 wilhelm.von.cloud@accumulo.net does not designate 209.85.216.52 as permitted
 sender)
MIME-Version: 1.0
In-Reply-To: 
 <CABwccFGTMMHi6gaEcKi-7W-_OuKXFF+xY-OCSz4FEhWB6HgDvA@mail.gmail.com>
References: 
 <CAL5zq9a-Vi9d8Z8YXBR0Hy-JiyiQhFTE9eEagV8qUBNXN-AOcw@mail.gmail.com>
	<CABwccFGRfwtrnByug3Qn52iSTZJXqtBSx51KS1M4qZaD6UUnLg@mail.gmail.com>
	<CAL5zq9axWU++o5LS72XRaSO5T3HbkHM8gemdCR2HGkz3AwNJSQ@mail.gmail.com>
	<CABwccFF9wN9je1GS-=m1pxv2OWgx=581iKVsC-+AecsN-Ha_=g@mail.gmail.com>
	<CAL5zq9a46UoOV43_EdJHUmeY-8LKgxDjbnDOWYSvGT80iYiW_Q@mail.gmail.com>
	<CAJRvFdoTDste36BM5S7cJQs=zP8noev3SLMtpS+njKaS07Nkkg@mail.gmail.com>
	<CAL5zq9bLcbcSGF7T+2dTS6igHyokWjd1yMbNV+gxWfQDK53gng@mail.gmail.com>
	<CAD-fFUJ2JCou_NW9rZrj6S5+8tod4c93SPhnj74PUPqSWgKM2A@mail.gmail.com>
	<CAL5zq9ZR5JtQWy0FuRwf2kjBkwpuD5vC6_RwYbusMw6+jO=BYg@mail.gmail.com>
	<CAGUtCHrnC-4q1rcAy8MMS+njWY=byN9M2dQe0U2kpAqKeS5S9g@mail.gmail.com>
	<CAFAMeY+ceRqPW1vxEXjapg6dJB25kca94VE06RD8AkQDE6xm2w@mail.gmail.com>
	<CAGUtCHrThjFW-u+JK+ZYALB-qut2VNd5AH1Sb8o7UcQFUXM4xA@mail.gmail.com>
	<CAFAMeYK_2b+XJuHJt2s=L9gfkjvWFFD3vP=miNQ+yRnuutWSCQ@mail.gmail.com>
	<CABwccFGTMMHi6gaEcKi-7W-_OuKXFF+xY-OCSz4FEhWB6HgDvA@mail.gmail.com>
Date: Fri, 27 Dec 2013 14:23:10 -0500
Message-ID: 
 <CAMz+DuvUZrg-2FyoL68+PbLqMn0Y4TKrPf2duvCkVCH-2XxRqg@mail.gmail.com>
Subject: Re: Resource leak warnings
From: William Slacum <wilhelm.von.cloud@accumulo.net>
To: dev@accumulo.apache.org
Content-Type: multipart/alternative; boundary=047d7bd75e90552a4604ee890688

--047d7bd75e90552a4604ee890688
Content-Type: text/plain; charset=ISO-8859-1

We need to actually define the usage pattern and lifetime of a
ZooKeeperInstance. Looking at the code, it's really masking a singleton
usage pattern. The resources backing a given set of zookeepers+timeout pair
all share a ZooCache, and we hand-rolled reference counting for
ZooKeeperInstances themselves. That indicates that a ZooKeeperInstance is
basically a global variable, and we have to be careful about the resources
it allocates, directly or indirectly, because their lifetimes are opaque
from the perspective of the client.

I'm a fan of the close method, because it puts, in code, how an instance
tidies up after itself. We didn't have any cleanup before because the
ZooCache for a given zookeeper+timeout lived on until the process died.
Since the side effects of our API aren't documented or made clear to the
client, it's on us to handle and manage them. Making it optional for a user
is a benefit, because maybe they don't care and someone else (gc, another
management thread) will call close() on the instance, or maybe they want to
force a close at class unloading.

The utility seems to be brute forcing shutdown- is it possible to get
something finer grained for specific instances? Shutting down every thing
will handle the "clean up at unload" time issue, but not necessarily
anything involving closing down a subset of ZooSessions.


On Thu, Dec 26, 2013 at 2:48 PM, Sean Busbey <busbey@clouderagovt.com>wrote:

> On Dec 26, 2013 12:27 PM, "Mike Drob" <mdrob@cloudera.com> wrote:
> >
> > I'm willing to stipulate that this solves the thread leak from web
> > containers - I haven't verified it, but I am ever hopeful. Does this
> > solution imply that we should nix the close() methods just added in the
> > snapshot branches?
> >
> >
>
> If we can verify that it solves the leaks for web containers, I would say
> yes.
>
> We can do proper life cycle for persistent state when we provide an updated
> client API.
>

--047d7bd75e90552a4604ee890688--