accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From William Slacum <wilhelm.von.cl...@accumulo.net>
Subject Re: Resource leak warnings
Date Mon, 23 Dec 2013 08:34:25 GMT
We're pretty clear on commit-then-review and lazy consensus, so I don't
really have an issue with regards to the commits.

That said, I still think ignoring the warnings is the best course of
action. I compiled with warnings on from the command line and don't see a
resource leak warning with Java 6. We voted not to use Java 7, so this
shouldn't be an issue until we make that move.

This is what I did to check if those warnings were present when building
from the command line. If this isn't sufficient, please let me know.

1) `git revert 335f693a4045d2c2501e2ed6ece0493734093143`
2) Added the following to the configuration block for the
maven-compiler-plugin:

          <compilerArgument>-Xlint:all</compilerArgument>
          <showWarnings>true</showWarnings>
          <showDeprecation>true</showDeprecation>
3) `mvn clean compile | grep -i leak`



On Sun, Dec 22, 2013 at 10:28 PM, Christopher <ctubbsii@apache.org> wrote:

> On Sun, Dec 22, 2013 at 2:23 PM, Bill Havanki <bhavanki@clouderagovt.com>
> 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
> at some point), but he and I have discussed this a bit, and I get the
> distinct impression that he thinks it should not be there.
>
> > 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.
>
> It's my understanding that these aren't Eclipse warnings, these are
> default JDK1.6 compiler warnings. I could be wrong here... they may
> need "javac -Xlint:all", or some other flag, to show up. In any case,
> whether it is Eclipse, or FindBugs, or some other tool reporting
> potential problems, I'm not concerned about them for aesthetics... I'm
> concerned because they hint at potential areas of improvements or
> bugs, that we should inspect with due diligence, and when they become
> numerous, it's hard to actually tell the difference between a non-bug
> warning that we've ignored and an actual bug warning that we've not
> examined yet.
>
> In any case, the point is moot here, because even if it didn't produce
> a warning, the current implementation does not warrant giving
> incorrect information to the API consumer that it can/should be
> closed, in accordance with Closeable's semantics (as in the case of
> the currently broken MapReduce configuration code... See comment on
> ACCUMULO-1923, which affects our code, and any subclasses of the
> Input/OutputFormat). I would even go so far as to say that this
> warning actually reflects an API bug: Instance does not actually
> conform to Closeable's semantics... because it doesn't free resources
> held by Instance... it frees static resources held elsewhere, and that
> becomes obvious when we actually try to close it in accordance with
> the semantics of Closeable, so it shouldn't be marked as such (until
> we write the code to make it conform to those semantics).
>
> > 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
> [snip]
>
> Granted, yes, absolutely, agreed, and so on :)
> (to be clear, when I say "committer norms", I mean of the CtR type...
> it's unilateral to a point, until an objection from review)
>
> --
> Christopher L Tubbs II
> http://gravatar.com/ctubbsii
>

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