BewareMyPower commented on code in PR #15663: URL: https://github.com/apache/pulsar/pull/15663#discussion_r881366047
########## pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java: ########## @@ -2551,29 +2551,46 @@ public void readEntryFailed(ManagedLedgerException exception, Object ctx) { }, null); return future; } else { - Long ledgerId = ((ManagedCursorContainer) ledger.getCursors()).getSlowestReaderPosition().getLedgerId(); + PositionImpl slowestPosition = ((ManagedCursorContainer) ledger.getCursors()).getSlowestReaderPosition(); try { - org.apache.bookkeeper.mledger.proto.MLDataFormats.ManagedLedgerInfo.LedgerInfo - ledgerInfo = ledger.getLedgerInfo(ledgerId).get(); - if (ledgerInfo != null && ledgerInfo.hasTimestamp() && ledgerInfo.getTimestamp() > 0 - && ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp() - > backlogQuotaLimitInSecond * 1000) { - if (log.isDebugEnabled()) { - log.debug("Time based backlog quota exceeded, quota {}, age of ledger " - + "slowest cursor currently on {}", backlogQuotaLimitInSecond * 1000, - ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp()); - } - return CompletableFuture.completedFuture(true); - } else { - return CompletableFuture.completedFuture(false); - } + return slowestReaderTimeBasedBacklogQuotaCheck(slowestPosition); } catch (Exception e) { log.error("[{}][{}] Error reading entry for precise time based backlog check", topicName, e); return CompletableFuture.completedFuture(false); } } } + private CompletableFuture<Boolean> slowestReaderTimeBasedBacklogQuotaCheck(PositionImpl slowestPosition) + throws ExecutionException, InterruptedException { + int backlogQuotaLimitInSecond = getBacklogQuota(BacklogQuotaType.message_age).getLimitTime(); + Long ledgerId = slowestPosition.getLedgerId(); + if (((ManagedLedgerImpl) ledger).getLedgersInfo().lastKey().equals(ledgerId)) { + return CompletableFuture.completedFuture(false); + } + int result; + org.apache.bookkeeper.mledger.proto.MLDataFormats.ManagedLedgerInfo.LedgerInfo + ledgerInfo = ledger.getLedgerInfo(ledgerId).get(); + if (ledgerInfo != null && ledgerInfo.hasTimestamp() && ledgerInfo.getTimestamp() > 0 + && ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp() + > backlogQuotaLimitInSecond * 1000 && (result = slowestPosition.compareTo( + new PositionImpl(ledgerInfo.getLedgerId(), ledgerInfo.getEntries() - 1))) <= 0) { + if (result < 0) { Review Comment: Yes. I also found it. So I think it's controversial here and marked it as resolved. ########## pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentTopic.java: ########## @@ -2551,29 +2551,46 @@ public void readEntryFailed(ManagedLedgerException exception, Object ctx) { }, null); return future; } else { - Long ledgerId = ((ManagedCursorContainer) ledger.getCursors()).getSlowestReaderPosition().getLedgerId(); + PositionImpl slowestPosition = ((ManagedCursorContainer) ledger.getCursors()).getSlowestReaderPosition(); try { - org.apache.bookkeeper.mledger.proto.MLDataFormats.ManagedLedgerInfo.LedgerInfo - ledgerInfo = ledger.getLedgerInfo(ledgerId).get(); - if (ledgerInfo != null && ledgerInfo.hasTimestamp() && ledgerInfo.getTimestamp() > 0 - && ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp() - > backlogQuotaLimitInSecond * 1000) { - if (log.isDebugEnabled()) { - log.debug("Time based backlog quota exceeded, quota {}, age of ledger " - + "slowest cursor currently on {}", backlogQuotaLimitInSecond * 1000, - ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp()); - } - return CompletableFuture.completedFuture(true); - } else { - return CompletableFuture.completedFuture(false); - } + return slowestReaderTimeBasedBacklogQuotaCheck(slowestPosition); } catch (Exception e) { log.error("[{}][{}] Error reading entry for precise time based backlog check", topicName, e); return CompletableFuture.completedFuture(false); } } } + private CompletableFuture<Boolean> slowestReaderTimeBasedBacklogQuotaCheck(PositionImpl slowestPosition) + throws ExecutionException, InterruptedException { + int backlogQuotaLimitInSecond = getBacklogQuota(BacklogQuotaType.message_age).getLimitTime(); + Long ledgerId = slowestPosition.getLedgerId(); + if (((ManagedLedgerImpl) ledger).getLedgersInfo().lastKey().equals(ledgerId)) { + return CompletableFuture.completedFuture(false); + } + int result; + org.apache.bookkeeper.mledger.proto.MLDataFormats.ManagedLedgerInfo.LedgerInfo + ledgerInfo = ledger.getLedgerInfo(ledgerId).get(); + if (ledgerInfo != null && ledgerInfo.hasTimestamp() && ledgerInfo.getTimestamp() > 0 + && ((ManagedLedgerImpl) ledger).getClock().millis() - ledgerInfo.getTimestamp() + > backlogQuotaLimitInSecond * 1000 && (result = slowestPosition.compareTo( + new PositionImpl(ledgerInfo.getLedgerId(), ledgerInfo.getEntries() - 1))) <= 0) { + if (result < 0) { Review Comment: Yes. I also realized it. So I think it's controversial here and marked it as resolved. -- 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: commits-unsubscr...@pulsar.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org