Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo >> mail # dev >> Resource leak warnings


Copy link to this message
-
Re: Resource leak warnings
At best the javadoc is incomplete and at worst incorrect. If it were just
representing configuration information, it would be a structure containing
only fixed data like the zookeeper list and timeout. Instead, it creates
resources and has a direct handle to those resources via its own ZooCache
property and it contains convenience methods to create other resources like
connectors. A javadoc comment is enough to warrant ignoring resource
management.

Storing state statically one thing, not cleaning up after ourselves is
another. We don't need a whole new API to do that because we've already
done that with the addition of `close()`. Keeping a list of
ZooKeeperInstances to close is already provides the same functionality as
just shutting down everything with the utility, as well as the ability to
free a subset of the resources.

That being said, has anyone started on the utility so we can at least have
a comparison/bake off? I assume this is going to block 1.6.0/1.5.1.

On Fri, Dec 27, 2013 at 6:52 PM, Christopher <[EMAIL PROTECTED]> wrote:

> The javadoc for Instance says: "This class represents the information
> a client needs to know to connect to an instance of accumulo."
>
> There's no mention of connection resources or shared state, or any
> indication that it is used for anything other than a one-time method
> to get a connection... it seems to be defined as configuration
> information. The fact that we're talking about it representing
> connection resources (which aren't even stored in ZooKeeperInstance
> itself, but happens to use some of the shared state we're talking
> about for its own implementation), is a bit confusing in the context
> of the declared semantics from the javadoc.
>
> The fact is, we store state statically, as global resources, in the
> JVM, and (I think) changing the definition of Instance to represent
> this statically stored state, is very confusing. I think a static
> utility makes a lot more sense to clean up static shared state hidden
> deep in the implementation... until we can invent (in a new API) an
> actual ConnectionResources object to represent connection resources,
> with a well-defined lifetime (not "for the duration of the JVM's
> lifetime", as it currently is defined in released versions) where the
> cleanup of these resources makes sense.
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Fri, Dec 27, 2013 at 2:23 PM, William Slacum
> <[EMAIL PROTECTED]> wrote:
> > 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 <[EMAIL PROTECTED]
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB