TakaHiR07 commented on PR #24898: URL: https://github.com/apache/pulsar/pull/24898#issuecomment-3459554210
> The change makes sense to me since there's other logic in ManagedLedger/ManagedCursor to avoid reading beyond last confirmed entry. > > @TakaHiR07 Would it be possible to add some tests? It is hard to add test for the case in https://github.com/apache/pulsar/issues/23027. Besides, This pr just revert the getMaxReadPosition() implementation to version-2.9.x, but there is a potential issue maybe need to consider. DispatcherRateLimiter may not work well in a corner case. For example, one topic set a small rate for topic-level dispatcherRateLimiter, and then multiple subscription try to consume one by one. But topic has no new message writing, so all the subscription would become waitingCursor and wait to be notify once new message publish successful. So if new message publish, all the subscription can read message in the same time, maybe exceed the rate of dispatch. Although getMaxReadPosition() return lastConfirmedEntry avoid this corner case to some degree. But it is not reasonable because when disable transaction, the maxReadPosition should be latest. Other code implementation should obey this point or it may bring unexpected error. -- 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]
