[GitHub] [pulsar] sijie commented on a change in pull request #8725: Issue 8677: Cannot get lastMessageId for an empty topic due to message retention

[GitBox <gi...@apache.org>]

sijie commented on a change in pull request #8725:
URL: https://github.com/apache/pulsar/pull/8725#discussion_r532327986



##########
File path: managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -2089,6 +2089,10 @@ void internalTrimConsumedLedgers(CompletableFuture<?> promise) {
                     log.debug("[{}] Ledger {} skipped for deletion as it is currently being written to", name,
                             ls.getLedgerId());
                     break;
+                } else if (currentLastConfirmedEntry != null && ls.getLedgerId() == currentLastConfirmedEntry.getLedgerId()) {

Review comment:
       I am not sure this is the right fix. It changes the trim behavior which causes a rolled ledger to never be deleted until a new message is produced. I think a correct fix is to fix getLastMessageId behavior to correctly handle an empty ledger case.




----------------------------------------------------------------
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:
users@infra.apache.org