sijie commented on a change in pull request #5809: [Issue:5669] Fix the
ledgerID not found cause NPE
URL: https://github.com/apache/pulsar/pull/5809#discussion_r355149324
##########
File path:
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -1790,8 +1790,12 @@ void updateCursor(ManagedCursorImpl cursor,
PositionImpl newPosition) {
}
}
- PositionImpl startReadOperationOnLedger(PositionImpl position) {
- long ledgerId = ledgers.ceilingKey(position.getLedgerId());
+ PositionImpl startReadOperationOnLedger(PositionImpl position,
ReadEntriesCallback callback) {
+ Long ledgerId = ledgers.ceilingKey(position.getLedgerId());
+ if (null == ledgerId) {
+ callback.readEntriesFailed(new
ManagedLedgerNotFoundException("read ledger failed, the ledgerID is null"),
null);
Review comment:
1) we should not use ManagedLedgerNotFoundExcception.
ManagedLedgerNotFoundExcception is used when a managed ledger is not found, not
a ledger in the managed ledger is not found
2) the error message should be improved. `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.` that means in this managed ledger there is no ledger
greater than or equal to `position.getLedgerId()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
