[GitHub] [bookkeeper] vzhikserg commented on a change in pull request #1949: [bookie-gc] add option to cache entry-log metadata map into rocksDB

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

vzhikserg commented on a change in pull request #1949: [bookie-gc] add option to cache entry-log metadata map into rocksDB
URL: https://github.com/apache/bookkeeper/pull/1949#discussion_r307052433
 
 

 ##########
 File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
 ##########
 @@ -381,31 +397,45 @@ private void doGcLedgers() {
     }
 
     /**
-     * Garbage collect those entry loggers which are not associated with any active ledgers.
+     * Garbage collect those entry loggers which are not associated with any
+     * active ledgers.
+     *
+     * @throws IOException
      */
-    private void doGcEntryLogs() {
+    private void doGcEntryLogs() throws EntryLogMetadataMapException {
         // Get a cumulative count, don't update until complete
         AtomicLong totalEntryLogSizeAcc = new AtomicLong(0L);
 
         // Loop through all of the entry logs and remove the non-active ledgers.
         entryLogMetaMap.forEach((entryLogId, meta) -> {
-           removeIfLedgerNotExists(meta);
-           if (meta.isEmpty()) {
-               // This means the entry log is not associated with any active ledgers anymore.
-               // We can remove this entry log file now.
-               LOG.info("Deleting entryLogId " + entryLogId + " as it has no active ledgers!");
-               removeEntryLog(entryLogId);
-               gcStats.getReclaimedSpaceViaDeletes().add(meta.getTotalSize());
-           }
-
-           totalEntryLogSizeAcc.getAndAdd(meta.getRemainingSize());
+            try {
+                removeIfLedgerNotExists(meta);
+                // update entryMetadta to persistent-map
+                entryLogMetaMap.put(meta.getEntryLogId(), meta);
+            } catch (EntryLogMetadataMapException e) {
+                // Ignore and continue because ledger will not be cleaned up
+                // from entry-logger in this pass and will be taken care in next
+                // schedule task
+                LOG.warn("Failed to remove ledger from entry-log metadata {}", entryLogId, e);
+            }
+            if (meta.isEmpty()) {
+                // This means the entry log is not associated with any active
+                // ledgers anymore.
+                // We can remove this entry log file now.
+                LOG.info("Deleting entryLogId " + entryLogId + " as it has no active ledgers!");
 
 Review comment:
   Please use a parametrized message

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


With regards,
Apache Git Services