poorbarcode commented on code in PR #21248:
URL: https://github.com/apache/pulsar/pull/21248#discussion_r1337131858
##########
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java:
##########
@@ -3633,12 +3633,19 @@ public boolean isValidPosition(PositionImpl position) {
log.debug("IsValid position: {} -- last: {}", position, last);
}
- if (position.getEntryId() < 0) {
+ if (!ledgers.containsKey(position.getLedgerId())){
return false;
- } else if (position.getLedgerId() > last.getLedgerId()) {
+ } else if (position.getEntryId() < 0) {
return false;
+ } else if (position.getLedgerId() > last.getLedgerId() &&
position.getLedgerId() != currentLedger.getId()) {
+ return false;
+ } else if (position.getLedgerId() == currentLedger.getId()) {
+ return position.getEntryId() <= currentLedgerEntries;
+ } else if (position.getLedgerId() == currentLedger.getId()) {
+ return position.getEntryId() <= currentLedgerEntries;
} else if (position.getLedgerId() == last.getLedgerId()) {
- return position.getEntryId() <= (last.getEntryId() + 1);
+ // Only last ledger can set entry id larger than the last entry.
+ return position.getEntryId() <= last.getEntryId();
Review Comment:
Because this logic branch contains two cases:
- `lastConfirmedPosition,ledger` equals `ml.currentLedger`
- `lastConfirmedPosition,ledger` is the latest ledger which is not empty,
but not equal `ml.currentLedger`. For example: LAC is `4:3`, current ledger is
`5` and it is empty.
In this patch, I split this logic branch into two:
- if equals `ml.currentLedger`, do `+1`
- else, do not `+1`
This method is also used to find a read position when initializing a cursor,
for example: `ml.currentLedger` is `5`(a empty ledger), there is only one
ledger(the current ledger) in the collection `ml.ledgers`, and users try to set
read position to `3:0`(the ledger `3` has been deleted), the valid position
should be `5:0` ( `0 <= -1 + 1`)
--
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]
