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 2022/12/07 18:32:33 UTC

[GitHub] [pulsar] poorbarcode opened a new pull request, #18808: [fix] [broker] Fix reader can not get any messages but hasMessageAvailable always return true

poorbarcode opened a new pull request, #18808:
URL: https://github.com/apache/pulsar/pull/18808

   ### Motivation
   
   #### Context
   - After [pip-14](https://github.com/apache/pulsar/wiki/PIP-14:-Topic-compaction), the consumer that enabled feature read-compacted will read messages from `compacted topic` instead of `original topic` if the task-compaction done, and read messages from `original topic` if task-compaction is not done.
   - If the data of the last message with key `k` sent to a topic is null, the compactor will mark all messages for that key as deleted. 
   - the logic of `consumer.getLastMessageId`:
     - return `managedLedger.lastConfirmPosition` if no compacted.
     - return the last message that not deleted by compaction, it maybe less than `managedLedger.lastConfirmPosition`.
   - the logic of `reader.hasMessageAvailable`
     1.  we can call the last received message id as `lastReceiveMessageId`
     1. get the last message id and cache it. aka `lastMessageIdInBroker`, it could be `managedLedger.lastConfirmPosition`, it could less than `managedLedger.lastConfirmPosition`.
     1. return `lastReceiveMessageId` < `lastMessageIdInBroker`
    
   #### Possible issue scenarios.
   
   Based on the above context, there are two scenarios that a reader can not get any messages but `reader.hasMessageAvailable` always return `true`: 
   
   - Scenario-1:  The last message is deleted by compaction
     1. send messages: [{`k1`, `v1`}, {`k2`, `v2`}, {`k2`, `null`}]
     1. there are three entries in topic: `[{3:0}, {3:1}, {3:2}]`
     1. call `consumer.getLastMessageId` will get `{3:2}`, and cache it as `lastMessageIdInBroker`. 
     1. do compaction will delete the messages `{3:1}` and `{3:2}`, because the data of this message with key `k2` sent to a topic is null.
     1. receive message `{3:0}`, and mark the last received message id is `{3:0}`, aka `lastReceiveMessageId`
     1. call `reader.hasMessageAvailable` will get `true`, because `lastReceiveMessageId < lastMessageIdInBroker`
     1. call receive will stuck, because there has no message in compacted topic.
   
   - Scenario-2:  The last message in batch is deleted by compaction
     1. send messages use batch feature: [{`k1`, `v1`}, {`k2`, `v2`}, {`k2`, `null`}]
     1. there are one entry in topic: `{position=3:0, batchSize=3}`
     1. call `consumer.getLastMessageId` will get `{3:0:-1:2}`, and cache it as `lastMessageIdInBroker`. 
     1. do compaction will mark the messages `{3:0:-1,1}` and `{3:0:-1,2}` as `compactedOut`, because the data of this message with key `k2` sent to a topic is null.
     1. receive entry `{3:0}`, when the consumer unpacks the package, the two messages `{3:0:-1,1}` and `{3:0:-1,1}` are lost, because them have be marked `compactedOut`. Mark the last received message id is `{3:0,-1,0}`, aka `lastReceiveMessageId`
     1. call `reader.hasMessageAvailable` will get `true`, because `lastReceiveMessageId < lastMessageIdInBroker`
     1. call receive will stuck, because there has no message in compacted topic.
   
   ### Modifications
   
   #### Fix issue
   
   In the following two scenarios, the consumer will read the message from the `original topic`, even if read-compacted is enabled and task-compaction is done. And will moves the `cursor.markDeletedPosition` to `compactionHorizon - 1` before reading messages.
   
   - After a task of compaction done, all the data is marked deleted by compactor.
   - Reading the last message of `compacted topic`
   
   #### Add tests
   - add the tests for changes above.
   - add the tests for `getLastMessageId` that were not covered before.
   - add the tests to ensure that the last ledger which is not empty will not be deleted by task-trimLedgers, even if a new ledger has been created (which has not yet written any data). Because after this PR,  it possible to read the original topic even if task-compaction is done.
   
   
   ### Documentation
   
   - [ ] `doc` <!-- Your PR contains doc changes. Please attach the local preview screenshots (run `sh start.sh` at `pulsar/site2/website`) to your PR description, or else your PR might not get merged. -->
   - [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
   - [x] `doc-not-needed` <!-- Your PR changes do not impact docs -->
   - [ ] `doc-complete` <!-- Docs have been already added -->
   
   ### Matching PR in forked repository
   
   PR in forked repository: 
   - https://github.com/poorbarcode/pulsar/pull/46
   


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] poorbarcode closed pull request #18808: [fix] [broker] Fix reader can not get any messages but hasMessageAvailable always return true

Posted by GitBox <gi...@apache.org>.
poorbarcode closed pull request #18808: [fix] [broker] Fix reader can not get any messages but hasMessageAvailable always return true
URL: https://github.com/apache/pulsar/pull/18808


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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

[GitHub] [pulsar] poorbarcode commented on pull request #18808: [fix] [broker] Fix reader can not get any messages but hasMessageAvailable always return true

Posted by GitBox <gi...@apache.org>.
poorbarcode commented on PR #18808:
URL: https://github.com/apache/pulsar/pull/18808#issuecomment-1344033632

   I will push another PR to solve this issue


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

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

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