horizonzy commented on code in PR #15098:
URL: https://github.com/apache/pulsar/pull/15098#discussion_r846920806
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -2230,9 +2236,8 @@ void updateCursor(ManagedCursorImpl cursor, PositionImpl
newPosition) {
PositionImpl startReadOperationOnLedger(PositionImpl position, OpReadEntry
opReadEntry) {
Long ledgerId = ledgers.ceilingKey(position.getLedgerId());
if (null == ledgerId) {
- opReadEntry.readEntriesFailed(new
ManagedLedgerException.NoMoreEntriesToReadException("The ceilingKey(K key"
- + ") method is used to return the least key greater than
or equal to the given key, "
- + "or null if there is no such key"), null);
+ opReadEntry.makeInvalid();
Review Comment:
> Can you add a unit test to cover this path? It seems `readPosition` of
this invalid OpReadEntry is used before `asyncReadEntries` in `hasMoreEntries`
Maybe not, it will still be used in asyncReadEntries.
At line_768, the wait OpReadEntry set to property `WAITING_READ_OP_UPDATER`.
https://github.com/apache/pulsar/blob/c1631588fd71f4e5d63557741bacf2b56f27563d/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L768
In notifyEntriesAvailable, get wait OpReadEntry from
`WAITING_READ_OP_UPDATER`, and handle it to `asyncReadEntries `.
And in notifyEntriesAvailable, there be a bad behavior. fixes at #15102
https://github.com/apache/pulsar/blob/c1631588fd71f4e5d63557741bacf2b56f27563d/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L2749-L2772
--
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]
