poorbarcode commented on PR #20179:
URL: https://github.com/apache/pulsar/pull/20179#issuecomment-1534014762
@equanz
> Currently, the position is the readPosition. The readPosition is updated
when the read is completed, even if messages are not dispatched. In this PR, I
will change the position to the last sent position.
#### Scenario-2
| time | `process: add consumer` | `process: delivery messages to client` |
| --- | --- | --- |
| 1 | read position is `1:6` |
| 2 | Add new consumer into the selector |
| 3 | | Read entries `1:6 ~ 1:10` complete |
| 4 | | Set read position `1:11` |
| 5 | Add the new consumer into recently joined consumers |
| 6 | <strong>(Highlight)</strong>The max read-position of the new consumer
is `1:11`, but the exact correct value is `1:6` |
| 7 | | Choose consumer by the selector |
| 8 | | Delivery entries `1:6 ~ 1:10` to all consumers(includes old
consumers and the new consumer) |
This scenario is what you are saying, right?<sup>[Q-1]</sup>
If yes<sup>[Q-1]</sup>, we have two solutions to fix it:
- Use `last send position` instead of `last read position`(what this PR does)
- Find a way to make the lock works for Scenario-1<sup>[scenario-1]</sup>,
and add a lock for `set read position` for Scenario-2.
I also think solution-1 is better, but declaring the data structure of
`lastSentPosition` as a Map is an improvement, right<sup>[Q-2]</sup>?
If yes<sup>[Q-2]</sup>, could you split this PR into two PR: one typed
"fix" and another one typed "improve"? And I think the PR which typed "improve"
should be submitted with a PIP because the mechanism of `Key_Shared` dispatcher
is so complicated now we need PIPs to trace the changes of it.
<strong>[scenario-1]<strong>:
https://github.com/apache/pulsar/pull/20179#pullrequestreview-1400545109
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
