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

Ivan Kelly commented on BOOKKEEPER-133:
---------------------------------------

I've taken a closer look at this. In general it looks good. 

As I said before, PersistenceManager#stop should be removed, since it does 
nothing.

SubscribeResponseHandler#messageConsumed shouldn't share a method with 
SubscribeResponseHandler#asyncConsumeBufferedMessages. The shared method spends 
half it's time checking if message is null, so you're not really extracting out 
common code at all.

Also, when you upload these changes, could you put them in reviewboard.
                
> Hub server should update subscription state to zookeeper when losing topic or 
> shutting down
> -------------------------------------------------------------------------------------------
>
>                 Key: BOOKKEEPER-133
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-133
>             Project: Bookkeeper
>          Issue Type: Bug
>          Components: hedwig-server
>    Affects Versions: 4.0.0
>            Reporter: Sijie Guo
>            Assignee: Sijie Guo
>             Fix For: 4.1.0
>
>         Attachments: BOOKKEEPER-133.patch
>
>
> Currently hub server use counter-based mechanism to update subscription state 
> lazily to zookeeper.
> But in the following case, it didn't do it.
> 1) losing ownership of Topic
> 2) hub server shuts down
> 3) a subscription channel disconnected

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