accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Drob <md...@mdrob.com>
Subject Re: Proposal for splitting ACCUMULO-1242 into subtasks.
Date Tue, 13 May 2014 01:49:12 GMT
+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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message