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

Ivan Kelly commented on BOOKKEEPER-170:
---------------------------------------

{quote}1. I'm not sure why you removed a number of calls to shutdown. Could you 
explain?{quote}
Because there's no need to shutdown if we've never started. Previously 
construction would automatically start the bookie. This is no longer the case.

{quote}2. In Bookie, I thought that we were setting the value of this.zk 
further down in the constructor because we needed to wait until the bookie 
starts up. With this patch, we set it earlier. I'm trying to understand why 
that is not a problem...{quote}
Again, this was due to EntryLogger contructor starting the GC, which is no 
longer the case. Now GC isn't started until we start the entry logger.

{quote}3. This change here "@@ -346,12 +350,11" does not seem to be 
necessary.{quote}
It removes trailing whitespace, which is ugly in reviewboard (shows up as red).

{quote}4. This is minor, but I was wondering if we should call startGC() 
something else, like initiate(). It leaks out information about 
EntryLogger.{quote}
Renamed to #start()
                
> Bookie constructor starts a number of threads
> ---------------------------------------------
>
>                 Key: BOOKKEEPER-170
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-170
>             Project: Bookkeeper
>          Issue Type: Bug
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.1.0
>
>         Attachments: BOOKKEEPER-170.diff
>
>
> Starting a thread in a constructor is bad[1]. Also, it makes unit testing on 
> Bookie a bit of a pain. For this reason, i've refactored the thread starting 
> code out, so that to start the bookie, you call start() like you usually have 
> to for a thread anyhow. As a bonus, it fixes some findbugs issues.
> [1] 
> http://stackoverflow.com/questions/84285/calling-thread-start-within-its-own-constructor

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