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

Patrick Hunt commented on ZOOKEEPER-1442:
-----------------------------------------

This is a great idea and a very nice patch. Thanks! However I do have a few 
comments.

1) indeed the line length formatting needs to be fixed, 80 char max (some 
wiggle room but generally 80 is max)

2) the default should be log and exit for 3.5. not exiting is really a bug, and 
since it's configurable I think it's fine to be b/w compatible when it comes to 
the new release. (so you'd have two patches that are slightly different, that's 
ok). I would mark this jira as "incompatible" and add a release note to this 
jira for inclusion in the release notes of the effected releases

3) this is a bigger comment. You shouldn't be using constants to mark "logonly" 
"logandexit" etc.... Rather you should be configuring which class to handle the 
strategy.

a) Define an interface for this strategy
b) Implement three default classes to be available for use in the config file. 
(log/logexit/none)
c) document this (intf and concrete classes) in the guide

the nice thing about this approach is say that someone comes along and needs a 
fourth option. Say it's osgi specific, or "page me", etc... They can implement 
their own class and then just specify that strategy in the config file. It will 
be much more adaptable going forward.
                
> Uncaught exception handler should exit on a java.lang.Error
> -----------------------------------------------------------
>
>                 Key: ZOOKEEPER-1442
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1442
>             Project: ZooKeeper
>          Issue Type: Bug
>          Components: java client, server
>    Affects Versions: 3.4.3, 3.3.5
>            Reporter: Jeremy Stribling
>            Assignee: Jeremy Stribling
>            Priority: Minor
>         Attachments: ZOOKEEPER-1442.patch, ZOOKEEPER-1442.patch
>
>
> The uncaught exception handler registered in NIOServerCnxnFactory and 
> ClientCnxn simply logs exceptions and lets the rest of ZooKeeper go on its 
> merry way.  However, errors such as OutOfMemoryErrors should really crash the 
> program, as they represent unrecoverable errors.  If the exception that gets 
> to the uncaught exception handler is an instanceof a java.lang.Error, ZK 
> should exit with an error code (in addition to logging the error).

--
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