accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher <ctubb...@apache.org>
Subject Re: Resource leak warnings
Date Fri, 13 Dec 2013 21:26:25 GMT
On Fri, Dec 13, 2013 at 3:55 PM, Sean Busbey <busbey+ml@clouderagovt.com> wrote:
> I think the answer should be #3 and I am willing to find time to implement
> it next week.

That's great. In the meantime, I'm doing #4, so I can clear the noise
and focus on my own tickets.

> I agree with your reasoning on #1 and #2. I think #0 is a no-go because the
> behavior before it is incorrect.

#0 would've been a temporary option, until a new patch is applied with
the appropriate fixes.

> As far as I'm concerned, these should have always been closable and users
> should be closing them. I think Christopher made the argument in chat that
> essentially we are just now enabling users to do what they should have done
> all along.
>
> That new resources can be opened is immaterial to closing an individual
> instance a user has up. I'd also say it's inaccurate to imply that we're
> stateless, because we very obviously are not. I think that makes it a bug
> in how we handle users closing and opening, not in marking things Closeable.

One concern, that I think Keith Turner has pointed out a few times, is
that Instance has grown from being a carrier of configuration for
Connector (which was itself just intended to just be a stateless
factory for scanners/writers), to being the entry point into a
stateful client API. It was not intended to carry or represent
resources. It was only ever intended to carry a mechanism (function
implementation) for finding the instanceId. (Maybe it was badly named
for this purpose... maybe it should have been InstanceIdFinder or
something other than Instance?)

However, what's done is done, I'm not sure it can be easily fixed. We
need a whole new API that admits to being stateful from the beginning,
and is designed in a way to manage resources locally to the API
objects where they are needed. I've got some ideas, and my intent is
to pursue this further in the next release after 1.6.0.

So, yes, in one sense, we're enabling users to do what should've been
able to do all along, but in another sense, it's not quite true...
because Instance was never intended to represent those resources.

> Also, you should not be able to use a Connection that is attached to a
> now-closed Instance. I would also consider such use a bug.
>
[snip]

It's not just the Connection you'd have to enforce not being re-used
after the Instance is closed... it's every object our factory-based
API can give you, all the way down. That's not a small change.

--
Christopher L Tubbs II
http://gravatar.com/ctubbsii

Mime
View raw message