[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16410416#comment-16410416 ] Hudson commented on HDFS-9118: -- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #13869 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/13869/]) HDFS-9118: libhdfs++ Add logging system. Contributed by James Clampffer (james.clampffer: rev 0f1a278dd5f2d3cafee240f2c2795ac0644410a2) * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filehandle.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/CMakeLists.txt * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/bindings/c/hdfs.cc * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/log.h * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/tests/logging_test.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/hdfs_public_api.cc * (add) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/logging.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/reader/block_reader.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_engine.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/logging.h * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/include/hdfspp/hdfs_ext.h * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/fs/filesystem.cc * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/common/CMakeLists.txt * (edit) hadoop-hdfs-project/hadoop-hdfs-native-client/src/main/native/libhdfspp/lib/rpc/rpc_connection.h > 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 >Priority: Major > Attachments: HDFS-9118.HDFS-8707.000.patch, > HDFS-9118.HDFS-8707.001.patch, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.patch, > HDFS-9118.HDFS-8707.004.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 (v7.6.3#76005) - To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15209270#comment-15209270 ] James Clampffer commented on HDFS-9118: --- Thanks for the reviews Bob. I'll open a jira for tying together ReportError and this. I committed this to HDFS-8707. > 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, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.patch, > HDFS-9118.HDFS-8707.004.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15209231#comment-15209231
]
Hadoop QA commented on HDFS-9118:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 18s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
28s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 34s
{color} | {color:green} HDFS-8707 passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 33s
{color} | {color:green} HDFS-8707 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 16s
{color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
11s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 40s
{color} | {color:green} the patch passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 40s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 40s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 47s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 47s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 47s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 14s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 46s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.8.0_74. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 50s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
22s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 42m 25s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:0cf5e66 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12795055/HDFS-9118.HDFS-8707.004.patch
|
| JIRA Issue | HDFS-9118 |
| Optional Tests | asflicense compile cc mvnsite javac unit |
| uname | Linux 6d9dd2d66875 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HDFS-8707 / 7751507 |
| Default Java | 1.7.0_95 |
| Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_74
/usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
| JDK v1.7.0_95 Test Results |
https://builds.apache.org/job/PreCommit-HDFS-Build/14913/testReport/ |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-native-client U:
hadoop-hdfs-project/hadoop-hdfs-native-client |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/14913/console |
| Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
> 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:
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15209202#comment-15209202 ] Bob Hansen commented on HDFS-9118: -- +1. Looks good. Can you file a new Jira and reference it here to cover moving the hdfs.cc ReportError code to the new logging system? > 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, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.patch, > HDFS-9118.HDFS-8707.004.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15208701#comment-15208701
]
James Clampffer commented on HDFS-9118:
---
Didn't see you had more comments above my last message, thanks for the very
detailed feedback, let me give those a shot.
bq. If log.h is going to be included in libhdfspp_ext, it should have its own
extern "C" blocks to make sure that C++ idioms don't creep in.
Good idea. I should be able to add this.
bq. Should LogData be part of hdfspp_ext.h rather than log.h? It seems to be
specific to the CForwardingLogger
Most likely. I can't remember offhand I left it in there to break up some
dependency issues or I just forgot about it. If it's the latter I'll move it.
bq. In isComponentValid/isLogLevel valid, we should declare a MAX_LOG_LEVEL and
MAX_COMPONENT in log.h so that it is tied in code closer to where it is
declared.
I can do this if I'm in the area while fixing other things. I don't see it as
being too critical in the short term and I think longer term the #defines
should get replaced with C enums, but that's a bunch of work to do and I'd like
to get this in fairly soon since it's proven useful as-is.
bq. Are we allowing multiple components to be specified in enableComponent,
disableComponent? If so, the upper limit on our bounds check should be
(MAX_COMPONENT << 1) - 1. If not, we should check that only one bit is set in
isComponentValid
It's intended to allow only 1 component at a time, I can add a popcount and
error/no-op if >1 bits are set.
bq. We might want to move ShouldLog into the header so it can be inlined
This is a good point, I was hoping -flto would be smart enough to handle this
but a quick look says support varies a lot by platform even with simple
functions.
bq. Is there a reason for the two distinct .reset calls in ::SetLoggerImpl
rather than just one?
The compiler was being grumpy when I first wrote it as a single one (I forget
why) so I just wrote two and moved on. I'll take a look at this, but if it
looks like it will take a significant amount of time to figure out while the
compiler is complaining I'm just going to leave it; it's pretty understandable
and is nearly free.
bq. std::asctime is deprecated and not thread-safe. We should use std::strftime
which is less straightforward but safer
I'll check this out, didn't realize it wasn't thread safe. May hold off on
this until the implementation changes enough that the logger plugins really can
get called by multiple threads.
bq. For null pointer output, as a consumer I would prefer to see just "nullptr"
or "NULL" rather than including the type of the null pointer in the output
Hm. This might be a matter of preference; I wanted to know the type in case it
could have been ambiguous. If adding the __FILE__/__LINE__ stuff looks like it
will disambiguate in most cases I'll get rid of the type.
bq. As part of HDFS-9616, I've added the cluster and filename to the relevant
objects. We should follow-up and either add them to the LogMessage
macros/object or to the output messages.
I'm going to leave this out for now to prevent the scope of this patch from
creeping too much. It seems like it would be fitting to do this when
integrating the C API ReportError with the logging system.
bq. In the logging test, it is good form to have each test set itself up and
tear itself down rather than putting the setup code in main. Either make it a
class with a SetUp method or add an RAII object to the top of each test to do
the register/unregister
This was done to minimize boilerplate and definitions of single use objects. I
think the current implementation is maintainable as-is, but if much more stuff
is added the cost of defining RAII objects would be amortized enough that I
think it'd be good to have.
{quote}
As a consumer, I would like to see more information in the Debug level between
Trace and Info.
All object's ctors and dtors (with "this" pointer)
Anything that happens more than once-per-file but less than once-per-block. I
might suggest:
FileHandleImpl::PositionRead
FileHandleImpl::Read
FileHandleImpl::Seek
Should we include the per-block operations (past BlockReader ctor/dtor)?
Anything else that's interesting here?
{quote}
(Not sure how to maintain list formatting in quotes..)
In order do track all ctor/dtor calls I'd like to make some RAII member object
that takes up no space in the class to handle it. I'll take a look at what I
can do with some macros, if it looks like it will be a pain I'm going to push
it off into another improvement jira. What I want is something like:
{code}
// Drop this into any class, will determine class name at build time (somehow,
doesn't look like __CLASS__ exists)
// and print class + this* on construction and destruction (instantiate some
RAII object).
#define TRACK_OBJECT_LIFETIME ... some stuff
class
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15208612#comment-15208612 ] James Clampffer commented on HDFS-9118: --- bq. Before we commit it, do you think we should add a _FILE_ and _LINE_ capture to the macro? I can imagine the additional default context could be handy, and it is very cheap at runtime. Sure; I'll capture it but won't print it by default in the StderrLogger to keep things a bit easier to read when skimming through. bq. We might also be able to use SFINAE (https://en.wikipedia.org/wiki/Substitution_failure_is_not_an_error) to detect if "this" is valid in the current context and capture this as a void pointer (which has been Very Helpful in recent debugging, especially for lifetime issues). I'd like to hold off on SFINAE simply because I'm not too experienced with it and I don't want to turn this into a playground for learning template magic. This patch is picking up on 'this' for the block reader using the FMT_THIS_AND macros but I can also add it to the RPC code where applicable. > 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, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15208411#comment-15208411 ] Bob Hansen commented on HDFS-9118: -- Before we commit it, do you think we should add a __FILE__ and __LINE__ capture to the macro? I can imagine the additional default context could be handy, and it is very cheap at runtime. We might also be able to use SFINAE (https://en.wikipedia.org/wiki/Substitution_failure_is_not_an_error) to detect if "this" is valid in the current context and capture this as a void pointer (which has been Very Helpful in recent debugging, especially for lifetime issues). > 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, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15206527#comment-15206527 ] Bob Hansen commented on HDFS-9118: -- I like the look of that patch, [~James Clampffer]. A few more comments to take or leave as you choose: * If log.h is going to be included in libhdfspp_ext, it should have its own extern "C" blocks to make sure that C++ idioms don't creep in. * Should LogData be part of hdfspp_ext.h rather than log.h? It seems to be specific to the CForwardingLogger * In isComponentValid/isLogLevel valid, we should declare a MAX_LOG_LEVEL and MAX_COMPONENT in log.h so that it is tied in code closer to where it is declared. * Are we allowing multiple components to be specified in enableComponent, disableComponent? If so, the upper limit on our bounds check should be (MAX_COMPONENT << 1) - 1. If not, we should check that only one bit is set in isComponentValid * We might want to move ShouldLog into the header so it can be inlined * Is there a reason for the two distinct .reset calls in ::SetLoggerImpl rather than just one? * std::asctime is deprecated and not thread-safe. We should use std::strftime which is less straightforward but safer * For null pointer output, as a consumer I would prefer to see just "nullptr" or "NULL" rather than including the type of the null pointer in the output * As part of HDFS-9616, I've added the cluster and filename to the relevant objects. We should follow-up and either add them to the LogMessage macros/object or to the output messages. * In the logging test, it is good form to have each test set itself up and tear itself down rather than putting the setup code in main. Either make it a class with a SetUp method or add an RAII object to the top of each test to do the register/unregister * As a consumer, I would like to see more information in the Debug level between Trace and Info. ** All object's ctors and dtors (with "this" pointer) ** Anything that happens more than once-per-file but less than once-per-block. I might suggest: *** FileHandleImpl::PositionRead *** FileHandleImpl::Read *** FileHandleImpl::Seek *** Should we include the per-block operations (past BlockReader ctor/dtor)? *** Anything else that's interesting here? * I think FileHandler::CancelOperations should be at the Info level None of those are show-stoppers, but I think some of them would make the first pass a bit better. > 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, HDFS-9118.HDFS-8707.002.patch, > HDFS-9118.HDFS-8707.003.patch, HDFS-9118.HDFS-8707.003.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15206193#comment-15206193
]
Hadoop QA commented on HDFS-9118:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 12s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m
44s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 40s
{color} | {color:green} HDFS-8707 passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 31s
{color} | {color:green} HDFS-8707 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 15s
{color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
11s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 32s
{color} | {color:green} the patch passed with JDK v1.8.0_74 {color} |
| {color:red}-1{color} | {color:red} cc {color} | {color:red} 22m 19s {color} |
{color:red} hadoop-hdfs-project_hadoop-hdfs-native-client-jdk1.8.0_74 with JDK
v1.8.0_74 generated 1 new + 28 unchanged - 1 fixed = 29 total (was 29) {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 32s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 32s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 53s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 53s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 53s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 12s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 32s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.8.0_74. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 17s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 40m 31s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:0cf5e66 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12794733/HDFS-9118.HDFS-8707.003.patch
|
| JIRA Issue | HDFS-9118 |
| Optional Tests | asflicense compile cc mvnsite javac unit |
| uname | Linux ea649427e419 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HDFS-8707 / 7751507 |
| Default Java | 1.7.0_95 |
| Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_74
/usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
| cc | hadoop-hdfs-project_hadoop-hdfs-native-client-jdk1.8.0_74:
https://builds.apache.org/job/PreCommit-HDFS-Build/14891/artifact/patchprocess/diff-compile-cc-hadoop-hdfs-project_hadoop-hdfs-native-client-jdk1.8.0_74.txt
|
| JDK v1.7.0_95 Test Results |
https://builds.apache.org/job/PreCommit-HDFS-Build/14891/testReport/ |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-native-client U:
hadoop-hdfs-project/hadoop-hdfs-native-client |
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15205581#comment-15205581
]
Hadoop QA commented on HDFS-9118:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 0s
{color} | {color:blue} Docker mode activated. {color} |
| {color:red}-1{color} | {color:red} docker {color} | {color:red} 7m 35s
{color} | {color:red} Docker failed to build yetus/hadoop:0cf5e66. {color} |
\\
\\
|| Subsystem || Report/Notes ||
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12794615/HDFS-9118.HDFS-8707.003.patch
|
| JIRA Issue | HDFS-9118 |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/14884/console |
| Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
> 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, HDFS-9118.HDFS-8707.002.patch,
> HDFS-9118.HDFS-8707.003.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15198193#comment-15198193
]
James Clampffer commented on HDFS-9118:
---
bq. The downside of this approach is that all of the functions calls that
construct the log message will still be called and construct return strings
only to throw them away
I agree. I just did some profiling today and a decent chunk of time gets spent
constructing std::stringstream owned by LogMessage, so that needs to be fixed.
{quote}
Does logging.h require hdfspp/hdfs_ext.h for anything other than the LogData
declaration (used only by the CForwardingLogger)?
Can we move the CForwardingLogger into hdfs.cc? Since it's C-API-specific, it
seems to be more tightly bound there than in the logging source file.
{quote}
I also needs the #defines. I can separate out a header and have hdfs_ext.h and
logging.h both include that to decouple things. I can also move the
CForwardingLogger.
{quote}
In the static log manager methods, we make calls to the logger_impl, calls that
are explicitly forbidden from being virtual. While I'm not keen on that design
choice (premature optimization and all that), why not just put the variables
and code right in the LogManager. Implementations can't override the semantics
of the calls, so it doesn't appear that we're not gaining any flexibility by
dereferencing the logger_impl, and there will some (minimal) runtime impact
(along with increasing the size and complexity of the code). The cleans up the
LoggerInterface to a single pure virtual method (which is very nice).
{quote}
Good point, I was going to move them into LogManager in the last round and I
guess I forgot.
bq. I would nix the GET_IMPL_LOCK macro and just copy/paste the code in the few
places where it needs to be. More explict that way, at the cost of evil
copypasta.
Sure, I think this was another remnant of when I had a lot more methods getting
locks or something like that. Not really that useful now.
{quote}
Upon reflection, do we have a strong use case for having different log levels
for different components, and enabling and disabling by component? Marking with
a component for identification I can understand.
{quote}
My main use case is for debugging, and I've been doing a lot of that. I've
used it quite a bit lately to focus on what could be a nasty bug in the RPC
engine; it lets me avoid grepping through the logs. It's also really
inexpensive to do so I don't see it hurting anything. Could always mark it as
unsupported or something like that.
bq. By my reading, the log currenlty implemented log manager does provide both
of those guarantees to the CForwardingLogger.
At the moment it does do both of those. In an earlier design I had it didn't
but I left those comments to leave some room in case I wanted to change things
around. I can strip them out but I don't see making the interface slightly
more restrictive for the client and implementation more flexible as being too
bad of a trade.
bq. Should the CForwardingLogger have a const * to the callback set in the ctor
rather than a setter? We could then get rid of the !callback_ checks.
It wouldn't hurt to have the setter create a new CForwardingLogger when the
client sets a new callback. The main reason I did this is so that I can pass a
null pointer to flush out the old callback and disable logging. I'm not a big
fan of having to define no-op callbacks, particularly for stuff with weird
signatures, to get that same effect. I'm not too concerned about the runtime
cost of the check (branch should be super predictable). My preference would be
to keep it how it is.
bq. Should LogMessage& LogMessage::operator<<(const std::string *str) log
"nullptr" if str is null?
Yea, that's a good point. I'll change that. Will do the same for the const
char * version as well.
{quote}
In logging.h, we've commented out:
//std::cerr << "msg ctor called, worth_reporting_=" << worth_reporting_
// << "level = " << level_string() << ", component=" << component_string() <<
std::endl;
It should be removed.
{quote}
Good catch, will fix that and the write to cerr you caught above.
bq. Can we tie ReportError in hdfs.cc to the logging system?
For sure, that would make things really clean. I'd like to push that into
another jira just so I can land this soon and minimize how much I have to
rebase before I do though.
> 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,
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15198029#comment-15198029
]
Bob Hansen commented on HDFS-9118:
--
James - thanks for carrying that forward and festooning our code with
appropriate messages. It has already proven useful.
The current implementation always constructs a LogMessage but turns the <<
operators into no-ops if the logging level is too low to be displayed. The
downside of this approach is that all of the functions calls that construct the
log message will still be called and construct return strings only to throw
them away. We tried to come up with a clever templating method to get rid of
that, but I think we should still take it on as a use-case and solve with
macros. If we construct the logging macros as:
#define LOG_TRACE (C,MSG) { if (LogManager::ShouldLog(kTrace ,C)) {
LogMessage(kTrace ) MSG; } }
#define LOG_DEBUG (C,MSG) { if (LogManager::ShouldLog(kDebug ,C)) {
LogMessage(kDebug ) MSG; } }
#define LOG_INFO (C,MSG) { if (LogManager::ShouldLog(kInfo ,C)) {
LogMessage(kInfo ) MSG; } }
#define LOG_WARNING(C,MSG) { if (LogManager::ShouldLog(kWarning,C)) {
LogMessage(kWarning) MSG; } }
#define LOG_ERROR (C,MSG) { if (LogManager::ShouldLog(kError ,C)) {
LogMessage(kError ) MSG; } }
The consumer code looks like:
LOG_WARNING(kRPC, << "RPC response with Unknown call id " << h.callid());
This gives us the advantage of clean code but skips the performance hit of
constructing useless data.
LoggerIntf::SetLogLevel:
std::cerr << should probably be removed
Does logging.h require hdfspp/hdfs_ext.h for anything other than the LogData
declaration (used only by the CForwardingLogger)?
Can we move the CForwardingLogger into hdfs.cc? Since it's C-API-specific, it
seems to be more tightly bound there than in the logging source file.
In the static log manager methods, we make calls to the logger_impl, calls that
are explicitly forbidden from being virtual. While I'm not keen on that design
choice (premature optimization and all that), why not just put the variables
and code right in the LogManager. Implementations can't override the semantics
of the calls, so it doesn't appear that we're not gaining any flexibility by
dereferencing the logger_impl, and there will some (minimal) runtime impact
(along with increasing the size and complexity of the code). The cleans up the
LoggerInterface to a single pure virtual method (which is very nice).
I would nix the GET_IMPL_LOCK macro and just copy/paste the code in the few
places where it needs to be. More explict that way, at the cost of evil
copypasta.
Upon reflection, do we have a strong use case for having different log levels
for different components, and enabling and disabling by component? Marking
with a component for identification I can understand.
>From hdfs_ext.h:
* Unlike the C++ logger this will not filter by level or component,
* it is up to the consumer to throw away messages they don't want.
*
* Note: The callback provided must be reentrant, the library does not
guarentee
* that there won't be concurrent calls.
By my reding, the log currenlty implemented log manager does provide both of
those guarantees to the CForwardingLogger.
Should the CForwardingLogger have a const * to the callback set in the ctor
rather than a setter? We could then get rid of the !callback_ checks.
Should LogMessage& LogMessage::operator<<(const std::string *str) log "nullptr"
if str is null?
In logging.h, we've commented out:
//std::cerr << "msg ctor called, worth_reporting_=" << worth_reporting_
// << "level = " << level_string() << ", component=" <<
component_string() << std::endl;
It should be removed.
Can we tie ReportError in hdfs.cc to the logging system?
> 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, HDFS-9118.HDFS-8707.002.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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15189643#comment-15189643 ] James Clampffer commented on HDFS-9118: --- Disregard the stuff about template instantiations not happening, I'm not sure what I was thinking. > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15189499#comment-15189499
]
James Clampffer commented on HDFS-9118:
---
bq. Great. If we can, let's implement the plugins for the C-API glue logger and
the stream/stderr logger. If we want to be super-cool, we could include example
plug-ins for google logging (but we don't want to have a dependency here).
I'm going to do the stderr + C api glue for now. I'll file a new jira for a
google logging plugin that we can keep in the examples directory to avoid
pulling too many third party bits into the core library. I think the stderr
and C interfaces should be somewhat special cases in that they should be able
to be turned on and off from the C API without doing any sort of rebuild.
At some point I'd also like to make a C++ plugin that can take pure C plugin
(struct of function pointers), mostly to keep as much parity between the C and
C++ interface, also because I really like jump tables.
bq. What data types to we need to support in the first pass? I would say all
the fundamental types http://en.cppreference.com/w/cpp/language/types and
special handling for const char * and const std::string &.
So for the first pass I'm thinking of supporting the following (some shortcuts
for brevity)
{code}
//using the widest (practical) type for integers so that any narrower integer
types can be widened and printed correctly
msg::operator<<(int64_t val)
msg::operator<<(uint64_t val)
//need to specialize for bool so it doesn't get converted to an integer type
and we can do nice "true"/"false"
msg::operator<<(bool)
//anything, except const char *, that can be coerced into void* gets printed as
hex. const char * gets it's own method to print as a string
msg::operator<<(void*)
//naturally strings, and const char* get printed as strings
msg::operator<<(const string&)
msg::operator<<(const string*)
msg::operator<<(const char*)
{code}
{code}
template
msg::operator<<(const T& val) {
do something with std::to_string(val)
}
{code}
I think this should only work for people who are statically linking the
library; in the dynamic library case the template instantiatiation is already
done so they don't benefit. I can go either way here; it's easy enough to add.
I think it's a small enough library that most people will be doing static
linkage but it might be odd for methods to stop working if the client changes
the build.
I was thinking more about this and I think a decent solution may be to add an
interface that looks like:
{code}
struct ExpensiveToSerialize {
virtual std::string ToLogString() = 0; //undefined behavior if
implementation has side effects!
}
{code}
This would allow us to hold on to a reference and only materialize the string
after the level and component checks have decided it's worth doing. Since
pruning is done on the same stack we don't need to worry about taking a
shared_ptr. This isn't an ideal solution because classes need to get some
boilerplate code or implement a operator(std::string)() to get around it, but
should let the dynamic library case work + cut down on string/stringstream
operations. I wasn't planning on including it in this JIRA though. Any
thoughts?
> 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15187915#comment-15187915 ] Bob Hansen commented on HDFS-9118: -- Great. If we can, let's implement the plugins for the C-API glue logger and the stream/stderr logger. If we want to be super-cool, we could include example plug-ins for google logging (but we don't want to have a dependency here). What data types to we need to support in the first pass? I would say all the fundamental types http://en.cppreference.com/w/cpp/language/types and special handling for const char * and const std::string &. I think we should be able to have a fallback << foo that calls std::to_string(foo) that covers many cases. > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15187470#comment-15187470 ] Bob Hansen commented on HDFS-9118: -- [~James Clampffer] - thanks for moving this forward. I would like to propose a few changes to the implementation: * 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.) * 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. * Most of the LogManager methods should be delegated to a logging plug-in: - Make a streamlogger plug-in that spits the messages to a stream/stdout/stderr - Make a C API logger that supports the exposed C API. SetHook/ClearHook should be part of that implementation - EnableLogForComponent/DisableLogForComponent should be the domain of the plug-in. LogEnabled(level,component) will ask the plug-in if we should log this thing. Some plug-ins may want to disable certain components, others wont. * Make the macros check LogEnabled first, and return a null-logging object that will compile but no-op with all of the elements * Logging should be disabled by default. This is a low-level library; it shouldn't spew if not asked to * logging.h should not depend on hdfs_ext.h; the C API should plug into the logging to support the C glue * 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. C API: * 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 * Rather than have the logging hooks exposed in the LogManager, make a new plug-in that is used by the C API that does the translation from C++-land to the C API. > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15186291#comment-15186291
]
Hadoop QA commented on HDFS-9118:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 12m 5s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 15m
42s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 15s
{color} | {color:green} HDFS-8707 passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 15s
{color} | {color:green} HDFS-8707 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 18s
{color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
15s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 13s
{color} | {color:green} the patch passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
9s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 4m 58s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.8.0_74. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 2s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 58m 4s {color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:0cf5e66 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12792102/HDFS-9118.HDFS-8707.001.patch
|
| JIRA Issue | HDFS-9118 |
| Optional Tests | asflicense compile cc mvnsite javac unit |
| uname | Linux 4f5734a30ca2 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HDFS-8707 / 8da3bbd |
| Default Java | 1.7.0_95 |
| Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_74
/usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
| JDK v1.7.0_95 Test Results |
https://builds.apache.org/job/PreCommit-HDFS-Build/14744/testReport/ |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-native-client U:
hadoop-hdfs-project/hadoop-hdfs-native-client |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/14744/console |
| Powered by | Apache Yetus 0.2.0 http://yetus.apache.org |
This message was automatically generated.
> 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:
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15181341#comment-15181341
]
Hadoop QA commented on HDFS-9118:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 16m 22s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 8m
22s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 20s
{color} | {color:green} HDFS-8707 passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 22s
{color} | {color:green} HDFS-8707 passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 17s
{color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
12s {color} | {color:green} HDFS-8707 passed {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m
10s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 22s
{color} | {color:green} the patch passed with JDK v1.8.0_74 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 22s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 22s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed with JDK v1.7.0_95 {color} |
| {color:green}+1{color} | {color:green} cc {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 4m 18s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 12s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
9s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} Patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 21s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.8.0_74. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 5m 15s
{color} | {color:green} hadoop-hdfs-native-client in the patch passed with JDK
v1.7.0_95. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
20s {color} | {color:green} Patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 56m 13s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:0cf5e66 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12791547/HDFS-9118.HDFS-8707.000.patch
|
| JIRA Issue | HDFS-9118 |
| Optional Tests | asflicense compile cc mvnsite javac unit |
| uname | Linux 604840463e95 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | HDFS-8707 / 8da3bbd |
| Default Java | 1.7.0_95 |
| Multi-JDK versions | /usr/lib/jvm/java-8-oracle:1.8.0_74
/usr/lib/jvm/java-7-openjdk-amd64:1.7.0_95 |
| JDK v1.7.0_95 Test Results |
https://builds.apache.org/job/PreCommit-HDFS-Build/14722/testReport/ |
| modules | C: hadoop-hdfs-project/hadoop-hdfs-native-client U:
hadoop-hdfs-project/hadoop-hdfs-native-client |
| Console output |
https://builds.apache.org/job/PreCommit-HDFS-Build/14722/console |
| Powered by | Apache Yetus 0.3.0-SNAPSHOT http://yetus.apache.org |
This message was automatically generated.
> Add logging system for libdhfs++
>
>
> Key: HDFS-9118
> URL: https://issues.apache.org/jira/browse/HDFS-9118
> Project: Hadoop HDFS
> Issue Type: Sub-task
>
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15115421#comment-15115421 ] James Clampffer commented on HDFS-9118: --- I'm going to take a shot at this. Things I'm planning on picking up implicitly in addition to the log message and level: -id of thread doing the logging -stack address of the logging function (add a local variable and grab its address) -line number, file name, function > 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 > > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15115434#comment-15115434 ] Bob Hansen commented on HDFS-9118: -- Grabbing the stack address is a fairly expensive operation. I would limit it to opt-in in rare circumstances, and perhaps when logging an error. > 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 > > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[ https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15116065#comment-15116065 ] James Clampffer commented on HDFS-9118: --- Shouldn't getting the stack address of a local variable boil down (at least on x86) to just reading whats in ESP-a constant offset? That should be doable in a few cycles, superscalar complications aside, unless I'm missing something. Either way good idea on allowing things to opt-in based on logging levels. I'll add that. > 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 > > 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)
[jira] [Commented] (HDFS-9118) Add logging system for libdhfs++
[
https://issues.apache.org/jira/browse/HDFS-9118?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=14901630#comment-14901630
]
Haohui Mai commented on HDFS-9118:
--
The interfaces of logging class are quite closed to the one used in snappy and
glog. A rational choice is to make it an abstract class and allow users to
specify the instance in the {{Options}} instance.
> 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
>
> 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)
