pulsar icon indicating copy to clipboard operation
pulsar copied to clipboard

[improve][broker] cursor read entry would trigger readMoreEntry() one more time when addWaitingCursor and notify

Open TakaHiR07 opened this issue 1 year ago • 0 comments

Search before asking

  • [X] I searched in the issues and found nothing similar.

Motivation

As the code shown, even if we disable txn, we use topic.getMaxReadPosition() which is topic.lastConfirmedEntry as OpReadEntry's maxPosition.

However, exist the situation:

  1. create OpReadEntry, using current topic's lastConfirmedEntry
  2. cursor hasNoMoreEntry, addWaitingCursor. opReadEntry become waiting.
  3. topic add new entry, trigger notifyEntriesAvailable() to notify opReadEntry. topic's lastConfirmedEntry move forward.
  4. Since opReadEntry.maxPosition is the previous lastConfirmedEntry, and now the lastConfirmedEntry has been changed. This op would enter opReadEntry.checkReadCompletion and trigger callback directly without reading any entry.
  5. Therefore, it would cause one more time to execute readEntry request.

So this is where we can improve, actually we may be able to execute the opReadEntry directly.

By the way, in version-2.9, if we disable txn, topic.getMaxReadPosition() is Position.latest, so opReadEntry.maxPosition is Position.latest too. And it would not have this issue since op.maxPosition is always > op.readPosition.

https://github.com/apache/pulsar/blob/c160cc9c3d44c1df073d63b325b45b9bc9bff8c7/pulsar-broker/src/main/java/org/apache/pulsar/broker/transaction/buffer/impl/TransactionBufferDisable.java#L109-L113

https://github.com/apache/pulsar/blob/c160cc9c3d44c1df073d63b325b45b9bc9bff8c7/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java#L368-L381

https://github.com/apache/pulsar/blob/5dc030431a60b49e81d577cd06a1ae63dbee0293/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L934-L979

https://github.com/apache/pulsar/blob/5dc030431a60b49e81d577cd06a1ae63dbee0293/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java#L2051-L2056

https://github.com/apache/pulsar/blob/5dc030431a60b49e81d577cd06a1ae63dbee0293/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java#L164-L186

Solution

Update the opReadEntry.maxPosition when trigger notifyEntriesAvailable()

Alternatives

No response

Anything else?

No response

Are you willing to submit a PR?

  • [X] I'm willing to submit a PR!

TakaHiR07 avatar Jul 12 '24 07:07 TakaHiR07