[ https://issues.apache.org/jira/browse/ZOOKEEPER-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13657506#comment-13657506 ]
Hadoop QA commented on ZOOKEEPER-1400: -------------------------------------- -1 overall. Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12583204/ZOOKEEPER-1400.patch against trunk revision 1482318. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 16 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1479//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1479//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-ZOOKEEPER-Build/1479//console This message is automatically generated. > Allow logging via callback instead of raw FILE pointer > ------------------------------------------------------ > > Key: ZOOKEEPER-1400 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1400 > Project: ZooKeeper > Issue Type: Improvement > Components: c client > Affects Versions: 3.5.0 > Environment: Linux > Reporter: Marshall McMullen > Assignee: Marshall McMullen > Fix For: 3.5.0 > > Attachments: ZOOKEEPER-1400.patch, ZOOKEEPER-1400.patch > > > The existing logging framework inside the C client uses a raw FILE*. Using a > FILE* is very limiting and potentially dangerous. A safer alternative is to > just provide a callback that the C client will call for each message. In our > environment, we saw some really nasty issues with multiple threads all > connecting to zookeeper via the C Client related to the use of a raw FILE*. > Specifically, if the FILE * is closed and that file descriptor is reused by > the kernel before the C client is notified then the C client will use it's > static global logStream pointer for subsequent logging messages. That FILE* > is now a loose cannon! In our environment, we saw zookeeper log messages > ending up in other sockets and even in our core data path. Clearly this is > dangerous. In our particular case, we'd omitted a call to > zoo_set_log_stream(NULL) to notify C client that the FILE* has been closed. > However, even with that bug fixed, there's still a race condition where log > messages in flight may be sent before the C client is notified of the FILE > closure, and the same problem can happen. > Other issues we've seen involved multiple threads, wherein one would close > the FILE*, and that's a global change that affects all threads connected > within that process. That's a pretty nasty limitation as well. > My proposed change is to allow setting a callback for log messages. A > callback is used in preference to a raw FILE*. If no callback is set, then it > will fallback to the existing FILE*. If that's not set, then it falls back to > stderr as it always has. > While refactoring this code, I removed the need for the double parens in all > the LOG macros as that wasn't necessary and didn't fit with my new approach. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira