315157973 commented on a change in pull request #11116:
URL: https://github.com/apache/pulsar/pull/11116#discussion_r659249273



##########
File path: 
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -3361,7 +3359,7 @@ private boolean currentLedgerIsFull() {
                 || currentLedgerSize >= (config.getMaxSizePerLedgerMb() * 
MegaByte));
 
         long timeSinceLedgerCreationMs = clock.millis() - 
lastLedgerCreatedTimestamp;
-        boolean maxLedgerTimeReached = timeSinceLedgerCreationMs >= 
maximumRolloverTimeMs;
+        boolean maxLedgerTimeReached = timeSinceLedgerCreationMs >= 
config.getMaximumRolloverTimeMs();

Review comment:
       I look at the previous logic, there will be a random value here to 
ensure that all Ledgers are not rollover together at the same time

##########
File path: 
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -401,8 +401,6 @@ public void operationFailed(MetaStoreException e) {
         });
 
         scheduleTimeoutTask();
-
-        scheduleRollOverLedgerTask();

Review comment:
       Just a question:
   It will be executed regularly, why is it “the scheduled task will not be 
re-executed” ?

##########
File path: 
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -3929,4 +3918,13 @@ public void clearBacklogFailed(ManagedLedgerException 
exception, Object ctx) {
             return CompletableFuture.completedFuture(ensembles);
         });
     }
+
+    private void updateLastLedgerCreatedTime() {

Review comment:
       The method name is different from what it actually does




-- 
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]


Reply via email to