[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15187815#comment-15187815
]
James Clampffer commented on HDFS-9118:
---------------------------------------
Thanks for the feedback [~bobthansen], I hadn't thought about some of these
things.
{quote}
Add a TRACE level for finer than Debug. My general mapping is
ERROR: Something went wrong that is catastrophic (invalid internal states, etc.)
WARN: Something went wrong that you'll always want to know about, but doesn't
halt operations (recoverable errors, messages out of order, etc.)
INFO: Here's a high-level list of what's going on with libhdfs++ (opening and
closing files, retrying operations, etc.)
DEBUG: Here's a low-level stream of libhfds++ operations (operation by
operation)
TRACE: Here's a packet-by-packet stream of libhdfs++ operations (function
entry/exit, each packet sent, etc.)
{quote}
I'll do this, right now there aren't a whole lot of messages that I'd consider
trace level, but I'll go through and push anything down that looks like it's
more detail than needed for typical debugging. Either way I'll add the
appropriate stuff to handle a trace level.
{quote}
Add a LogEnabled(level, component)-->bool function that can be checked before
constructing the LogMessage. This allows client code to circumvent expensive
logging operations. Make it pluggable so that we can hook it into the current
Google logging settings, etc.
{quote}
Yea it looks like I can have the macro instantiate a dummy object if logging
isn't enabled. I was thinking of putting this off until I had a chance to do
some decent profiling, but it doesn't look hard to extend the macros. This
still ends up evaluating any function calls in the log, but at least gets rid
of the stringstream allocations and LogManager::Write (which grabs a lock)
calls.
I'll look into having the LogManager delegate to a plugin. Depending on how
large the patch starts getting I might save that for later.
{quote}
Logging should be disabled by default. This is a low-level library; it
shouldn't spew if not asked to
{quote}
Good point. It was getting annoying to look through all the logs for the tests
I'm writing.
{quote}
Can we make the LogMessage a real ostream? That way consumers could do
LogMessage(...) << hex << my_pointer rather than LogHelpers::PointerToHex(...).
It's a bit more idiomatic, but I don't want to create a lot more work.
{quote}
It looks implementing a new ostream correctly involves a fair amount of work
(according to stackoverflow). For now I think I can just overload
operator<<(void*) to deal with hex, and put in a operator<<(const char *) to
specialize for c style strings. Does that sound like a decent solution?
{quote}
Rather than require the consumers to call hdfsFreeLogData, always have the
library free the object. If the consumer wants to retain the data, they can
copy the structure. We could provide an hdfsCopyLogData method if we wanted to
be very nice.
{quote}
This sounds like a good improvement, I assume most people just want to get the
info to stick into their own log systems on the C side. This will also help
cut back on heap allocations with short lives.
> Add logging system for libdhfs++
> --------------------------------
>
> Key: HDFS-9118
> URL: https://issues.apache.org/jira/browse/HDFS-9118
> Project: Hadoop HDFS
> Issue Type: Sub-task
> Components: hdfs-client
> Affects Versions: HDFS-8707
> Reporter: Bob Hansen
> Assignee: James Clampffer
> Attachments: HDFS-9118.HDFS-8707.000.patch,
> HDFS-9118.HDFS-8707.001.patch
>
>
> With HDFS-9505, we've starting logging data from libhdfs++. Consumers of the
> library are going to have their own logging infrastructure that we're going
> to want to provide data to.
> libhdfs++ should have a logging library that:
> * Is overridable and can provide sufficient information to work well with
> common C++ logging frameworks
> * Has a rational default implementation
> * Is performant
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
