Home | About | Sematext search-lucene.com search-hadoop.com
 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
Mike Drob 2013-12-23, 21:36
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,