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
Keith,

I'm not sure I understand the alternative solution that you and Christopher
discussed. Can you explain it in a bit more detail, please? I have my own
interpretation of what I _think_ you were proposing, but I'd rather not
risk putting words in your mouth. Specifically, I'm interested in the
auto-cleanup aspect of things.

I do think that the numbered changes you propose are good ideas. I also
agree with Christopher that we need to seriously examine the API that we
have, because chaining methods four- or five-deep [e.g. new
Instance().getConnector().tableOperations().createTable()] to do something
is not a great user experience. Determining the scope of the changes is
another community conversation.

Mike
On Mon, Dec 23, 2013 at 9:17 AM, Keith Turner <[EMAIL PROTECTED]> wrote:

> On Sun, Dec 22, 2013 at 10:28 PM, Christopher <[EMAIL PROTECTED]> wrote:
>
> > On Sun, Dec 22, 2013 at 2:23 PM, Bill Havanki <[EMAIL PROTECTED]
> >
> > wrote:
> > [snip]
> > > Although there was no intention of circumventing consensus, looking at
> > the
> > > email exchange, consensus was clearly not reached.
> >
> > It is my understanding that typically, in CtR, consensus is needed to
> > resolve issues after they are committed, where there is
> > conflict/objections. Perhaps it was my misunderstanding of the
> > responses, but it was my understanding that while there was no
> > consensus on the final solution, there was no objection that would
> > have prevented the interim action taken.
> >
> > > The short time span did
> > > not give others the chance to work on eliminating the warnings, as they
> > > offered, or to instead come around to just dropping Closeable.
> >
> > True... the timespan was short. My goal, as stated in the original
> > email, was to commit first (just like I might commit any improvement
> > to the current state of the code), and I intended the email to just be
> > an explanation of the reasoning, as it related to the prior commits,
> > and a prompt for discussion of further action. The fact that I
> > submitted the email chronologically first was a bit arbitrary. I
> > accept blame for the confusion of that, and any inciting wording the
> > email may have caused... I probably could have prepped things a bit
> > better... I have many personal "lessons learned" from this. :)
> >
> > > Personally,
> > > I am ambivalent about it. In any event, -1923 now exists to
> > comprehensively
> > > tackle the issue, and I eagerly welcome input and help on it.
> > >
> > > Removing Closeable did not undo all the work done, but it did undo some
> > of
> > > it. It's OK to call it that. Sometimes undoing is fine. That part of
> the
> > > commit for -2010 is a minimal change. We all agree Closeable should be
> > > there eventually, which is more important. We'll get it back.
> >
> > "undo" or "improve upon" is probably a semantic difference... but
> > yeah, my intent was to make it trivial to re-introduce if we decided
> > it was best to keep it.
> >
> > However, I'm not sure we all agree that Closeable should be there
> > eventually. I cannot speak for Keith Turner (hopefully, he'll chime in
> >
>
> Its not just Closeable, I am uncertain about adding a Instance.close() to
> the API.  Its a very broad change to solve a very specific problem, which
> is not a problem in itself.  My specific concern is that its a big change
> to the API w/o much vetting.  I think the following changes should be made
> before release.
>
>  1) Modify existing examples to use the new instance.close() call.
>  2) Modify existing test to use the new instance.close() call.
>  3) Address the warnings
>  4) Create test to verify the expected behavior of the new instance.close()
> call.  For example verify that Scanners, BatchScanner, tablet operations,
> etc all stop working when an instance is closed (if this is the intended
> behavior?).  Verify that closing one instance object does not impact other
> instance objects.
>
> The purpose of #1, #2, and #3 is to eat our own dog food.  Doing #1, #2,
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