accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ed Coleman" <d...@etcoleman.com>
Subject RE: Proposal for splitting ACCUMULO-1242 into subtasks.
Date Tue, 13 May 2014 12:31:05 GMT
Originally, when I first started work on the ticket, using logback would have been my goal.
 After going through the code a few times, what is driving me now is really to clean things
up a little and position things for an eventual ability to use logback or to upgrade to log4j-2.
 

After seeing how log4j is being used in Accumulo, well it is going to take a while to get
there.  I use a bunch of code that uses logback and it would be nice to manage one logging
framework.  It is not a big hurdle, just a nit.  And something always seems to require log4j
so I end up with both anyway.

I would at least like to provide the ability of client code to use their logging framework
of choice.

I like the message formatting (maybe I'm used to looking at it) and I don't need to remember
to put guards (isDegugEnabled) around log statements.

The marker interface seems promising and I'd like to see how it could be leveraged.

And with a request for internationalized logging in ACCUMULO-2797, well slf4j (http://www.slf4j.org/localization.html)
seems like it would position us to support that too. 

Ed Coleman

(sorry if this gets duplicated, but my first reply seems to be wandering around on the nets
somewhere)

Sent: Tuesday, May 13, 2014 12:18 AM, William Slacum wrote 

Sounds good, Ed.

Just out of curiosity, are you planning on doing this with the goal of being able to swap
out log4j for logback? In personal projects, I like slf4j solely for the message formatting
feature.


On Mon, May 12, 2014 at 10:45 PM, Sean Busbey <busbey@cloudera.com> wrote:

> +1 LGTM
>
> Overall approach looks good, we can deal with details in review.
>
> --
> Sean
> On May 12, 2014 8:49 PM, "Mike Drob" <mdrob@mdrob.com> wrote:
>
> > +1.
> >
> > You've spent more time thinking about this than the rest of us 
> > combined, probably, so if you think this is the best approach I 
> > recommend just
> going
> > for it. If we discover other issues as they crop up, then we can 
> > deal
> with
> > them at that point.
> >
> > Mike
> >
> >
> > On Mon, May 12, 2014 at 9:15 PM, Ed Coleman <dev1@etcoleman.com> wrote:
> >
> > > I am willing to take another run at the Consistent Logging ticket, 
> > > ACCUMULO-1242, but I'd like to achieve a consensus on an approach
> before
> > > starting.
> > >
> > > The tl;dr version - I would like to split ACCUMULO-1242 into subtasks.
> > > Target version would be 1.7.0 (or whatever it gets called, would 
> > > not
> mind
> > > doing it for 1.6.1 too, to ease merges of bug fixes - esp. for the
> "easy"
> > > conversions.
> > >
> > > Now the novel-length version (and sorry for the length)
> > >
> > > I think that the ACCUMULO-1242 should be split into a number of
> subtasks
> > -
> > > at least three or maybe four. This way individual subtasks can be
> > committed
> > > independently to allow a thorough review of the more complex changes.
> The
> > > breakdown that I am thinking of would go from easy, mostly
> non-functional
> > > changes and progressively become more complex and could require
> > rethinking
> > > the way certain things are done for the "hardest" ones.  The 
> > > breakdown would also narrow the number of files effected as the 
> > > subtasks progressed
> from
> > > easy to hard.  The "easy" changes would impact most files, while 
> > > the
> most
> > > complex changes would impact relatively few.
> > >
> > > To be clear, with this approach some files may be changed multiple
> times
> > by
> > > different sub-tasks - in case that influences anyone's opinion to 
> > > this approach.
> > >
> > > The breakdown that I am suggesting as a starting point for 
> > > discussion
> is:
> > >
> > > Subtask-1:
> > >
> > > a) Replace package statements and Logger.getLogger to 
> > > LoggerFactory.getLogger
> > >
> > > b) Use parameterized messages ( {} ) instead of concatenation and
> remove
> > > any
> > > if level enabled tests (.isDebugEnabled(), .isInfoEnabled().)- 
> > > this may provide a very slight performance gain.
> > >
> > > c) Add messages to all exceptions - required by slf4j and 
> > > generally an accepted practice.
> > >
> > > d) Eliminate printStackTrace with log messages of an appropriate 
> > > level
> > > (ACCUMULO-2628 covers this and could be done at the same time.)
> > >
> > > This is the low hanging fruit and should eliminate log4j 
> > > dependencies
> in
> > > most classes - maybe 80% to 90% or more. [Because (c) and (d) will
> > slightly
> > > change the log output, maybe they are more appropriate for 
> > > subtask-2?]
> > > [Question: any issue with changing log statement wording in (b) if 
> > > it improves clarity? - which would also slightly change log output 
> > > which
> > would
> > > break anyone that is doing log scraping.]
> > >
> > > Subtask-2:
> > >
> > > a) Remove FATAL level and replace with MARKER interface supported 
> > > by logback and log4j-2 [a future effort could be to extend MARKER 
> > > usage to allow
> > finer
> > > grained log filtering, but probably not as part of this effort.]
> > >
> > > b) Remove dynamic manipulation of log levels in testing by using 
> > > test-specific parameter files (if desired)
> > >
> > > Subtask-3:
> > >
> > > a) Rework TRACING and log forwarding so they do not have a log4j
> > dependency
> > >
> > > Subtask-4:
> > >
> > > a) Rework shell debug command facility that dynamically changes 
> > > the log level.
> > >
> > > With the current code base it may be impractical to completely 
> > > remove direct log4j dependencies, but we should be able to isolate 
> > > it to a few
> > instances
> > > in the server-side code and completely remove it from the 
> > > client-side
> > code.
> > >
> > > Another thing to note is that many of the limitations of slf4j are
> > present
> > > in log4j-2 -neither allow dynamic log level changes 
> > > programmatically or through DOM manipulation but instead watch the 
> > > property file and react
> > when
> > > it is modified. So, even if you really don't care about slf4j, 
> > > similar changes will be required to upgrade log4j-2.
> > >
> > > Once there is a consensus I (or Christopher ?) could make the 
> > > sub-tasks
> > and
> > > I'll get started.
> > >
> > >
> > >
> > >
> >
>


Mime
View raw message