This is an automated email from the ASF dual-hosted git repository.

sijie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bookkeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 468743e  In DbLedgerStorage use default values when config key is 
present but empty
468743e is described below

commit 468743e7e45ce5a862ddef41de0e4b87d988402a
Author: Matteo Merli <[email protected]>
AuthorDate: Sun Mar 24 19:43:27 2019 -0700

    In DbLedgerStorage use default values when config key is present but empty
    
    ### Motivation
    
    Currently setting the `dbStorage_writeCacheMaxSizeMb=` with empty value is 
making the
    DbLedgerStorage initialize to fail since the empty string is being parsed 
as long.
    
    Instead, we should just apply the default value as in the case where the 
config key is not there.
    
    Reviewers: Enrico Olivelli <[email protected]>, Jia Zhai 
<[email protected]>, Sijie Guo <[email protected]>
    
    This closes #1996 from merlimat/db-storage-config
---
 .../bookie/storage/ldb/DbLedgerStorage.java        | 23 +++++++++++++++++++---
 .../bookie/storage/ldb/KeyValueStorageRocksDB.java |  3 ++-
 .../storage/ldb/DbLedgerStorageBookieTest.java     |  6 ++++++
 3 files changed, 28 insertions(+), 4 deletions(-)

diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
index 287f452..e118349 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorage.java
@@ -62,6 +62,7 @@ import org.apache.bookkeeper.meta.LedgerManager;
 import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.stats.StatsLogger;
 import org.apache.bookkeeper.util.DiskChecker;
+import org.apache.commons.lang3.StringUtils;
 
 
 /**
@@ -80,7 +81,8 @@ public class DbLedgerStorage implements LedgerStorage {
 
     private static final long DEFAULT_WRITE_CACHE_MAX_SIZE_MB = (long) (0.25 * 
PlatformDependent.maxDirectMemory())
             / MB;
-    private static final long DEFAULT_READ_CACHE_MAX_SIZE_MB = (long) (0.25 * 
PlatformDependent.maxDirectMemory()) / MB;
+    private static final long DEFAULT_READ_CACHE_MAX_SIZE_MB = (long) (0.25 * 
PlatformDependent.maxDirectMemory())
+            / MB;
     private int numberOfDirs;
     private List<SingleDirectoryDbLedgerStorage> ledgerStorageList;
 
@@ -94,8 +96,10 @@ public class DbLedgerStorage implements LedgerStorage {
     public void initialize(ServerConfiguration conf, LedgerManager 
ledgerManager, LedgerDirsManager ledgerDirsManager,
             LedgerDirsManager indexDirsManager, StateManager stateManager, 
CheckpointSource checkpointSource,
             Checkpointer checkpointer, StatsLogger statsLogger, 
ByteBufAllocator allocator) throws IOException {
-        long writeCacheMaxSize = conf.getLong(WRITE_CACHE_MAX_SIZE_MB, 
DEFAULT_WRITE_CACHE_MAX_SIZE_MB) * MB;
-        long readCacheMaxSize = conf.getLong(READ_AHEAD_CACHE_MAX_SIZE_MB, 
DEFAULT_READ_CACHE_MAX_SIZE_MB) * MB;
+        long writeCacheMaxSize = getLongVariableOrDefault(conf, 
WRITE_CACHE_MAX_SIZE_MB,
+                DEFAULT_WRITE_CACHE_MAX_SIZE_MB) * MB;
+        long readCacheMaxSize = getLongVariableOrDefault(conf, 
READ_AHEAD_CACHE_MAX_SIZE_MB,
+                DEFAULT_READ_CACHE_MAX_SIZE_MB) * MB;
 
         this.allocator = allocator;
         this.numberOfDirs = ledgerDirsManager.getAllLedgerDirs().size();
@@ -335,4 +339,17 @@ public class DbLedgerStorage implements LedgerStorage {
         return ledgerStorageList.stream()
             .map(single -> 
single.getGarbageCollectionStatus().get(0)).collect(Collectors.toList());
     }
+
+    static long getLongVariableOrDefault(ServerConfiguration conf, String 
keyName, long defaultValue) {
+        Object obj = conf.getProperty(keyName);
+        if (obj instanceof Number) {
+            return ((Number) obj).longValue();
+        } else if (obj == null) {
+            return defaultValue;
+        } else if (StringUtils.isEmpty(conf.getString(keyName))) {
+            return defaultValue;
+        } else {
+            return conf.getLong(keyName);
+        }
+    }
 }
diff --git 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java
 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java
index c06a3cc..88411ce 100644
--- 
a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java
+++ 
b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorageRocksDB.java
@@ -103,6 +103,8 @@ public class KeyValueStorageRocksDB implements 
KeyValueStorage {
             if (dbConfigType == DbConfigType.Huge) {
                 // Set default RocksDB block-cache size to 10% of direct mem, 
unless override
                 long defaultRocksDBBlockCacheSizeBytes = 
PlatformDependent.maxDirectMemory() / 10;
+                long blockCacheSize = 
DbLedgerStorage.getLongVariableOrDefault(conf, ROCKSDB_BLOCK_CACHE_SIZE,
+                        defaultRocksDBBlockCacheSizeBytes);
 
                 long writeBufferSizeMB = 
conf.getInt(ROCKSDB_WRITE_BUFFER_SIZE_MB, 64);
                 long sstSizeMB = conf.getInt(ROCKSDB_SST_SIZE_MB, 64);
@@ -110,7 +112,6 @@ public class KeyValueStorageRocksDB implements 
KeyValueStorage {
                 int numFilesInLevel0 = 
conf.getInt(ROCKSDB_NUM_FILES_IN_LEVEL0, 4);
                 long maxSizeInLevel1MB = 
conf.getLong(ROCKSDB_MAX_SIZE_IN_LEVEL1_MB, 256);
                 int blockSize = conf.getInt(ROCKSDB_BLOCK_SIZE, 64 * 1024);
-                long blockCacheSize = conf.getLong(ROCKSDB_BLOCK_CACHE_SIZE, 
defaultRocksDBBlockCacheSizeBytes);
                 int bloomFilterBitsPerKey = 
conf.getInt(ROCKSDB_BLOOM_FILTERS_BITS_PER_KEY, 10);
                 boolean lz4CompressionEnabled = 
conf.getBoolean(ROCKSDB_LZ4_COMPRESSION_ENABLED, true);
 
diff --git 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageBookieTest.java
 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageBookieTest.java
index cfc7447..11c3ef3 100644
--- 
a/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageBookieTest.java
+++ 
b/bookkeeper-server/src/test/java/org/apache/bookkeeper/bookie/storage/ldb/DbLedgerStorageBookieTest.java
@@ -43,6 +43,12 @@ public class DbLedgerStorageBookieTest extends 
BookKeeperClusterTestCase {
         baseConf.setLedgerStorageClass(DbLedgerStorage.class.getName());
         baseConf.setFlushInterval(60000);
         baseConf.setGcWaitTime(60000);
+
+        // Leave it empty to pickup default
+        baseConf.setProperty(DbLedgerStorage.WRITE_CACHE_MAX_SIZE_MB, "");
+
+        // Configure explicitely with a int object
+        baseConf.setProperty(DbLedgerStorage.READ_AHEAD_CACHE_MAX_SIZE_MB, 16);
     }
 
     @Test

Reply via email to