accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William Slacum <>
Subject Re: Resource leak warnings
Date Fri, 13 Dec 2013 20:54:07 GMT
Voting for #1.

On Fri, Dec 13, 2013 at 3:44 PM, Christopher <> wrote:

> What should we do about all these additional resource leak warnings
> added as a result of ACCUMULO-1984? (ACCUMULO-2010)
> As I see it, there's a few options:
> 0. Revert the previous patch for ACCUMULO-1984
> 1. Ignore them
> 2. Suppress them
> 3. Fix them
> 4. Remove Closeable from the interface, but leave the close method
> I don't like the idea of reverting the patch.
> "1" is not really an option for me, because they're creating noise
> that's getting in the way of me debugging stuff I'm working on.
> Given that by making the interface "Closeable", we're in effect
> recommending that users close it, we should probably follow our own
> recommendation, so "2" is probably not a good idea, and "3" is
> probably better. I don't have time to go back and do "3", though.
> "4" might be a good option, at least for 1.4.5-SNAPSHOT and
> 1.5.1-SNAPSHOT, so we don't convey the idea (which represents a change
> in API semantics) that you *should* close the Instance. Rather, it
> conveys the idea that it's optional, which I think is more consistent
> with those previous versions, and is suitable for the vast majority of
> use cases.
> All of this is completely overshadowing the real issue, though, which
> is that the close method doesn't actually prevent the resources from
> being opened again. It's a superficial fix, that doesn't really
> enforce it. Our API looks like it's stateless, with factory methods...
> but it's not actually stateless. We can close the instance, but the
> resources that were left open aren't isolated to the instance... they
> are used inside the Connector and below. Closing the instance may free
> up resources, but it doesn't stop new ones from being opened again
> inside the connector and below. The problem is that the "Instance"
> object does not fully represent the resources used inside client code,
> so closing it is semantically unintuitive, incorrect, and functionally
> broken if not used in a very specific way.
> For the time being, I'm going to pursue option "4", so I can proceed
> with working on things I need to work on, without all the noise.
> Loosely related comments, but probably separate points for discussion:
> A. It'd be nice to require that contributions do not introduce
> compiler warnings (or malformed javadocs) before applying them.
> B. The option to revert is much harder to consider seriously when
> we're simultaneously developing 3 releases, because of the merge
> nightmare: you not only have to revert the patch, but also revert the
> merges, which is not a quick action, because it could result in
> conflicts. Reverting is much more daunting in this scenario. Merge
> windows might help, by providing scheduled times for merging work to a
> common branch, which means that reverts can be considered in a more
> timely manner, because we'll know that new code only shows up during a
> predictable window.
> --
> Christopher L Tubbs II

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message