accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bill Havanki <bhava...@clouderagovt.com>
Subject Re: Resource leak warnings
Date Sun, 22 Dec 2013 19:23:05 GMT
My input for the "discussion of procedure" part of the thread (feel free to
fork the thread to talk about the resource closing), as the contributor to
-1984.

I was unable to be involved with the discussion around -2010 until too
late, as I was out sick and missed the span between when Christopher raised
his concerns to the dev list and committed the removal of Closeable. Like
Mike, I decided to let the whole thing cool off.

Although there was no intention of circumventing consensus, looking at the
email exchange, consensus was clearly not reached. 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. 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.

I never saw any compiler warnings because I don't use Eclipse. I can
appreciate wanting to kill annoying warnings, but it would have been better
to tell Eclipse to STFU about them, until we could come around to resolving
them. If and when we do introduce some pertinent bylaws, the peculiarities
of an IDE should not drive them. Tools are there to help us, not tell us
what to do.

There should be no committer norm of unilaterality. (OK, for the most
obviously trivial of changes, but that's it.) Never mind whether this case
was unilateral: we can agree that a unilateral action has the chance to
make others feel less valued and frustrated … even if the action is a
beneficial one! Bylaws are a great way to avoid this, by setting ground
rules. They can strike a balance, because we also do not want to be
paralyzed by excessive multilaterality.

This is all part of the maturing of a software project. We need to focus on
it. A healthy community around Accumulo is necessary for it to succeed.

Thanks for reading!
Bill


On Sat, Dec 21, 2013 at 5:47 PM, Christopher <ctubbsii@apache.org> wrote:

> tl;dr - skip to the dotted line "......" if you don't care about the
> discussion of procedure and just want to move on to the topic of how
> best provide a way to clean up resources in the API.
>
> Mike, et. al.-
>
> Well, I certainly apologize if there were any hurt feelings. It was
> not my intention to circumvent consensus or community involvement. I
> was just trying to open the conversation, while still proceeding with
> a slight improvement to the work already done that allowed progress,
> but didn't undo that work already done. Even after receiving feedback,
> I didn't see any response that was a blocker for my proceeding with
> what I indicated I was going to do, as that feedback was not mutually
> exclusive to what I had said I was going to do for an improvement.
>
> As for the temporary git workaround... I can try that in the future...
> but it's still a lot of hardship to work around this in 3 different
> branches, and then dealing with the side-effects (it's not a free
> solution... it comes with trade-offs). I might have been more willing
> to pursue this as a temporary solution in one branch if I knew that
> fix was imminent. I did notice that some commits have since been made
> (which broke the M/R tests) to close some of the instances... but it's
> not complete.
>
> What I was going for was a minimal change solution that didn't undo
> the previous work, but still left open the possibility of improvement
> on the foundation that was already laid (unless we come up with a
> better solution). I was not trying to undermine anybody's efforts or
> unilaterally make decisions (outside the committer norms, that is). I
> was simply trying to make an improvement to the previous commit, and
> provide an explanation for transparency and discussion on the mailing
> list so we could take further action, if warranted. We make these
> sorts of improvements to previous commits/contributions all the time
> (sometimes without discussion at all, if the improvements are as minor
> as this one was), and it doesn't cause issue, so I don't understand
> why it should have generated any controversy or anger this time, but I
> nevertheless apologize for the fact that it evidently did.
>
> I think most of this can really be address, though, with some bylaws
> that establish standard procedures for bad, or incomplete commits,
> introduction of new warnings, backporting, reverting bad patches vs.
> making a minor mod to improve them, etc. I think we should make it a
> priority in 2014 to put together those guidelines in bylaws.
>
> .........................................................................
> Getting back to what to do about resources...
>
> For the present:
>
> Keith and I brainstormed a little bit... and he suggested that there
> might be a way to have the RPC threads automatically clean up
> themselves, and I suggested that we could provide an Instance
> constructor that takes a ZooKeeper object that the user can
> manage/close. This solves the cleanup issue without incorporating
> confusing API semantics of a close() method... but it depends on the
> somebody putting in the work to do the auto-cleanup of RPC threads.
> Keith seemed like he knew how that could be implemented, but right
> now, it's beyond me.
>
> For the future:
>
> A better solution might be to localize all connection-state resources
> such that resources are not shared between connection-configuration
> (Instance), a factory (Connector) and the objects it produces
> (BatchWriter, etc.). Objects created by the factory can either be
> closeable themselves to clean up their own resources, or a shared
> connection resource object can be closeable and used by other objects.
> I've been thinking a lot about a new, better, cleaner API, based on
> lessons learned here and elsewhere... and I want to pursue it as part
> of new accumulo-client-api and accumulo-client-runtime modules (JIRA
> is down right now, so I can't recall the relevant ticket number(s)).
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>
>
> On Thu, Dec 19, 2013 at 11:43 PM, Mike Drob <mdrob@mdrob.com> wrote:
> > This conversation is way past dead at this point, but I wanted to respond
> > now that things have had a time to cool off. I didn't really pursue a
> > response last week because I felt like you [Christopher] had unilaterally
> > imposed your will on the rest of us and were unwilling to discuss things
> > further. I was pretty upset and decided to just step back from the
> > conversation completely.
> >
> > For the record, you don't have to keep things rebased, you can do a "git
> > update-index --assume-unchanged" on the closeable files in your workspace
> > and that would magically solve all your warnings. (I love that git is
> > magical).
> >
> > The larger discussion that we need to have is what we do about the API
> > problems, and the long-lived resources. There has been some discussion in
> > IRC, on various JIRAs, and sprinkled across email about the proper
> > solution, but I'm having a hard time mentally merging all of those
> > conversations, so I'll propose that we refocus on it here.
> >
> > What are our invariants? What are the goals? What tools are available to
> > solve the problem?
> >
> > Mike
> >
> >
> > On Fri, Dec 13, 2013 at 4:03 PM, Christopher <ctubbsii@apache.org>
> wrote:
> >
> >> On Fri, Dec 13, 2013 at 4:34 PM, Sean Busbey <
> busbey+ml@clouderagovt.com>
> >> wrote:
> >> > Could you just do #4 as a patch on your own local repo? It should be
> >> > relatively easy to keep rebased given that it's just an interface.
> >> [snip]
> >>
> >> No, I cannot easily keep 3 branches rebase'd, nor do I wish to
> >> constantly rebase and reorder my other work that I wish to push,
> >> waiting on this issue. I do not see an issue with this (essentially)
> >> partial revert, of what was (in my view) incomplete work. The option
> >> that seemed to be favored (by myself, included) was #3. Proceeding
> >> with #4 in the meantime does not preclude #3 from proceeding, but it
> >> does address the immediate issues at hand. We make improvements on
> >> changes all the time. I don't see this as any different.
> >>
> >> --
> >> Christopher L Tubbs II
> >> http://gravatar.com/ctubbsii
> >>
>



-- 
| - - -
| Bill Havanki
| Solutions Architect, Cloudera Government Solutions
| - - -

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