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. > > > > > > > > > > > > > > >