From 114cc73b4bede6ea32a94e6520610208cfd78543 Mon Sep 17 00:00:00 2001 From: Maurice Barnum Date: Mon, 12 Apr 2021 10:55:33 -0700 Subject: [PATCH 1/4] update rocksdb to 6.16.4 There have been several bug fixes and performance improvements in RocksDB since 6.10.2 was released. See https://github.com/facebook/rocksdb/releases --- bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt | 6 +++--- bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt | 6 +++--- pom.xml | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt index 7273c245177..627ad63a528 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-all.bin.txt @@ -253,7 +253,7 @@ Apache Software License, Version 2. - lib/org.eclipse.jetty-jetty-server-9.4.33.v20201020.jar [22] - lib/org.eclipse.jetty-jetty-servlet-9.4.33.v20201020.jar [22] - lib/org.eclipse.jetty-jetty-util-9.4.33.v20201020.jar [22] -- lib/org.rocksdb-rocksdbjni-6.10.2.jar [23] +- lib/org.rocksdb-rocksdbjni-6.16.4.jar [23] - lib/com.beust-jcommander-1.78.jar [24] - lib/com.yahoo.datasketches-memory-0.8.3.jar [25] - lib/com.yahoo.datasketches-sketches-core-0.8.3.jar [25] @@ -327,7 +327,7 @@ Apache Software License, Version 2. [20] Source available at https://git-wip-us.apache.org/repos/asf?p=commons-lang.git;a=shortlog;h=refs/tags/LANG_3_6 [21] Source available at https://github.com/apache/zookeeper/tree/release-3.6.2 [22] Source available at https://github.com/eclipse/jetty.project/tree/jetty-9.4.33.v20201020 -[23] Source available at https://github.com/facebook/rocksdb/tree/v5.13.1 +[23] Source available at https://github.com/facebook/rocksdb/tree/v6.16.4 [24] Source available at https://github.com/cbeust/jcommander/tree/1.78 [25] Source available at https://github.com/DataSketches/sketches-core/tree/sketches-0.8.3 [26] Source available at https://github.com/lz4/lz4-java/tree/1.3.0 @@ -584,7 +584,7 @@ This private header is also used by Apple's open source * http://www.opensource.apple.com/source/configd/configd-453.19/dnsinfo/dnsinfo.h ------------------------------------------------------------------------------------ -lib/org.rocksdb-rocksdbjni-6.10.2.jar is derived from leveldb, which is under the following license. +lib/org.rocksdb-rocksdbjni-6.16.4.jar is derived from leveldb, which is under the following license. Copyright (c) 2011 The LevelDB Authors. All rights reserved. diff --git a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt index 4311a9498db..61bf0b82659 100644 --- a/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt +++ b/bookkeeper-dist/src/main/resources/LICENSE-server.bin.txt @@ -253,7 +253,7 @@ Apache Software License, Version 2. - lib/org.eclipse.jetty-jetty-server-9.4.33.v20201020.jar [22] - lib/org.eclipse.jetty-jetty-servlet-9.4.33.v20201020.jar [22] - lib/org.eclipse.jetty-jetty-util-9.4.33.v20201020.jar [22] -- lib/org.rocksdb-rocksdbjni-6.10.2.jar [23] +- lib/org.rocksdb-rocksdbjni-6.16.4.jar [23] - lib/com.beust-jcommander-1.78.jar [24] - lib/com.yahoo.datasketches-memory-0.8.3.jar [25] - lib/com.yahoo.datasketches-sketches-core-0.8.3.jar [25] @@ -325,7 +325,7 @@ Apache Software License, Version 2. [20] Source available at https://git-wip-us.apache.org/repos/asf?p=commons-lang.git;a=shortlog;h=refs/tags/LANG_3_6 [21] Source available at https://github.com/apache/zookeeper/tree/release-3.6.2 [22] Source available at https://github.com/eclipse/jetty.project/tree/jetty-9.4.33.v20201020 -[23] Source available at https://github.com/facebook/rocksdb/tree/v5.13.1 +[23] Source available at https://github.com/facebook/rocksdb/tree/v6.16.4 [24] Source available at https://github.com/cbeust/jcommander/tree/1.78 [25] Source available at https://github.com/DataSketches/sketches-core/tree/sketches-0.8.3 [26] Source available at https://github.com/lz4/lz4-java/tree/1.3.0 @@ -582,7 +582,7 @@ This private header is also used by Apple's open source * http://www.opensource.apple.com/source/configd/configd-453.19/dnsinfo/dnsinfo.h ------------------------------------------------------------------------------------ -lib/org.rocksdb-rocksdbjni-6.10.2.jar is derived from leveldb, which is under the following license. +lib/org.rocksdb-rocksdbjni-6.16.4.jar is derived from leveldb, which is under the following license. Copyright (c) 2011 The LevelDB Authors. All rights reserved. diff --git a/pom.xml b/pom.xml index daa39d586fd..de66916cab8 100644 --- a/pom.xml +++ b/pom.xml @@ -160,7 +160,7 @@ 3.14.0 ${grpc.version} 0.9.11 - 6.10.2 + 6.16.4 3.0.1 1.7.25 1.19 From 0c5ef8f53e0415eeb994d46bbde69445e360bb78 Mon Sep 17 00:00:00 2001 From: Maurice Barnum Date: Tue, 6 Apr 2021 18:43:05 -0700 Subject: [PATCH 2/4] KeyValueStorageRocksDB.getFloor() - reimplement to avoid two seeks Use Rocksdb's upper bound support to avoid seeking performing two seeks per invocation: seekTo() followed by prev(), if key is found, or seekToLast() if it isn't. --- .../storage/ldb/KeyValueStorageRocksDB.java | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) 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 6a710097667..5ec23397ed6 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 @@ -47,6 +47,7 @@ import org.rocksdb.RocksDB; import org.rocksdb.RocksDBException; import org.rocksdb.RocksIterator; +import org.rocksdb.Slice; import org.rocksdb.WriteBatch; import org.rocksdb.WriteOptions; import org.slf4j.Logger; @@ -239,31 +240,15 @@ public int get(byte[] key, byte[] value) throws IOException { @Override public Entry getFloor(byte[] key) throws IOException { - try (RocksIterator iterator = db.newIterator(optionCache)) { - // Position the iterator on the record whose key is >= to the supplied key - iterator.seek(key); - - if (!iterator.isValid()) { - // There are no entries >= key - iterator.seekToLast(); - if (iterator.isValid()) { - return new EntryWrapper(iterator.key(), iterator.value()); - } else { - // Db is empty - return null; - } - } - - iterator.prev(); - - if (!iterator.isValid()) { - // Iterator is on the 1st entry of the db and this entry key is >= to the target - // key - return null; - } else { + try (Slice upperBound = new Slice(key); + ReadOptions option = new ReadOptions(optionCache).setIterateUpperBound(upperBound); + RocksIterator iterator = db.newIterator(option)) { + iterator.seekToLast(); + if (iterator.isValid()) { return new EntryWrapper(iterator.key(), iterator.value()); } } + return null; } @Override From 0be68a1d0387d33e7567ffffacf22f3e9ed7b52b Mon Sep 17 00:00:00 2001 From: Maurice Barnum Date: Tue, 6 Apr 2021 18:47:13 -0700 Subject: [PATCH 3/4] KeyValueStorageRocksDB bounded key iterator: use RocksDB upper bound Use RocksDB upper bound option to move key comparisons into native code and avoid copying a (key, value) pair that will be discarded. This will also address a minor (irrelevant?) error where the error check in next() will falsely succeed after hasNext() returns false on the bound. --- .../bookie/storage/ldb/KeyValueStorageRocksDB.java | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) 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 5ec23397ed6..7ffc2424fc7 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 @@ -22,14 +22,11 @@ import static com.google.common.base.Preconditions.checkState; -import com.google.common.primitives.UnsignedBytes; - //CHECKSTYLE.OFF: IllegalImport import io.netty.util.internal.PlatformDependent; //CHECKSTYLE.ON: IllegalImport import java.io.IOException; -import java.util.Comparator; import java.util.Map.Entry; import java.util.concurrent.TimeUnit; @@ -311,13 +308,15 @@ public void close() { @Override public CloseableIterator keys(byte[] firstKey, byte[] lastKey) { - final RocksIterator iterator = db.newIterator(optionCache); + final Slice upperBound = new Slice(lastKey); + final ReadOptions option = new ReadOptions(optionCache).setIterateUpperBound(upperBound); + final RocksIterator iterator = db.newIterator(option); iterator.seek(firstKey); return new CloseableIterator() { @Override public boolean hasNext() { - return iterator.isValid() && ByteComparator.compare(iterator.key(), lastKey) < 0; + return iterator.isValid(); } @Override @@ -331,6 +330,8 @@ public byte[] next() { @Override public void close() { iterator.close(); + option.close(); + upperBound.close(); } }; } @@ -459,7 +460,5 @@ public byte[] getKey() { } } - private static final Comparator ByteComparator = UnsignedBytes.lexicographicalComparator(); - private static final Logger log = LoggerFactory.getLogger(KeyValueStorageRocksDB.class); } From c02996853c6a327171a522013aab9c43041a5a86 Mon Sep 17 00:00:00 2001 From: Maurice Barnum Date: Tue, 6 Apr 2021 18:51:43 -0700 Subject: [PATCH 4/4] Partially compact RocksDB after removing deleted ledgers Accumulating many delete tombstones can severely impact performance of seek operations. Compact out the tombstones after potentially adding many when cleainging up the ledger metadata index and the entry location index. KeyValueStorage.compact() - new method to compact a range of keys. Default implementation does nothing. KeyValueStorageRocksDB.compact() - implement with RocksDB.compactRange() --- .../bookie/storage/ldb/EntryLocationIndex.java | 9 +++++++-- .../bookkeeper/bookie/storage/ldb/KeyValueStorage.java | 10 ++++++++++ .../bookie/storage/ldb/KeyValueStorageRocksDB.java | 9 +++++++++ .../bookie/storage/ldb/LedgerMetadataIndex.java | 8 +++++++- 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java index 6b01c503fff..f60b8e471f6 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/EntryLocationIndex.java @@ -196,6 +196,7 @@ public void removeOffsetFromDeletedLedgers() throws IOException { long deletedEntriesInBatch = 0; Batch batch = locationsDb.newBatch(); + final byte[] firstDeletedKey = new byte[keyToDelete.array.length]; try { for (long ledgerId : ledgersToDelete) { @@ -238,7 +239,9 @@ public void removeOffsetFromDeletedLedgers() throws IOException { } batch.remove(keyToDelete.array); ++deletedEntriesInBatch; - ++deletedEntries; + if (deletedEntries++ == 0) { + System.arraycopy(keyToDelete.array, 0, firstDeletedKey, 0, firstDeletedKey.length); + } } if (deletedEntriesInBatch > DELETE_ENTRIES_BATCH_SIZE) { @@ -251,8 +254,10 @@ public void removeOffsetFromDeletedLedgers() throws IOException { try { batch.flush(); batch.clear(); + if (deletedEntries != 0) { + locationsDb.compact(firstDeletedKey, keyToDelete.array); + } } finally { - firstKeyWrapper.recycle(); lastKeyWrapper.recycle(); keyToDelete.recycle(); diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorage.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorage.java index aa0119ad6bc..27f987c2c4d 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorage.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/KeyValueStorage.java @@ -96,6 +96,16 @@ public interface KeyValueStorage extends Closeable { */ void delete(byte[] key) throws IOException; + /** + * Compact storage within a specified range. + * + * @param firstKey + * the first key in the range (included) + * @param lastKey + * the last key in the range (not included) + */ + default void compact(byte[] firstKey, byte[] lastKey) throws IOException {} + /** * Get an iterator over to scan sequentially through all the keys in the * database. 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 7ffc2424fc7..2856d92a5d1 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 @@ -271,6 +271,15 @@ public void delete(byte[] key) throws IOException { } } + @Override + public void compact(byte[] firstKey, byte[] lastKey) throws IOException { + try { + db.compactRange(firstKey, lastKey); + } catch (RocksDBException e) { + throw new IOException("Error in RocksDB compact", e); + } + } + @Override public void sync() throws IOException { try { diff --git a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgerMetadataIndex.java b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgerMetadataIndex.java index 517681fcd93..aeec6afd3ff 100644 --- a/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgerMetadataIndex.java +++ b/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/storage/ldb/LedgerMetadataIndex.java @@ -228,13 +228,16 @@ public void flush() throws IOException { public void removeDeletedLedgers() throws IOException { LongWrapper key = LongWrapper.get(); + final byte[] startKey = new byte[key.array.length]; int deletedLedgers = 0; while (!pendingDeletedLedgers.isEmpty()) { long ledgerId = pendingDeletedLedgers.poll(); key.set(ledgerId); ledgersDb.delete(key.array); - deletedLedgers++; + if (deletedLedgers++ == 0) { + System.arraycopy(key.array, 0, startKey, 0, startKey.length); + } } if (log.isDebugEnabled()) { @@ -242,6 +245,9 @@ public void removeDeletedLedgers() throws IOException { } ledgersDb.sync(); + if (deletedLedgers != 0) { + ledgersDb.compact(startKey, key.array); + } key.recycle(); }