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 <bus...@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" <md...@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 <d...@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.
> > >
> > >
> > >
> > >
> >
>

Reply via email to