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

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

hangc0276 commented on a change in pull request #1949:
URL: https://github.com/apache/bookkeeper/pull/1949#discussion_r767791844



##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/ServerConfiguration.java
##########
@@ -441,6 +444,50 @@ public ServerConfiguration setVerifyMetadataOnGc(boolean verifyMetadataOnGC) {
         return this;
     }
 
+    /**
+     * Get whether the bookie is configured to use persistent
+     * entrylogMetadataMap.
+     * @return use persistent entry-log metadata map
+     */
+    public boolean isGcEntryLogMetadataCacheEnabled() {
+        return this.getBoolean(GC_ENTRYLOGMETADATA_CACHE_ENABLED, false);
+    }
+
+    /**
+     * Set whether the bookie is configured to use persistent
+     * entrylogMetadataMap.
+     * @param gcEntryLogMetadataCacheEnabled
+     * @return server configuration
+     */
+    public ServerConfiguration setGcEntryLogMetadataCacheEnabled(
+            boolean gcEntryLogMetadataCacheEnabled) {
+        this.setProperty(GC_ENTRYLOGMETADATA_CACHE_ENABLED, gcEntryLogMetadataCacheEnabled);
+        return this;
+    }
+
+    /**
+     * Get directory to persist Entrylog metadata if
+     * gcPersistentEntrylogMetadataMapEnabled is true.
+     *
+     * @return entrylog metadata-map persistent store dir path.(default: it
+     *         creates a sub-directory under a first available base ledger
+     *         directory with name "entrylogIndexCache").
+     */
+    public String getGcEntryLogMetadataCachePath() {
+        return getString(GC_ENTRYLOG_METADATA_CACHE_PATH, getLedgerDirNames()[0] + "/" + ENTRYLOG_INDEX_CACHE);
+    }
+
+    /**
+     * Set directory to persist Entrylog metadata if gcPersistentEntrylogMetadataMapEnabled is true.
+     *
+     * @param gcPersistentEntrylogMetadataMapPath.

Review comment:
       @param gcEntrylogMetadataCachePath ?

##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/GarbageCollectorThread.java
##########
@@ -127,6 +126,20 @@ public GarbageCollectorThread(ServerConfiguration conf, LedgerManager ledgerMana
                 Executors.newSingleThreadScheduledExecutor(new DefaultThreadFactory("GarbageCollectorThread")));
     }
 
+    private EntryLogMetadataMap createEntryLogMetadataMap() throws IOException {
+        if (conf.isGcEntryLogMetadataCacheEnabled()) {
+            String baseDir = this.conf.getGcEntryLogMetadataCachePath();
+            try {
+                return new PersistentEntryLogMetadataMap(baseDir, conf);

Review comment:
       If we configured multi ledger directories, all the ledger directories' PersistentEntryLogMetadataMap use the same RocksDB and located in the first ledger directory. 
   
   When we do `forEach` operation for PersistentEntryLogMetadataMap, it will iterate all the items in RocksDB. However most of items we iterated are not belongs to current ledger storage, it will wast of time and cpu resource.  
   
   We'd better separate the PersistentEntryLogMetadataMap's RocksDB for each ledger directory. Do you have any ideas? @rdhabalia 




-- 
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: issues-unsubscribe@bookkeeper.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org