gaoran10 commented on a change in pull request #8938:
URL: https://github.com/apache/pulsar/pull/8938#discussion_r545578288



##########
File path: 
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java
##########
@@ -1411,9 +1411,21 @@ synchronized void ledgerClosed(final LedgerHandle lh) {
             log.debug("[{}] Ledger has been closed id={} entries={}", name, 
lh.getId(), entriesInLedger);
         }
         if (entriesInLedger > 0) {
-            LedgerInfo info = 
LedgerInfo.newBuilder().setLedgerId(lh.getId()).setEntries(entriesInLedger)
-                    
.setSize(lh.getLength()).setTimestamp(clock.millis()).build();
-            ledgers.put(lh.getId(), info);
+            synchronized (ledgers) {

Review comment:
       Thanks, eolivelli, I think you are right.
   
   I checked the code and found out all situations that change the `ledgers` in 
the `ManagedLedgerImpl`, for a ledger its `LedgerInfo` update operations don't 
exist competition.
   
       1. When the ManagedLedgerImpl initialized, recover metadata from the 
metadata store.
       2. When creating a new ledger.
       3. When the ledger closed.
       4. When the offloaded ledger data should be deleted from bk.
       5. When preparing for offload.
       6. When offloading complete.
   
   I think the `LedgerInfo` should be initialized when the ledger created, 
after this, the `LedgerInfo` should be updated, not be created repeatedly.
   
   PTAL




----------------------------------------------------------------
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:
[email protected]


Reply via email to