You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/05/18 10:40:20 UTC

[GitHub] [pulsar] codelipenghui opened a new pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

codelipenghui opened a new pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977


   Fixes #6554
   
   ### Motivation
   
   Introduce messages fence mechanism for Key_Shared subscription to guarantee ordering dispatching.
   
   The fenced message means that the messages can't be delivered at this time. In Key_Shared subscription, the new consumer needs to wait for all messages before the first message that delivers to this consumer are acknowledged. The `fencedMessagesContainer` maintains all fenced messages by a map (mark-delete-position -> fenced messages).  When the mark delete position of the cursor is greater than the mark-delete-position in the `fencedMessagesContainer`, it means the fenced messages will be lifted.
   
   ### Modifications
   
   *Describe the modifications you've done.*
   
   ### Verifying this change
   
   New unit tests added.
   
   ### Does this pull request potentially affect one of the following parts:
   
   *If `yes` was chosen, please highlight the changes*
   
     - Dependencies (does it add or upgrade a dependency): (no)
     - The public API: (no)
     - The schema: (no)
     - The default values of configurations: (no)
     - The wire protocol: (no)
     - The rest endpoints: (no)
     - The admin cli options: (no)
     - Anything that affects deployment: (no)
   
   ### Documentation
   
     - Does this pull request introduce a new feature? (no)
     - If yes, how is the feature documented? (docs)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] feeblefakie commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
feeblefakie commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-634546905


   @codelipenghui 
   Just a question.
   So this and #6791 are solving the same issue with different approaches but the both will be applied ?
   #6791 looks like more promising but not fully sure.  
   Could I get some information about it ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-634488144


   /pulsarbot run-failure-checks


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-631836725


   @merlimat I have added a pointer to indicate where the consumer joined and if the pointer has been mark deleted, the dispatcher dispatch messages to the consumer. The fenced messages just messages buffer, If messages don't belong to this consumer, the messages can continue dispatch to other consumers. If the buffer is full, the dispatcher will stop dispatch messages. I'm not sure if this is a good approach, the main idea does not stop dispatch messages to other consumers.
   
   >  I have already a fix (working in prod) but it's backed up on #6791.
   
   Is this mean the fix depends on #6791? Or the fix is only for the consistent hash approach.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] merlimat commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-631604909


   @codelipenghui This is not quite what I was describing in the comment. I don't think it's desirable to "fence" the individual messages, rather just keeping a pointer to where the consumer joined. I have already a fix (working in prod) but it's backed up on #6791.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-639215552


   close via #7106 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Huanli-Meng commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
Huanli-Meng commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-634532636


   should the key_shared subscription in messaging section be updated?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] Huanli-Meng commented on a change in pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
Huanli-Meng commented on a change in pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#discussion_r427045024



##########
File path: site2/docs/reference-configuration.md
##########
@@ -144,6 +144,8 @@ Pulsar brokers are responsible for handling incoming messages from producers, di
 |statusFilePath|  Path for the file used to determine the rotation status for the broker when responding to service discovery health checks ||
 |preferLaterVersions| If true, (and ModularLoadManagerImpl is being used), the load manager will attempt to use only brokers running the latest software version (to minimize impact to bundles)  |false|
 |maxNumPartitionsPerPartitionedTopic|Max number of partitions per partitioned topic. Use 0 or negative number to disable the check|0|
+|subscriptionKeySharedEnable| Enable Key_Shared subscription |true|

Review comment:
       ```suggestion
   |subscriptionKeySharedEnable| Enable Key_Shared subscription. |true|
   ```

##########
File path: site2/docs/reference-configuration.md
##########
@@ -144,6 +144,8 @@ Pulsar brokers are responsible for handling incoming messages from producers, di
 |statusFilePath|  Path for the file used to determine the rotation status for the broker when responding to service discovery health checks ||
 |preferLaterVersions| If true, (and ModularLoadManagerImpl is being used), the load manager will attempt to use only brokers running the latest software version (to minimize impact to bundles)  |false|
 |maxNumPartitionsPerPartitionedTopic|Max number of partitions per partitioned topic. Use 0 or negative number to disable the check|0|
+|subscriptionKeySharedEnable| Enable Key_Shared subscription |true|
+|maxFencedMessagesForKeySharedSubscription| Max fenced messages for Key_Shared subscription. Messages for the new consumer will be fenced until the previous messages are acknowledged |10000|

Review comment:
       ```suggestion
   |maxFencedMessagesForKeySharedSubscription| Set the maximum number of fenced messages for Key_Shared subscription. Messages for a new consumer are fenced until the previous messages are acknowledged. |10000|
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-630099311


   @merlimat Please help take a look at this PR. This implementation is based on your idea https://github.com/apache/pulsar/issues/6554#issuecomment-618123128. Thanks for your great idea.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui edited a comment on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui edited a comment on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-634564244


   @feeblefakie #6791 introduces a consistent hash approach for the consumer selector. The current implementation is a hash range split approach.
   
   This PR is to fix #6554


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui commented on pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977#issuecomment-634564244


   @feeblefakie #6791 introduces a consistent hash approach for the consumer selector. The current implementation is a hash range split approach.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar] codelipenghui closed pull request #6977: Introduce messages fence mechanism for Key_Shared subscription

Posted by GitBox <gi...@apache.org>.
codelipenghui closed pull request #6977:
URL: https://github.com/apache/pulsar/pull/6977


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org