[jira] [Comment Edited] (BOOKKEEPER-411) Add CloseSubscription Request for multiplexing support

["Sijie Guo (JIRA)" <ji...@apache.org>]

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

Sijie Guo edited comment on BOOKKEEPER-411 at 10/11/12 1:22 PM:
----------------------------------------------------------------

ah. I almost missed this comment.

{quote}
However, isn't the same true for TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED? Or at least for TOPIC_MOVED?
{quote}

for TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED, we close the channel now. but after multiplexing, we should not close it, instead we sending a SubscriptionEvent thru the channel to notify the client the event.

NOP introduced here 1) for now, we don't need to close subscription channel. 2) for multiplexing, we don't need to send subscription event to client. so NOP is different is from TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED.

I agreed that NOP is a bad name. SUBSCRIPTION_CLOSED is better. How about making it more exactly, using SUBSCRIPTION_ALREADY_CLOSED, which means the subscription is already closed, so we don't need to close channel now, and don't need to send any subscription event in future for multiplexing.

{quote}
Could you split this bit out into a smaller patch first, and put the subscription closed stuff on top of that then.
{quote}

OK. How about creating a new jira to abstract the SubscriptionChannelManager and this jira generate the closeSubscription part based on that jira? Does it make sense for you?
                
      was (Author: hustlmsp):
    ah. I almost missed this comment.

{quote}
However, isn't the same true for TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED? Or at least for TOPIC_MOVED?
{quote}

for TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED, we close the channel now. but after multiplexing, we should not close it, instead we sending a SubscriptionEvent thru the channel to notify the client the event.

NOP introduced here 1) for now, we don't need to close subscription channel. 2) for multiplexing, we don't need to send subscription event to client. so NOP is different is from TOPIC_MOVED and SUBSCRIPTION_FORCED_CLOSED.

I agreed that NOP is not a bad name. SUBSCRIPTION_CLOSED is better. How about making it more exactly, using SUBSCRIPTION_ALREADY_CLOSED, which means the subscription is already closed, so we don't need to close channel now, and don't need to send any subscription event in future for multiplexing.

{quote}
Could you split this bit out into a smaller patch first, and put the subscription closed stuff on top of that then.
{quote}

OK. How about creating a new jira to abstract the SubscriptionChannelManager and this jira generate the closeSubscription part based on that jira? Does it make sense for you?
                  
> Add CloseSubscription Request for multiplexing support
> ------------------------------------------------------
>
>                 Key: BOOKKEEPER-411
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-411
>             Project: Bookkeeper
>          Issue Type: Sub-task
>          Components: hedwig-client
>            Reporter: Sijie Guo
>            Assignee: Sijie Guo
>             Fix For: 4.2.0
>
>         Attachments: BOOKKEEPER-411.diff
>
>
> for multiplexing, we could not close the underlying channel directly when closing subscription. so we need to close subscription from server side.

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