[ 
https://issues.apache.org/jira/browse/ZOOKEEPER-1400?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215748#comment-13215748
 ] 

Marshall McMullen commented on ZOOKEEPER-1400:
----------------------------------------------

Thanks for the great feedback. I absolutely agree that the logging should be on 
a connection basis. I was hesitant to make an even more drastic change than I 
already had. Do you think changing the logging to be per-connection should be a 
separate JIRA or does it make sense to do it as part of this case as well?
                
> 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.4.2
>         Environment: Linux
>            Reporter: Marshall McMullen
>             Fix For: 3.4.2
>
>         Attachments: case-2739.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: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira


Reply via email to