| 331e619 | sdong | 11 March 2020, 23:20:13 UTC | Include more information in file lock failure (#6507) Summary: When users fail to open a DB with file lock failure, it is sometimes hard for users to debug. We now include the time the lock is acquired and the thread ID that acquired the lock, to help users debug problems like this. Default Env's thread ID is used. Since type of lockedFiles is changed, rename it to follow naming convention too. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6507 Test Plan: Add a unit test and improve an existing test to validate the case. Differential Revision: D20378333 fbshipit-source-id: 312fe0e9733fd1d1e9969c321b90ce523cf4708a | 11 March 2020, 23:23:08 UTC |
| 37a635c | Levi Tamasi | 11 March 2020, 21:41:34 UTC | Disambiguate CustomFieldTags for the unity build (#6513) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6513 Test Plan: `make unity_test` Differential Revision: D20388919 Pulled By: ltamasi fbshipit-source-id: 88dbceab0723a54ee3939e1644e13dc9a4c70420 | 11 March 2020, 21:45:12 UTC |
| 8fc20ac | Adam Retter | 11 March 2020, 19:31:06 UTC | Add ppc64le builds to Travis (#6144) Summary: Let's see how this goes... Pull Request resolved: https://github.com/facebook/rocksdb/pull/6144 Differential Revision: D20387515 Pulled By: pdillinger fbshipit-source-id: ba2669348c267141dfddff910b4c2224a22cbb38 | 11 March 2020, 19:33:45 UTC |
| 65b60db | Adam Retter | 11 March 2020, 18:43:23 UTC | Update to latest Snappy to fix compilation issue on latest MacOS XCode (#6496) Summary: * **macOS version:** 10.15.2 (Catalina) * **XCode/Clang version:** Apple clang version 11.0.0 (clang-1100.0.33.16) Before this bugfix the error generated is: ``` In file included from ./util/compression.h:23: ./snappy-1.1.7/snappy.h:76:59: error: unknown type name 'string'; did you mean 'std::string'? size_t Compress(const char* input, size_t input_length, string* output); ^~~~~~ std::string /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/iosfwd:211:65: note: 'std::string' declared here typedef basic_string<char, char_traits<char>, allocator<char> > string; ^ In file included from db/builder.cc:10: In file included from ./db/builder.h:12: In file included from ./db/range_tombstone_fragmenter.h:15: In file included from ./db/pinned_iterators_manager.h:12: In file included from ./table/internal_iterator.h:13: In file included from ./table/format.h:25: In file included from ./options/cf_options.h:14: In file included from ./util/compression.h:23: ./snappy-1.1.7/snappy.h:85:19: error: unknown type name 'string'; did you mean 'std::string'? string* uncompressed); ^~~~~~ std::string /Library/Developer/CommandLineTools/usr/bin/../include/c++/v1/iosfwd:211:65: note: 'std::string' declared here typedef basic_string<char, char_traits<char>, allocator<char> > string; ^ 2 errors generated. make: *** [jls/db/builder.o] Error 1 ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/6496 Differential Revision: D20389254 Pulled By: pdillinger fbshipit-source-id: 2864245c8d0dba7b2ab81294241a62f2adf02e20 | 11 March 2020, 18:46:13 UTC |
| 00c4ab0 | Adam Retter | 11 March 2020, 15:50:20 UTC | When CMake fails to download a file, display the error message (#6511) Summary: This helps to diagnose errors in the CMake build where it tries to retrieve dependencies. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6511 Differential Revision: D20387392 Pulled By: pdillinger fbshipit-source-id: 7028dfd62704bcc747f39ff864ea9c9bf51cd1be | 11 March 2020, 15:52:46 UTC |
| f5bc3b9 | Levi Tamasi | 11 March 2020, 00:24:38 UTC | Split BlobFileState into an immutable and a mutable part (#6502) Summary: It's never too soon to refactor something. The patch splits the recently introduced (`VersionEdit` related) `BlobFileState` into two classes `BlobFileAddition` and `BlobFileGarbage`. The idea is that once blob files are closed, they are immutable, and the only thing that changes is the amount of garbage in them. In the new design, `BlobFileAddition` contains the immutable attributes (currently, the count and total size of all blobs, checksum method, and checksum value), while `BlobFileGarbage` contains the mutable GC-related information elements (count and total size of garbage blobs). This is a better fit for the GC logic and is more consistent with how SST files are handled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6502 Test Plan: `make check` Differential Revision: D20348352 Pulled By: ltamasi fbshipit-source-id: ff93f0121e80ab15e0e0a6525ba0d6af16a0e008 | 11 March 2020, 00:27:26 UTC |
| 4028eba | Chao Zhao | 10 March 2020, 20:37:37 UTC | Optional sequence number exporting during checkpoint creation (#5528) Summary: Add sequence_number_ptr to the checkpoint interface to expose the sequence number during taking the checkpoint. The number will be consistent with the seq # in rocksdb log. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5528 Test Plan: make check -j64 Reviewed By: Winger1994 Differential Revision: D16080209 fbshipit-source-id: 6dc3c7680287ee97d673c5e61f89aae1f43e33df | 10 March 2020, 20:40:18 UTC |
| fd1da22 | Yanqin Jin | 10 March 2020, 01:42:27 UTC | Support options.max_open_files != -1 with FIFO compaction (#6503) Summary: Allow user to specify options.max_open_files != -1 with FIFO compaction. If max_open_files != -1, not all table files are kept open. In the past, FIFO style compaction requires all table files to be open in order to read file creation time from table properties. Later, we added file creation time to MANIFEST, making it possible to read file creation time without opening file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6503 Test Plan: make check Differential Revision: D20353758 Pulled By: riversand963 fbshipit-source-id: ba5c61a648419e47e9ef6d74e0e280e3ee24f296 | 10 March 2020, 01:45:06 UTC |
| d93812c | Yanqin Jin | 07 March 2020, 00:21:03 UTC | Iterator with timestamp (#6255) Summary: Preliminary support for iterator with user timestamp. Current implementation does not consider merge operator and reverse iterator. Auto compaction is also disabled in unit tests. Create an iterator with timestamp. ``` ... read_opts.timestamp = &ts; auto* iter = db->NewIterator(read_opts); // target is key without timestamp. for (iter->Seek(target); iter->Valid(); iter->Next()) {} for (iter->SeekToFirst(); iter->Valid(); iter->Next()) {} delete iter; read_opts.timestamp = &ts1; // lower_bound and upper_bound are without timestamp. read_opts.iterate_lower_bound = &lower_bound; read_opts.iterate_upper_bound = &upper_bound; auto* iter1 = db->NewIterator(read_opts); // Do Seek or SeekToFirst() delete iter1; ``` Test plan (dev server) ``` $make check ``` Simple benchmarking (dev server) 1. The overhead introduced by this PR even when timestamp is disabled. key size: 16 bytes value size: 100 bytes Entries: 1000000 Data reside in main memory, and try to stress iterator. Repeated three times on master and this PR. - Seek without next ``` ./db_bench -db=/dev/shm/rocksdbtest-1000 -benchmarks=fillseq,seekrandom -enable_pipelined_write=false -disable_wal=true -format_version=3 ``` master: 159047.0 ops/sec this PR: 158922.3 ops/sec (2% drop in throughput) - Seek and next 10 times ``` ./db_bench -db=/dev/shm/rocksdbtest-1000 -benchmarks=fillseq,seekrandom -enable_pipelined_write=false -disable_wal=true -format_version=3 -seek_nexts=10 ``` master: 109539.3 ops/sec this PR: 107519.7 ops/sec (2% drop in throughput) Pull Request resolved: https://github.com/facebook/rocksdb/pull/6255 Differential Revision: D19438227 Pulled By: riversand963 fbshipit-source-id: b66b4979486f8474619f4aa6bdd88598870b0746 | 07 March 2020, 00:24:27 UTC |
| 0a0151f | Cheng Chang | 06 March 2020, 22:02:09 UTC | Remove memcpy from RandomAccessFileReader::Read in direct IO mode (#6455) Summary: In direct IO mode, RandomAccessFileReader::Read allocates an internal aligned buffer, and then copies the result into the scratch buffer. If the result is only temporarily used inside a function, there is no need to do the memcpy and just let the result Slice refer to the internally allocated buffer. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6455 Test Plan: make check Differential Revision: D20106753 Pulled By: cheng-chang fbshipit-source-id: 44f505843837bba47a56e3fa2c4dd3bd76486b58 | 06 March 2020, 22:05:12 UTC |
| f6c2777 | Otto Kekäläinen | 06 March 2020, 20:29:30 UTC | Fix spelling: commited -> committed (#6481) Summary: In most places in the code the variable names are spelled correctly as COMMITTED but in a couple places not. This fixes them and ensures the variable is always called COMMITTED everywhere. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6481 Differential Revision: D20306776 Pulled By: pdillinger fbshipit-source-id: b6c1bfe41db559b4bc6955c530934460c07f7022 | 06 March 2020, 20:45:20 UTC |
| e171a21 | Yuqi Gu | 06 March 2020, 01:15:49 UTC | Fix db_wal_test::TruncateLastLogAfterRecoverWithoutFlush failure (#6437) Summary: `TruncateLastLogAfterRecoverWithoutFlush` case depends on fallocate support of underlying file system. On a file system which lacks of this feature, like zfs, it will fail to allocate predefined file size as this test case intends to do; So a check block is added to detect fallocate support and skip test if not. The related work is done by JunHe77. Thanks! Signed-off-by: Yuqi Gu <yuqi.gu@arm.com> Pull Request resolved: https://github.com/facebook/rocksdb/pull/6437 Differential Revision: D20145032 Pulled By: pdillinger fbshipit-source-id: c8b691dc508e95acfa2a004ddbc07e2faa76680d | 06 March 2020, 01:18:16 UTC |
| afb9709 | Cheng Chang | 05 March 2020, 04:12:23 UTC | Skip high levels with no key falling in the range in CompactRange (#6482) Summary: In CompactRange, if there is no key in memtable falling in the specified range, then flush is skipped. This PR extends this skipping logic to SST file levels: it starts compaction from the highest level (starting from L0) that has files with key falling in the specified range, instead of always starts from L0. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6482 Test Plan: A new test ManualCompactionTest::SkipLevel is added. Also updated a test related to statistics of index block cache hit in db_test2, the index cache hit is increased by 1 in this PR because when checking overlap for the key range in L0, OverlapWithLevelIterator will do a seek in the table cache iterator, which will read from the cached index. Also updated db_compaction_test and db_test to use correct range for full compaction. Differential Revision: D20251149 Pulled By: cheng-chang fbshipit-source-id: f822157cf4796972bd5035d9d7178d8dfb7af08b | 05 March 2020, 04:15:25 UTC |
| e62fe50 | Zhichao Cao | 04 March 2020, 20:30:34 UTC | Introduce FaultInjectionTestFS to test fault File system instead of Env (#6414) Summary: In the current code base, we can use FaultInjectionTestEnv to simulate the env issue such as file write/read errors, which are used in most of the test. The PR https://github.com/facebook/rocksdb/issues/5761 introduce the File System as a new Env API. This PR implement the FaultInjectionTestFS, which can be used to simulate when File System has issues such as IO error. user can specify any IOStatus error as input, such that FS corresponding actions will return certain error to the caller. A set of ErrorHandlerFSTests are introduced for testing Pull Request resolved: https://github.com/facebook/rocksdb/pull/6414 Test Plan: pass make asan_check, pass error_handler_fs_test. Differential Revision: D20252421 Pulled By: zhichao-cao fbshipit-source-id: e922038f8ce7e6d1da329fd0bba7283c4b779a21 | 04 March 2020, 20:35:05 UTC |
| 8bbd76e | Fabrice Fontaine | 04 March 2020, 02:06:56 UTC | Check for sys/auxv.h (#6359) Summary: Check for sys/auxv.h and getauxval before using them as they are not always available (for example on uclibc) Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com> Pull Request resolved: https://github.com/facebook/rocksdb/pull/6359 Differential Revision: D20239797 fbshipit-source-id: 175a098094d81545628c2372e7c388e70a32fd48 | 04 March 2020, 02:09:59 UTC |
| 03dbd11 | Kefu Chai | 03 March 2020, 16:39:14 UTC | s/const auto/const auto&/ when doing loop (#6477) Summary: this silences following warning from clang-11 ``` rocksdb/db/db_impl/db_impl_compaction_flush.cc:1040:21: warning: loop variable 'newf' of type 'const std::pair<int, rocksdb::FileMetaData>' creates a copy from type 'const std::pair<int\ , rocksdb::FileMetaData>' [-Wrange-loop-analysis] for (const auto newf : c->edit()->GetNewFiles()) { ^ rocksdb/db/db_impl/db_impl_compaction_flush.cc:1040:10: note: use reference type 'const std::pair<int, rocksdb::FileMetaData> &' to prevent copying for (const auto newf : c->edit()->GetNewFiles()) { ^~~~~~~~~~~~~~~~~ & ``` Signed-off-by: Kefu Chai <tchaikov@gmail.com> Pull Request resolved: https://github.com/facebook/rocksdb/pull/6477 Differential Revision: D20211850 Pulled By: ltamasi fbshipit-source-id: 3e89e13a12bba79f1b934d46b7c4c0576cdafb01 | 03 March 2020, 16:41:57 UTC |
| 48d8d07 | sumeerbhola | 03 March 2020, 04:49:27 UTC | Add missing MutexLock to MockEnv::CreateDir (#6474) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6474 Differential Revision: D20205109 Pulled By: ltamasi fbshipit-source-id: ec136005c63740f5b713ff537b5671ea9b8e217a | 03 March 2020, 04:52:19 UTC |
| 17bef7d | sdong | 03 March 2020, 00:34:36 UTC | Fix data race of GetCreationTimeOfOldestFile() (#6473) Summary: When DBImpl::GetCreationTimeOfOldestFile() calls Version::GetCreationTimeOfOldestFile(), the version is not directly or indirectly referenced, so an event like compaction can race with the operation and cause DBImpl::GetCreationTimeOfOldestFile() to access delocated data. This was caught by an ASAN run: ==268==ERROR: AddressSanitizer: heap-use-after-free on address 0x612000b7d198 at pc 0x000018332913 bp 0x7f391510d310 sp 0x7f391510d308 READ of size 8 at 0x612000b7d198 thread T845 (store_load-33) SCARINESS: 51 (8-byte-read-heap-use-after-free) #0 0x18332912 in rocksdb::Version::GetCreationTimeOfOldestFile(unsigned long*) rocksdb/src/db/version_set.cc:1488 https://github.com/facebook/rocksdb/issues/1 0x1803ddaa in rocksdb::DBImpl::GetCreationTimeOfOldestFile(unsigned long*) rocksdb/src/db/db_impl/db_impl.cc:4499 https://github.com/facebook/rocksdb/issues/2 0xe24ca09 in rocksdb::StackableDB::GetCreationTimeOfOldestFile(unsigned long*) rocksdb/utilities/stackable_db.h:392 ...... 0x612000b7d198 is located 216 bytes inside of 296-byte region [0x612000b7d0c0,0x612000b7d1e8) freed by thread T28 here: ...... https://github.com/facebook/rocksdb/issues/5 0x1832c73f in std::vector<rocksdb::FileMetaData*, std::allocator<rocksdb::FileMetaData*> >::~vector() third-party-buck/platform007/build/libgcc/include/c++/trunk/bits/stl_vector.h:435 https://github.com/facebook/rocksdb/issues/6 0x1832c73f in rocksdb::VersionStorageInfo::~VersionStorageInfo() rocksdb/src/db/version_set.cc:734 https://github.com/facebook/rocksdb/issues/7 0x1832cf42 in rocksdb::Version::~Version() rocksdb/src/db/version_set.cc:758 https://github.com/facebook/rocksdb/issues/8 0x9d1bb5 in rocksdb::Version::Unref() rocksdb/src/db/version_set.cc:2869 https://github.com/facebook/rocksdb/issues/9 0x183e7631 in rocksdb::Compaction::~Compaction() rocksdb/src/db/compaction/compaction.cc:275 https://github.com/facebook/rocksdb/issues/10 0x9e6de6 in std::default_delete<rocksdb::Compaction>::operator()(rocksdb::Compaction*) const third-party-buck/platform007/build/libgcc/include/c++/trunk/bits/unique_ptr.h:78 https://github.com/facebook/rocksdb/issues/11 0x9e6de6 in std::unique_ptr<rocksdb::Compaction, std::default_delete<rocksdb::Compaction> >::reset(rocksdb::Compaction*) third-party-buck/platform007/build/libgcc/include/c++/trunk/bits/unique_ptr.h:376 https://github.com/facebook/rocksdb/issues/12 0x9e6de6 in rocksdb::DBImpl::BackgroundCompaction(bool*, rocksdb::JobContext*, rocksdb::LogBuffer*, rocksdb::DBImpl::PrepickedCompaction*, rocksdb::Env::Priority) rocksdb/src/db/db_impl/db_impl_compaction_flush.cc:2826 https://github.com/facebook/rocksdb/issues/13 0x9ac3b8 in rocksdb::DBImpl::BackgroundCallCompaction(rocksdb::DBImpl::PrepickedCompaction*, rocksdb::Env::Priority) rocksdb/src/db/db_impl/db_impl_compaction_flush.cc:2320 https://github.com/facebook/rocksdb/issues/14 0x9abff7 in rocksdb::DBImpl::BGWorkCompaction(void*) rocksdb/src/db/db_impl/db_impl_compaction_flush.cc:2096 ...... Fix the issue by reference the super version and use the referenced version from it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6473 Test Plan: Run ASAN for all existing tests. Differential Revision: D20196416 fbshipit-source-id: 5f4a7918110fc7b8dd7841932d376bc9d1e59d6f | 03 March 2020, 00:37:01 UTC |
| 8d73137 | Zhichao Cao | 03 March 2020, 00:14:00 UTC | Replace Directory with FSDirectory in DB (#6468) Summary: In the current code base, we can use Directory from Env to manage directory (e.g, Fsync()). The PR https://github.com/facebook/rocksdb/issues/5761 introduce the File System as a new Env API. So we further replace the Directory class in DB with FSDirectory such that we can have more IO information from IOStatus returned by FSDirectory. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6468 Test Plan: pass make asan_check Differential Revision: D20195261 Pulled By: zhichao-cao fbshipit-source-id: 93962cb9436852bfcfb76e086d9e7babd461cbe1 | 03 March 2020, 00:16:26 UTC |
| 904a60f | Huisheng Liu | 02 March 2020, 23:58:32 UTC | return timestamp from get (#6409) Summary: Added new Get() methods that return timestamp. Dummy implementation is given so that classes derived from DB don't need to be touched to provide their implementation. MultiGet is not included. ReadRandom perf test (10 minutes) on the same development machine ram drive with the same DB data shows no regression (within marge of error). The test is adapted from https://github.com/facebook/rocksdb/wiki/RocksDB-In-Memory-Workload-Performance-Benchmarks. base line (commit 72ee067b9): 101.712 micros/op 314602 ops/sec; 36.0 MB/s (5658999 of 5658999 found) This PR: 100.288 micros/op 319071 ops/sec; 36.5 MB/s (5674999 of 5674999 found) ./db_bench --db=r:\rocksdb.github --num_levels=6 --key_size=20 --prefix_size=20 --keys_per_prefix=0 --value_size=100 --cache_size=2147483648 --cache_numshardbits=6 --compression_type=none --compression_ratio=1 --min_level_to_compress=-1 --disable_seek_compaction=1 --hard_rate_limit=2 --write_buffer_size=134217728 --max_write_buffer_number=2 --level0_file_num_compaction_trigger=8 --target_file_size_base=134217728 --max_bytes_for_level_base=1073741824 --disable_wal=0 --wal_dir=r:\rocksdb.github\WAL_LOG --sync=0 --verify_checksum=1 --delete_obsolete_files_period_micros=314572800 --max_background_compactions=4 --max_background_flushes=0 --level0_slowdown_writes_trigger=16 --level0_stop_writes_trigger=24 --statistics=0 --stats_per_interval=0 --stats_interval=1048576 --histogram=0 --use_plain_table=1 --open_files=-1 --mmap_read=1 --mmap_write=0 --memtablerep=prefix_hash --bloom_bits=10 --bloom_locality=1 --duration=600 --benchmarks=readrandom --use_existing_db=1 --num=25000000 --threads=32 Pull Request resolved: https://github.com/facebook/rocksdb/pull/6409 Differential Revision: D20200086 Pulled By: riversand963 fbshipit-source-id: 490edd74d924f62bd8ae9c29c2a6bbbb8410ca50 | 03 March 2020, 00:01:00 UTC |
| 8637bc1 | Levi Tamasi | 02 March 2020, 23:31:56 UTC | Fix the description of unordered_write in db_bench (#6476) Summary: As reported in https://github.com/facebook/rocksdb/issues/6467, the description of the `unordered_write` switch of `db_bench` was incorrect. (Note: the new description is based on https://rocksdb.org/blog/2019/08/15/unordered-write.html). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6476 Test Plan: `db_bench --help` Differential Revision: D20200653 Pulled By: ltamasi fbshipit-source-id: 4c3683fcfa6a069164167af5aaff9974a810c16a | 02 March 2020, 23:34:19 UTC |
| 5f2f8cd | Yanqin Jin | 02 March 2020, 20:51:17 UTC | Ignore compile_commands.json file (#6472) Summary: Both clangd and cquery-language-server requires a compile_commands.json file to index the project. This file can be ignored by git. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6472 Differential Revision: D20194899 Pulled By: riversand963 fbshipit-source-id: ea1587f2e5d10b7591147073b61efe262a1cf747 | 02 March 2020, 20:54:13 UTC |
| 9b3c9ef | sdong | 02 March 2020, 19:52:36 UTC | Add --index_with_first_key and --index_shortening_mode to DB bench (#5859) Summary: Some combinatino of --index_with_first_key and --index_shortening_mode can signifcantly improve performance for large values. Expose them in db_bench. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5859 Test Plan: Run them with the new options and observe the behavior. Differential Revision: D20104434 fbshipit-source-id: 21d48a732a9caf20b82312c7d7557d747ea3c304 | 02 March 2020, 19:55:28 UTC |
| 86f1ad7 | sdong | 29 February 2020, 00:40:13 UTC | Add more unit test coverage to MultiRead (#6452) Summary: MultiRead tests in env_test cannot simulate the io_uring case when queries need to be submitted in multiple rounds. Add a new unit test to cover up more requests per MultiRead Pull Request resolved: https://github.com/facebook/rocksdb/pull/6452 Test Plan: Run it and see it pass when liburing is enabled or not enabled. Differential Revision: D20078924 fbshipit-source-id: 6cff7fe345a4c5aa47135186e6181bf00df02b68 | 29 February 2020, 00:42:44 UTC |
| 051696b | Michael R. Crusoe | 28 February 2020, 22:10:51 UTC | fix some spelling typos (#6464) Summary: Found from Debian's "Lintian" program Pull Request resolved: https://github.com/facebook/rocksdb/pull/6464 Differential Revision: D20162862 Pulled By: zhichao-cao fbshipit-source-id: 06941ee2437b038b2b8045becbe9d2c6fbff3e12 | 28 February 2020, 22:14:03 UTC |
| 41535d0 | Manuel Ung | 28 February 2020, 19:16:50 UTC | WriteUnPrepared: Pass in correct subbatch count during rollback (#6463) Summary: Today `WriteUnpreparedTxn::RollbackInternal` will write the rollback batch assuming that there is only a single subbatch. However, because untracked_keys_ are currently not deduplicated, it's possible for duplicate keys to exist, and thus split the batch. Also, tracked_keys_ also does not support compators outside of the bytewise comparators, so it's possible for duplicates to occur there as well. To solve this, just pass in the correct subbatch count. Also, removed `WriteUnpreparedRollbackPreReleaseCallback` to unify the Commit/Rollback codepaths some more. Also, fixed a bug in `CommitInternal` where if 1. two_write_queue is true and 2. include_data is true, then `WriteUnpreparedCommitEntryPreReleaseCallback` ends up calling `AddCommitted` on the commit time write batch a second time on the second write. To fix, `WriteUnpreparedCommitEntryPreReleaseCallback` is re-initialized. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6463 Differential Revision: D20150153 Pulled By: lth fbshipit-source-id: df0b42d39406c75af73df995aa1138f0db539cd1 | 28 February 2020, 19:19:32 UTC |
| 72ee067 | Jermy Li | 27 February 2020, 20:16:27 UTC | fix assert error while db.getDefaultColumnFamily().getDescriptor() (#6006) Summary: Threw assert error at assert(isOwningHandle()) in ColumnFamilyHandle.getDescriptor(), because default CF don't own a handle, due to [RocksDB.getDefaultColumnFamily()](https://github.com/facebook/rocksdb/blob/3a408eeae95614150ac930fc7f244524ed8c6f1c/java/src/main/java/org/rocksdb/RocksDB.java#L3702) called cfHandle.disOwnNativeHandle(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6006 Differential Revision: D19031448 fbshipit-source-id: 2420c45e835bda0e552e919b1b63708472b91538 | 27 February 2020, 20:37:46 UTC |
| 741decf | Cheng Chang | 26 February 2020, 00:45:43 UTC | Return early on failure when constructing CuckooTableReader (#6453) Summary: If file is not mmaped, CuckooTableReader should not try to read table properties from the file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6453 Test Plan: Added a new unit test Differential Revision: D20103334 Pulled By: cheng-chang fbshipit-source-id: 48539f14d93f6c1ebe12c3df5a14719e9d7b8726 | 26 February 2020, 00:48:28 UTC |
| f52db84 | Andrew Kryczka | 26 February 2020, 00:43:33 UTC | support SstFileManager in db_stress (#6454) Summary: Add some flags for configuring an SstFileManager. An SstFileManager is only created when one or more of these flags are set. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6454 Test Plan: - ran it a while: ``` $ python ./tools/db_crashtest.py blackbox --simple -max_key=100000 -write_buffer_size=131072 -target_file_size_base=131072 -max_bytes_for_level_base=524288 -value_size_mult=33 --interval=10 -max_background_compactions=4 -max_background_flushes=2 -sst_file_manager_bytes_per_sec=1048576 ``` - verified with strace the SstFileManager is behaving as configured: ``` $ strace -fp `pidof db_stress` -e ftruncate,unlink ... [pid 3074805] ftruncate(9</tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash>, 67423) = 0 [pid 3074805] ftruncate(9</tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash>, 51039) = 0 [pid 3074805] ftruncate(9</tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash>, 34655) = 0 [pid 3074805] ftruncate(9</tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash>, 18271) = 0 [pid 3074805] ftruncate(9</tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash>, 1887) = 0 [pid 3074805] unlink("/tmp/rocksdb_crashtest_blackbox6OJywh/000070.sst.trash") = 0 ... ``` Differential Revision: D20103315 Pulled By: ajkr fbshipit-source-id: b3e1092747157459d244b047947a979b85c98f48 | 26 February 2020, 00:45:30 UTC |
| 69679e7 | Andrew Kryczka | 25 February 2020, 23:29:17 UTC | Fix range deletion tombstone ingestion with global seqno (#6429) Summary: Original author: jeffrey-xiao If we are writing a global seqno for an ingested file, the range tombstone metablock gets accessed and put into the cache during ingestion preparation. At the time, the global seqno of the ingested file has not yet been determined, so the cached block will not have a global seqno. When the file is ingested and we read its range tombstone metablock, it will be returned from the cache with no global seqno. In that case, we use the actual seqnos stored in the range tombstones, which are all zero, so the tombstones cover nothing. This commit removes global_seqno_ variable from Block. When iterating over a block, the global seqno for the block is determined by the iterator instead of storing this mutable attribute in Block. Additionally, this commit adds a regression test to check that keys are deleted when ingesting a file with a global seqno and range deletion tombstones. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6429 Differential Revision: D19961563 Pulled By: ajkr fbshipit-source-id: 5cf777397fa3e452401f0bf0364b0750492487b7 | 25 February 2020, 23:31:48 UTC |
| d87c10c | Levi Tamasi | 25 February 2020, 02:38:00 UTC | Add blob file state to VersionEdit (#6416) Summary: BlobDB currently does not keep track of blob files: no records are written to the manifest when a blob file is added or removed, and upon opening a database, the list of blob files is populated simply based on the contents of the blob directory. This means that lost blob files cannot be detected at the moment. We plan to solve this issue by making blob files a part of `Version`; as a first step, this patch makes it possible to store information about blob files in `VersionEdit`. Currently, this information includes blob file number, total number and size of all blobs, and total number and size of garbage blobs. However, the format is extensible: new fields can be added in both a forward compatible and a forward incompatible manner if needed (similarly to `kNewFile4`). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6416 Test Plan: `make check` Differential Revision: D19894234 Pulled By: ltamasi fbshipit-source-id: f9753e1f2aedf6dadb70c09b345207cb9c58c329 | 25 February 2020, 02:39:53 UTC |
| eb367d4 | sdong | 24 February 2020, 23:45:47 UTC | Buck config: Re-enable liburing under Linux (#6451) Summary: The known bug of liburing has been fixed. Now we can re-enable liburing under Linux Pull Request resolved: https://github.com/facebook/rocksdb/pull/6451 Test Plan: Watch internal CI Differential Revision: D20079009 fbshipit-source-id: 04a6f53a900ff721f9a62a188cf906771b5d68d2 | 24 February 2020, 23:47:34 UTC |
| b47a714 | Cheng Chang | 24 February 2020, 19:42:37 UTC | Update release version to 6.8 (#6450) Summary: Update release version to 6.8 Pull Request resolved: https://github.com/facebook/rocksdb/pull/6450 Test Plan: no code change Differential Revision: D20071889 Pulled By: cheng-chang fbshipit-source-id: 91450aae09b201926469ff32f59ed436366f3b74 | 24 February 2020, 19:44:47 UTC |
| 43dde33 | Peter Dillinger | 22 February 2020, 15:59:38 UTC | Share kPageSize (and other small tweaks) (#6443) Summary: Make kPageSize extern const size_t (used in draft https://github.com/facebook/rocksdb/issues/6427) Make kLitteEndian constexpr bool Clarify a couple of comments Pull Request resolved: https://github.com/facebook/rocksdb/pull/6443 Test Plan: make check, CI Differential Revision: D20044558 Pulled By: pdillinger fbshipit-source-id: e0c5cc13229c82726280dc0ddcba4078346b8418 | 22 February 2020, 16:01:36 UTC |
| 942eaba | sdong | 22 February 2020, 00:56:04 UTC | Handle io_uring partial results (#6441) Summary: The logic that handles io_uring partial results was wrong. Fix the logic by putting it into a queue and continue reading. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6441 Test Plan: Make sure this patch fixes the application test case where the bug was discovered; in env_test, add a unit test that simulates partial results and make sure the results are still correct. Differential Revision: D20018616 fbshipit-source-id: 5398a7e34d74c26d52aa69dfd604e93e95d99c62 | 22 February 2020, 00:57:37 UTC |
| 890d87f | Yanqin Jin | 21 February 2020, 23:07:55 UTC | Some minor fix-ups (#6440) Summary: Cleanup some code without any real change in functionality. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6440 Differential Revision: D20015891 Pulled By: riversand963 fbshipit-source-id: 33e18754b0f002006a6d4805e9aaf84c0c8ad25a | 21 February 2020, 23:09:56 UTC |
| ab65278 | Peter Dillinger | 21 February 2020, 21:29:41 UTC | Misc filter_bench improvements (#6444) Summary: Useful in validating/testing internal fragmentation changes (https://github.com/facebook/rocksdb/issues/6427) Pull Request resolved: https://github.com/facebook/rocksdb/pull/6444 Test Plan: manual (no changes to production code) Differential Revision: D20040076 Pulled By: pdillinger fbshipit-source-id: 32d26f363d2a9ab9f5bebd281dcebd9915ae340e | 21 February 2020, 21:31:57 UTC |
| fcec56e | Zaiyang Li | 21 February 2020, 19:10:54 UTC | Add function to set row cache on rocksdb_options_t (#6442) Summary: Adding a C API function to set `row_cache` on `rocksdb_options_t` as this functionality is missing. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6442 Differential Revision: D20036813 Pulled By: riversand963 fbshipit-source-id: c1fa95ea343345fbc1e57961d0d048e0e79be373 | 21 February 2020, 19:12:42 UTC |
| d75ce0a | sdong | 20 February 2020, 22:53:51 UTC | Mention rocksdb_namespace.h in HISTORY.md (#6439) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6439 Differential Revision: D20012442 fbshipit-source-id: a7c9569826eac39cd7ea69c90f08a21dd4caa335 | 20 February 2020, 22:55:19 UTC |
| 362b8d4 | Yanqin Jin | 20 February 2020, 21:13:53 UTC | Fix MANIFEST name assignment (#6426) Summary: Currently, a new MANIFEST file is assigned a new file number when 1) no MANIFEST is open, or 2) current MANIFEST file size exceeds a threshold. This is not sufficient. There are cases when the caller explicitly specifies that a new MANIFEST be created. For example, if user sets options.write_dbid_to_manifest = true, and there are WAL files, then RocksDB will run into an issue during recovery. `DBImpl::Recover()` will call `LogAndApply()` to write dbid. At this point, the db being recovered creates a new MANIFEST, say, MANIFEST-000003. Since there are WALs, `DBImpl::RecoverLogFiles` will be called. Towards the end of this function, we call `LogAndApply(new_descriptor_log=true)`, which explicitly creates a new MANIFEST. However, the manifest_file_number is wrong before this fix. Consequently, RocksDB opens an existing, non-empty file for append, effectively truncating the file to zero. If a crash occurs, then there will be data loss. Test Plan (devserver): make check Pull Request resolved: https://github.com/facebook/rocksdb/pull/6426 Test Plan: make check Differential Revision: D19951866 Pulled By: riversand963 fbshipit-source-id: 4b1b9fc28d4fe2ac12764b388ef9e61f05e766da | 20 February 2020, 22:30:58 UTC |
| fdf882d | sdong | 20 February 2020, 20:07:53 UTC | Replace namespace name "rocksdb" with ROCKSDB_NAMESPACE (#6433) Summary: When dynamically linking two binaries together, different builds of RocksDB from two sources might cause errors. To provide a tool for user to solve the problem, the RocksDB namespace is changed to a flag which can be overridden in build time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6433 Test Plan: Build release, all and jtest. Try to build with ROCKSDB_NAMESPACE with another flag. Differential Revision: D19977691 fbshipit-source-id: aa7f2d0972e1c31d75339ac48478f34f6cfcfb3e | 20 February 2020, 20:09:57 UTC |
| 4e33f1e | Gaurav Singh | 20 February 2020, 18:25:14 UTC | simplify user_access_only expression (#6360) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6360 Differential Revision: D19698918 Pulled By: riversand963 fbshipit-source-id: d20ecca541376cccd32fc7afb504ea90021860ee | 20 February 2020, 18:27:56 UTC |
| a993cc3 | Remington Brasga | 20 February 2020, 01:06:14 UTC | Fixed typo in benchmark.sh (#6434) Summary: TB = 1024 * GB Pull Request resolved: https://github.com/facebook/rocksdb/pull/6434 Differential Revision: D19978339 Pulled By: zhichao-cao fbshipit-source-id: 5a89890110b23f0ebda4a95223f66da6736321ac | 20 February 2020, 01:08:02 UTC |
| 5a29751 | Yanqin Jin | 19 February 2020, 00:08:14 UTC | Ignore .vs/.vscode dir used by visual studio/ vscode (#6428) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6428 Differential Revision: D19959176 Pulled By: riversand963 fbshipit-source-id: 5b863944663462b246bb82883fa683f75ab33fc1 | 19 February 2020, 00:10:15 UTC |
| c6abe30 | Andrew Kryczka | 18 February 2020, 21:52:09 UTC | Fix concurrent full purge and WAL recycling (#5900) Summary: We were removing the file from `log_recycle_files_` before renaming it with `ReuseWritableFile()`. Since `ReuseWritableFile()` occurs outside the DB mutex, it was possible for a concurrent full purge to sneak in and delete the file before it could be renamed. Consequently, `SwitchMemtable()` would fail and the DB would enter read-only mode. The fix is to hold the old file number in `log_recycle_files_` until after the file has been renamed. Full purge uses that list to decide which files to keep, so it can no longer delete a file pending recycling. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5900 Test Plan: new unit test Differential Revision: D19771719 Pulled By: ajkr fbshipit-source-id: 094346349ca3fb499712e62de03905acc30b5ce8 | 18 February 2020, 21:54:13 UTC |
| 0f9dcb8 | Andrew Kryczka | 18 February 2020, 19:14:37 UTC | Return NotSupported from WriteBatchWithIndex::DeleteRange (#5393) Summary: As discovered in https://github.com/facebook/rocksdb/issues/5260 and https://github.com/facebook/rocksdb/issues/5392, reads on the indexed batch do not account for range tombstones. So, return `Status::NotSupported` from `WriteBatchWithIndex::DeleteRange` until we properly support it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/5393 Test Plan: added unit test Differential Revision: D19912360 Pulled By: ajkr fbshipit-source-id: 0bbfc978ea015d64516ca708fce2429abba524cb | 18 February 2020, 19:18:25 UTC |
| 3a34575 | acelyc111 | 14 February 2020, 23:53:29 UTC | Fix compile error when LZ4 is up to r123 (#6412) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6412 Differential Revision: D19914063 Pulled By: ajkr fbshipit-source-id: 4e401e665d4b449d24c4cdec35a4585eeda95996 | 14 February 2020, 23:55:23 UTC |
| dc23c12 | Manuel Ung | 14 February 2020, 19:20:37 UTC | WriteUnPrepared: Untracked keys (#6404) Summary: For write unprepared, some applications may bypass the transaction api, and write keys directly into the write batch. However, since they are not tracked, rollbacks (both for savepoint and transaction) are not aware that these keys have to be rolled back. The fix is to track them in `WriteUnpreparedTxn::untracked_keys_`. This is populated whenever we flush unprepared batches into the DB. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6404 Differential Revision: D19842023 Pulled By: lth fbshipit-source-id: a9edfc643d5c905fc89da9a9a9094d30c9b70108 | 14 February 2020, 19:31:39 UTC |
| 152f8a8 | Cheng Chang | 14 February 2020, 16:24:09 UTC | Remove unnecessary computation of index (#6406) Summary: `index` can be replaced by `iter`, saving the computation of `index++`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6406 Test Plan: make check Differential Revision: D19905056 Pulled By: cheng-chang fbshipit-source-id: add4638959c0d2e4e77a11f3fa04ffabaf0de790 | 14 February 2020, 16:26:23 UTC |
| 4034e28 | Cheng Chang | 14 February 2020, 16:15:24 UTC | Fail fast in paranoid mode when LoadTableHandlers fail during recovering (#6368) Summary: Previously, when recovering version set, LoadTableHandlers failures are ignored. If paranoid_checks is true, this failure should not be ignored, otherwise, the opened db might be in an inconsistent state. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6368 Test Plan: make check Differential Revision: D19713459 Pulled By: cheng-chang fbshipit-source-id: 68cb94f4f2cc43f8b024b14755193cd45cfcad55 | 14 February 2020, 16:17:10 UTC |
| 29e2443 | wolfkdy | 14 February 2020, 02:50:04 UTC | refine code (#6420) Summary: I create a new branch from the branch new upsteram/master and "git merge --squash". Maybe it will fix everything. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6420 Differential Revision: D19897152 Pulled By: zhichao-cao fbshipit-source-id: 6575d9e3b23e360f42ee1480b43028b5fcc20136 | 14 February 2020, 02:55:02 UTC |
| 908b1ee | Manuel Ung | 14 February 2020, 02:50:01 UTC | WriteUnPrepared: Fix assertion during recovery (#6419) Summary: During recovery, multiple (un)prepared batches could exist in the same WAL record due to group commit. This breaks an assertion in `MemTableInserter::MarkBeginPrepare`. To fix, reset unprepared_batch_ to false after `MarkEndPrepare`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6419 Differential Revision: D19896148 Pulled By: lth fbshipit-source-id: b1a32ef88f775a0881264a18bd1a4a5b8c85eee3 | 14 February 2020, 02:52:05 UTC |
| fb57150 | Manuel Ung | 14 February 2020, 02:43:16 UTC | WriteUnPrepared: Enable WAL during crash recovery (#6418) Summary: Unfortunately, it seems like mysqld reuses xids across machine restarts. When that happens, we could have something like the following happening: ``` BEGIN_PREPARE(unprepared) Put(a) END_PREPARE(xid = 1) -- crash and recover with Put(a) rolled back as it was not prepared BEGIN_PREPARE(prepared) Put(b) END_PREPARE(xid = 1) COMMIT(xid = 1) -- crash and recover with both a, b ``` To solve this, we will have to log the rollback batch into the WAL during recovery. WritePrepared already logs the rollback batch into the WAL, if a rollback happens after prepare, so there is no problem there. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6418 Differential Revision: D19896151 Pulled By: lth fbshipit-source-id: 2ff65ddc5fe75efd57736fed4b7cd7a109d26609 | 14 February 2020, 02:44:39 UTC |
| ac8e89a | sdong | 14 February 2020, 02:39:38 UTC | Should flush and sync WAL when writing it in DB::Open() (#6417) Summary: A recent fix related to 2pc https://github.com/facebook/rocksdb/pull/6313/ writes something to WAL, but does not flush or sync. This causes assertion failure "impl->TEST_WALBufferIsEmpty()" if manual_wal_flush = true. We should fsync the entry to make sure a second power reset can recover. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6417 Test Plan: Add manual_wal_flush=true case in TransactionTest.DoubleCrashInRecovery and fix a bug in the test so that the bug can be reproduced. It passes with the fix. Differential Revision: D19894537 fbshipit-source-id: f1e84e49e2269f583c6019743118292cd8b6598e | 14 February 2020, 02:41:04 UTC |
| 4651677 | Cheng Chang | 14 February 2020, 01:25:29 UTC | Fix flaky test DecreaseNumBgThreads (#6393) Summary: The DecreaseNumBgThreads test keeps failing on Windows in AppVeyor. It fails because it depends on a timed wait for the tasks to be dequeued from the threadpool's internal queue, but within the specified time, the task might have not been scheduled onto the newly created threads. https://github.com/facebook/rocksdb/pull/6232 tries to fix this by waiting for longer time to let the threads scheduled. This PR tries to fix this by replacing the timed wait with a synchronization on the task's internal conditional variable. When the number of threads increases, instead of guessing the time needed for the task to be scheduled, it directly blocks on the conditional variable until the task starts running. But when thread number is reduced, it still does a timed wait, but this does not lead to the flakiness now, will try to remove these timed waits in a future PR. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6393 Test Plan: Wait to see whether AppVeyor tests pass. Differential Revision: D19890928 Pulled By: cheng-chang fbshipit-source-id: 4e56e4addf625c98c0876e62d9d57a6f0a156f76 | 14 February 2020, 01:27:18 UTC |
| acfee40 | sdong | 14 February 2020, 00:49:29 UTC | Remove IO URING compiler flags (#6415) Summary: Since IO Uring feature is not stable. Remove it from buck configuration. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6415 Test Plan: See internal build pass Differential Revision: D19892988 fbshipit-source-id: 7fc01efc2af5ed707fb8e4e4674223aeb83cd5ea | 14 February 2020, 00:51:10 UTC |
| 5138764 | Huisheng Liu | 13 February 2020, 19:19:39 UTC | Fix destroydb (#6308) Summary: It's observed on Windows DestroyDB failed to remove the log file because the logger is still alive in sst file manager and holding a handle to the log file. This fix makes sure the logger is released before attempt to clear the database directory. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6308 Differential Revision: D19818829 Pulled By: riversand963 fbshipit-source-id: 54c3e6859aadaaba4a49b3e851b73dc35ec7dc6a | 13 February 2020, 19:21:27 UTC |
| df3f33d | sdong | 13 February 2020, 18:50:03 UTC | Fix db_bench LITE build recently broken (#6411) Summary: A recent change https://github.com/facebook/rocksdb/pull/6386 broke LITE build in a trivial way. Fix it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6411 Test Plan: Run "LITE=1 make all" Differential Revision: D19871765 fbshipit-source-id: 74f0ad3f8a9d666fbde0da7fd29ba1547a811f77 | 13 February 2020, 18:52:50 UTC |
| a676001 | Cheng Chang | 13 February 2020, 18:18:23 UTC | Revert usage of Defer. (#6410) Summary: Seems like this caused the following test failure on AppVeyor: DBTest2.CrashInRecoveryMultipleCF c:\projects\rocksdb\db\db_test_util.cc(107): error: DestroyDB(dbname_, options) IO error: Failed to delete: C:\projects\rocksdb\db_tests\\testrocksdb-3112//db_test2_10791409581227174103/000013.sst: Access is denied. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6410 Test Plan: Wait to see whether the AppVeyor test passes. Differential Revision: D19879872 Pulled By: cheng-chang fbshipit-source-id: 59a9c55ca88566e9210c0b715ecc45a4fd9afe26 | 13 February 2020, 18:31:32 UTC |
| 6e97d4d | sdong | 13 February 2020, 02:00:24 UTC | By default turn IO Uring off. (#6405) Summary: We realized bugs related to IO Uring. Turn it off by default. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6405 Test Plan: Manually run build_tools/build_detect_platform and observe outputs. Differential Revision: D19862792 fbshipit-source-id: 5d5e8e2762997b72a145ae59389ef3d7e4ccd060 | 13 February 2020, 02:01:49 UTC |
| e645089 | Burton Li | 12 February 2020, 22:44:46 UTC | db_bench supports for generating random variable sized value. (#6386) Summary: 1. `db_bench` now supports `value_size_distribution_type`, `value_size_min`, `value_size_max` options for generating random variable sized value. 2. Added `blob_db_compression_type` option for BlobDB to enable blob compression. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6386 Differential Revision: D19859406 Pulled By: zhichao-cao fbshipit-source-id: ace52674090023fde15d832392110bf288a8e215 | 12 February 2020, 22:47:03 UTC |
| 3e49249 | anand76 | 12 February 2020, 01:25:10 UTC | Ensure all MultiGet IO errors are propagated to user (#6403) Summary: Unrevert the previous fix to propagate error status, and an additional fix to not treat a memtable lookup MergeInProgress status as an error. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6403 Test Plan: Unit tests Tried running stress tests but couldn't repro the stress failure Differential Revision: D19846721 Pulled By: anand1976 fbshipit-source-id: 7db10cccbdc863d9b559497f0a46b608d2488ca4 | 12 February 2020, 01:27:22 UTC |
| e412a42 | Tomas Kolda | 11 February 2020, 22:45:15 UTC | JNI direct buffer support for basic operations (#2283) Summary: It is very useful to support direct ByteBuffers in Java. It allows to have zero memory copy and some serializers are using that directly so one do not need to create byte[] array for it. This change also contains some fixes for Windows JNI build. Pull Request resolved: https://github.com/facebook/rocksdb/pull/2283 Differential Revision: D19834971 Pulled By: pdillinger fbshipit-source-id: 44173aa02afc9836c5498c592fd1ea95b6086e8e | 11 February 2020, 22:48:30 UTC |
| 28aa09d | Yanqin Jin | 11 February 2020, 06:24:29 UTC | Pass info_log_level to the inner logger of AutoRollLogger (#6388) Summary: Before this fix, the info_log_level passed from CreateLoggerFromOptions() will be ignored by AutoRollLogger::logger_. This PR fixes it by setting the info log level of logger_ during ResetLogger(). Test plan (dev server): ``` COMPILE_WITH_TSAN=1 make all && make check make all && make check ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/6388 Differential Revision: D19828045 Pulled By: riversand963 fbshipit-source-id: e1ac7de3a2090bee53b6c667f71a11f1774163e6 | 11 February 2020, 06:26:08 UTC |
| 35ed530 | anand76 | 11 February 2020, 06:21:53 UTC | Revert "Check KeyContext status in MultiGet (#6387)" (#6401) Summary: This reverts commit d70011bccc9a9383f368e509671d5e4eb8f3a2c2. The commit is causing some stress test failure due to unexpected Status::MergeInProgress() return for some keys. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6401 Differential Revision: D19826623 Pulled By: anand1976 fbshipit-source-id: edd634cede9cb7bdd2cb8f46e662ea709b16d2f1 | 11 February 2020, 06:23:36 UTC |
| d3ba398 | Cheng Chang | 11 February 2020, 02:12:04 UTC | Update unit tests for PinnableSlice (#6399) Summary: 1. remove AssertEmpty because calling methods on moved objects is discouraged. 2. add a test to assert that the internal buffer is moved instead of being copied. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6399 Test Plan: make slice_test && ./slice_test USE_CLANG=1 make analyze Differential Revision: D19825372 Pulled By: cheng-chang fbshipit-source-id: 2e26f8ce5ec3edbfce067db045e80bd433e704f4 | 11 February 2020, 02:13:27 UTC |
| dafb568 | Cheng Chang | 11 February 2020, 01:58:03 UTC | Add utility class Defer (#6382) Summary: Add a utility class `Defer` to defer the execution of a function until the Defer object goes out of scope. Used in VersionSet:: ProcessManifestWrites as an example. The inline comments for class `Defer` have more details. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6382 Test Plan: `make defer_test version_set_test && ./defer_test && ./version_set_test` Differential Revision: D19797538 Pulled By: cheng-chang fbshipit-source-id: b1a9b7306e4fd4f48ec2ab55783caa561a315f0f | 11 February 2020, 01:59:47 UTC |
| cbf5f3b | Levi Tamasi | 11 February 2020, 01:03:21 UTC | Do not move VersionEdit into AtomicGroupReadBuffer (#6400) Summary: https://github.com/facebook/rocksdb/pull/6383 surfaced an issue with `VersionSet`/`ReactiveVersionSet` and `AtomicGroupReadBuffer::AddEdit` (which was added in https://github.com/facebook/rocksdb/pull/5411): `AddEdit` moves the `VersionEdit` passed to it into `replay_buffer_`, however, the client `VersionSet` classes keep using it afterwards. This *seemed to* work before the refactoring but it really did not: since `VersionEdit` used to have a user-declared destructor, no move constructor/move assignment operator was generated, and the `move` in `AddEdit` was really a copy. The patch makes the copy explicit. Note: it should be possible to rework this logic so that we can get away with the move but for now, this should fix the issue. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6400 Test Plan: `make check` `make analyze` Differential Revision: D19824466 Pulled By: ltamasi fbshipit-source-id: f38033967daf2a39c78dcd6e12978bafe37632b4 | 11 February 2020, 01:15:42 UTC |
| 4369f2c | Zhichao Cao | 10 February 2020, 23:42:46 UTC | Checksum for each SST file and stores in MANIFEST (#6216) Summary: In the current code base, RocksDB generate the checksum for each block and verify the checksum at usage. Current PR enable SST file checksum. After a SST file is generated by Flush or Compaction, RocksDB generate the SST file checksum and store the checksum value and checksum method name in the vs_info and MANIFEST as part for the FileMetadata. Added the enable_sst_file_checksum to Options to enable or disable file checksum. Added sst_file_checksum to Options such that user can plugin their own SST file checksum calculate method via overriding the SstFileChecksum class. The checksum information inlcuding uint32_t checksum value and a checksum name (string). A new tool is added to LDB such that user can dump out a list of file checksum information from MANIFEST. If user enables the file checksum but does not provide the sst_file_checksum instance, RocksDB will use the default crc32checksum implemented in table/sst_file_checksum_crc32c.h Pull Request resolved: https://github.com/facebook/rocksdb/pull/6216 Test Plan: Added the testing case in table_test and ldb_cmd_test to verify checksum is correct in different level. Pass make asan_check. Differential Revision: D19171461 Pulled By: zhichao-cao fbshipit-source-id: b2e53479eefc5bb0437189eaa1941670e5ba8b87 | 10 February 2020, 23:52:52 UTC |
| 594e815 | sdong | 10 February 2020, 23:42:23 UTC | Make clang analyze happy with options_test (#6398) Summary: clang analysis shows following warning: options/options_test.cc:1554:24: warning: The left operand of '-' is a garbage value (file_size - 1) / readahead_size + 1); ~~~~~~~~~ ^ Explicitly initialize file_size and add an assertion to make clang analysis happy. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6398 Test Plan: Run "make analysis" and see the warning goes away. Differential Revision: D19819662 fbshipit-source-id: 1589ea91c0c8f78242538f01448e4ad0e5fbc219 | 10 February 2020, 23:50:25 UTC |
| b2bc1da | sdong | 10 February 2020, 21:35:04 UTC | Try to fix some analysis failures Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6384 Test Plan: Wait and see the analysis result. Differential Revision: D19781072 fbshipit-source-id: 75e7cb6ee619ebd289841eaabea03dd075c09d3b | 10 February 2020, 21:37:14 UTC |
| 2e0159e | Yutian Li | 10 February 2020, 20:31:12 UTC | Add error status for no_slowdown & low priority write (#6396) Summary: When `no_slowdown` is enabled, it returns `Status::Incomplete("Write stall")` if a stall would occur. This patch adds descriptive text for when `no_slowdown` and `low_pri` are enabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6396 Differential Revision: D19808978 Pulled By: cheng-chang fbshipit-source-id: a53b0d25ed414c821a086531e0222027f925e627 | 10 February 2020, 20:33:16 UTC |
| debc4ef | Kefu Chai | 10 February 2020, 19:29:58 UTC | utilities/env_librados: copy use bufferlist::iterator (#6395) Summary: to adapt the change in ceph upstream where the bufferlist::copy() method was removed in https://github.com/ceph/ceph/commit/c724369010a753bd44e11a534d1f42156c4fc12d Signed-off-by: Kefu Chai <tchaikov@gmail.com> Pull Request resolved: https://github.com/facebook/rocksdb/pull/6395 Differential Revision: D19816815 Pulled By: zhichao-cao fbshipit-source-id: 9210767b91af0ecdcf5dfaa3e70edcaeea55135f | 10 February 2020, 19:31:16 UTC |
| 84b41a6 | blackyblack | 08 February 2020, 01:06:43 UTC | Only add gtest target when building with tests (#6377) Summary: External CMAKE projects cannot add own gtest target due to CMP0002 policy. We should not add gtest target unless we build with tests. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6377 Differential Revision: D19801708 fbshipit-source-id: 662484683e8737e10397ddb0d17705da5243c4f9 | 08 February 2020, 01:16:44 UTC |
| d70011b | anand76 | 08 February 2020, 00:46:32 UTC | Check KeyContext status in MultiGet (#6387) Summary: Currently, any IO errors and checksum mismatches while reading data blocks, are being ignored by the batched MultiGet. Its only looking at the GetContext state. Fix that. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6387 Test Plan: Add unit tests Differential Revision: D19799819 Pulled By: anand1976 fbshipit-source-id: 46133dccbb04e64067b9fe6cda73e282203db969 | 08 February 2020, 00:48:16 UTC |
| 4e45727 | Robert Yang | 07 February 2020, 23:18:04 UTC | db/write_thread.cc: Initialize state (#6275) Summary: Fixed an error when compiled with -Og: db/write_thread.cc:183:14: error: 'state' may be used uninitialized in this function [-Werror=maybe-uninitialized] Signed-off-by: Robert Yang <liezhi.yang@windriver.com> Pull Request resolved: https://github.com/facebook/rocksdb/pull/6275 Differential Revision: D19381755 fbshipit-source-id: a90bf3cd4a7248d9d71219e918fc6253deb97e3c | 07 February 2020, 23:20:38 UTC |
| 876c2db | sdong | 07 February 2020, 23:16:29 UTC | Allow readahead when reading option files. (#6372) Summary: Right, when reading from option files, no readahead is used and 8KB buffer is used. It might introduce high latency if the file system provide high latency and doesn't do readahead. Instead, introduce a readahead to the file. When calling inside DB, infer the value from options.log_readahead. Otherwise, a default 512KB readahead size is used. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6372 Test Plan: Add --log_readahead_size in db_bench. Run it with several options and observe read size from option files using strace. Differential Revision: D19727739 fbshipit-source-id: e6d8053b0a64259abc087f1f388b9cd66fa8a583 | 07 February 2020, 23:18:26 UTC |
| b42fa14 | Cheng Chang | 07 February 2020, 22:24:41 UTC | Support move semantics for PinnableSlice (#6374) Summary: It's logically correct for PinnableSlice to support move semantics to transfer ownership of the pinned memory region. This PR adds both move constructor and move assignment to PinnableSlice. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6374 Test Plan: A set of unit tests for the move semantics are added in slice_test. So `make slice_test && ./slice_test`. Differential Revision: D19739254 Pulled By: cheng-chang fbshipit-source-id: f898bd811bb05b2d87384ec58b645e9915e8e0b1 | 07 February 2020, 22:26:26 UTC |
| 25fbdc5 | Chad Austin | 07 February 2020, 22:18:24 UTC | Fix Buck build on macOS (#6378) Summary: liburing is a Linux-specific dependency, so make sure it's configured in the Linux-only Buck rules. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6378 Test Plan: ``` ~/fbcode $ cp internal_repo_rocksdb/repo/TARGETS rocksdb/src ~/fbcode $ buck build mode/mac eden ``` Reviewed By: chadaustin Differential Revision: D19760039 Pulled By: riversand963 fbshipit-source-id: 2abfce81c8b17965ef76012262cd117708e0294f | 07 February 2020, 22:20:12 UTC |
| 752c87a | Levi Tamasi | 07 February 2020, 21:25:07 UTC | Clean up VersionEdit a bit (#6383) Summary: This is a bunch of small improvements to `VersionEdit`. Namely, the patch * Makes the names and order of variables, methods, and code chunks related to the various information elements more consistent, and adds missing getters for the sake of completeness. * Initializes previously uninitialized stack variables. * Marks all getters const to improve const correctness. * Adds in-class initializers and removes the default ctor that would create an object with uninitialized built-in fields and call `Clear` afterwards. * Adds a new type alias for new files and changes the existing `typedef` for deleted files into a type alias as well. * Makes the helper method `DecodeNewFile4From` private. * Switches from long-winded iterator syntax to range based loops in a couple of places. * Fixes a couple of assignments where an integer 0 was assigned to boolean members. * Fixes a getter which used to return a `const std::string` instead of the intended `const std::string&`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6383 Test Plan: make check Differential Revision: D19780537 Pulled By: ltamasi fbshipit-source-id: b0b4f09fee0ec0e7c7b7a6d76bfe5346e91824d0 | 07 February 2020, 21:27:06 UTC |
| 5f478b9 | Cheng Chang | 07 February 2020, 21:17:12 UTC | Remove outdated comment (#6379) Summary: Since the logic for handling IDENTITY file is now inside `NewDB`, the comment above `NewDB` is no longer relevant. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6379 Test Plan: not needed Differential Revision: D19795440 Pulled By: cheng-chang fbshipit-source-id: 0b1cca87ac6d92474701c46aa4c8d4d708bfa19b | 07 February 2020, 21:18:43 UTC |
| 1b4be4c | Levi Tamasi | 07 February 2020, 20:49:11 UTC | BlobDB: ignore trivially moved files when updating the SST<->blob file mapping (#6381) Summary: BlobDB keeps track of the mapping between SSTs and blob files using the `OnFlushCompleted` and `OnCompactionCompleted` callbacks of the `EventListener` interface: upon receiving a flush notification, a link is added between the newly flushed SST and the corresponding blob file; for compactions, links are removed for the inputs and added for the outputs. The earlier code performed this link deletion and addition even for trivially moved files; the new code walks through the two lists together (in a fashion that's similar to merge sort) and skips such files. This should mitigate https://github.com/facebook/rocksdb/issues/6338, wherein an assertion is triggered with the earlier code when a compaction notification for a trivial move precedes the flush notification for the moved SST. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6381 Test Plan: make check Differential Revision: D19773729 Pulled By: ltamasi fbshipit-source-id: ae0f273ded061110dd9334e8fb99b0d7786650b0 | 07 February 2020, 20:50:57 UTC |
| 107a7ca | Cheng Chang | 07 February 2020, 20:31:28 UTC | Remove inappropriate comments (#6371) Summary: The comments are for iterators, not Cleanable. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6371 Test Plan: no need Differential Revision: D19727527 Pulled By: cheng-chang fbshipit-source-id: c74aeffa27ea0ce15a36ff6f9694826712cd1c70 | 07 February 2020, 20:35:24 UTC |
| 0a74e1b | Cheng Chang | 07 February 2020, 20:29:41 UTC | Add status checks during DB::Open (#6380) Summary: Several statuses were not checked during DB::Open. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6380 Test Plan: make check Differential Revision: D19780237 Pulled By: cheng-chang fbshipit-source-id: c8d189d20344bd1607890dd1449345bda2ef96b9 | 07 February 2020, 20:32:09 UTC |
| f361ced | Yanqin Jin | 07 February 2020, 18:50:17 UTC | Atomic flush rollback once on failure (#6385) Summary: Before this fix, atomic flush codepath may hit an assertion failure on a specific failure case. If all flush jobs within an atomic flush succeed (they do not write to MANIFEST), but batch writing version edits to MANIFEST fails, then `cfd->imm()->RollbackMemTableFlush()` will be called twice, and the second invocation hits assertion failure `assert(m->flush_in_progress_)` since the first invocation resets the variable `flush_in_progress_` to false already. Test plan (dev server): ``` ./db_flush_test --gtest_filter=DBAtomicFlushTest/DBAtomicFlushTest.RollbackAfterFailToInstallResults make check ``` Both must succeed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6385 Differential Revision: D19782943 Pulled By: riversand963 fbshipit-source-id: 84e1592625e729d1b70fdc8479959387a74cb121 | 07 February 2020, 18:52:10 UTC |
| c6f7551 | atul | 06 February 2020, 18:29:06 UTC | Fixing the documentation of the function (#4803) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6354 Differential Revision: D19725459 Pulled By: riversand963 fbshipit-source-id: fded24576251bfa4b289399f0909f1fe43426e28 | 06 February 2020, 18:30:44 UTC |
| f5f79f0 | Cheng Chang | 06 February 2020, 18:14:19 UTC | Be able to read compatible leveldb sst files (#6370) Summary: In `DBSSTTest.SSTsWithLdbSuffixHandling`, some sst files are renamed to ldb files, the original intention of the test is to test that the ldb files can be loaded along with the sst files. The original test checks this by `ASSERT_NE("NOT_FOUND", Get(Key(k)))`, but the problem is `Get(Key(k))` returns IO error due to path not found instead of NOT_FOUND, so the success of ASSERT_NE does not mean the key can be retrieved. This PR updates the test to make sure Get(Key(k)) returns the original value. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6370 Test Plan: make db_sst_test && ./db_sst_test Differential Revision: D19726278 Pulled By: cheng-chang fbshipit-source-id: 993127f56457b315e669af4eeb92d6f956b7a4b7 | 06 February 2020, 18:15:44 UTC |
| 24c9dce | sdong | 06 February 2020, 04:59:13 UTC | Remove include math.h (#6373) Summary: We see some odd errors complaining math. However, it doesn't seem that it is needed to be included. Remove the include of math.h. Just removing it from db_bench doesn't seem to break anything. Replacing sqrt from std::sqrt seems to work for histogram.cc Pull Request resolved: https://github.com/facebook/rocksdb/pull/6373 Test Plan: Watch Travis and appveyor to run. Differential Revision: D19730068 fbshipit-source-id: d3ad41defcdd9f51c2da1a3673fb258f5dfacf47 | 06 February 2020, 05:00:49 UTC |
| 1ed7d9b | Mike Kolupaev | 04 February 2020, 21:38:00 UTC | Avoid lots of calls to Env::GetFileSize() in SstFileManagerImpl when opening DB (#6363) Summary: Before this PR it calls GetFileSize() once for each sst file in the DB. This can take a long time if there are be tens of thousands of sst files (e.g. in thousands of column families), and even longer if Env is talking to some remote service rather than local filesystem. This PR makes DB::Open() use sst file sizes that are already known from manifest (typically almost all files in the DB) and only call GetFileSize() for non-sst or obsolete files. Note that GetFileSize() is also called and checked against manifest in CheckConsistency(), so the calls in SstFileManagerImpl were completely redundant. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6363 Test Plan: deployed to a test cluster, looked at a dump of Env calls (from a custom instrumented Env) - no more thousands of GetFileSize()s. Differential Revision: D19702509 Pulled By: al13n321 fbshipit-source-id: 99f8110620cb2e9d0c092dfcdbb11f3af4ff8b73 | 04 February 2020, 21:41:53 UTC |
| 3a07323 | sdong | 04 February 2020, 19:37:07 UTC | Consolidate ReadFileToString() (#6366) Summary: It's a minor refactoring. We have two ReadFileToString() but they are very similar. Make the one with Env argument calls the one with FS argument instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6366 Test Plan: Run all existing tests Differential Revision: D19712332 fbshipit-source-id: 5ae6fabf6355938690d95cda52afd1f39e0a7823 | 04 February 2020, 19:39:23 UTC |
| 69c8614 | sdong | 04 February 2020, 19:37:03 UTC | Avoid to get manifest file size when recovering from it. (#6369) Summary: Right now RocksDB gets manifest file size before recovering from it. The information is available in LogReader. Use it instead to prevent one file system call. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6369 Test Plan: Run all existing tests Differential Revision: D19714872 fbshipit-source-id: 0144be324d403c99e3da875ea2feccc8f64e883d | 04 February 2020, 19:39:23 UTC |
| 637e64b | Mike Kolupaev | 04 February 2020, 09:24:29 UTC | Add an option to prevent DB::Open() from querying sizes of all sst files (#6353) Summary: When paranoid_checks is on, DBImpl::CheckConsistency() iterates over all sst files and calls Env::GetFileSize() for each of them. As far as I could understand, this is pretty arbitrary and doesn't affect correctness - if filesystem doesn't corrupt fsynced files, the file sizes will always match; if it does, it may as well corrupt contents as well as sizes, and rocksdb doesn't check contents on open. If there are thousands of sst files, getting all their sizes takes a while. If, on top of that, Env is overridden to use some remote storage instead of local filesystem, it can be *really* slow and overload the remote storage service. This PR adds an option to not do GetFileSize(); instead it does GetChildren() for parent directory to check that all the expected sst files are at least present, but doesn't check their sizes. We can't just disable paranoid_checks instead because paranoid_checks do a few other important things: make the DB read-only on write errors, print error messages on read errors, etc. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6353 Test Plan: ran the added sanity check unit test. Will try it out in a LogDevice test cluster where the GetFileSize() calls are causing a lot of trouble. Differential Revision: D19656425 Pulled By: al13n321 fbshipit-source-id: c2c421b367633033760d1f56747bad206d1fbf82 | 04 February 2020, 09:27:26 UTC |
| 7330ec0 | anand76 | 04 February 2020, 02:15:12 UTC | Fix a test failure in error_handler_test (#6367) Summary: Fix an intermittent failure in DBErrorHandlingTest.CompactionManifestWriteError due to a race between background error recovery and the main test thread calling TEST_WaitForCompact(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/6367 Test Plan: Run the test using gtest_parallel Differential Revision: D19713802 Pulled By: anand1976 fbshipit-source-id: 29e35dc26e0984fe8334c083e059f4fa1f335d68 | 04 February 2020, 02:16:52 UTC |
| f195d8d | sdong | 04 February 2020, 01:38:50 UTC | Use ReadFileToString() to get content from IDENTITY file (#6365) Summary: Right now when reading IDENTITY file, we use a very similar logic as ReadFileToString() while it does an extra file size check, which may be expensive in some file systems. There is no reason to duplicate the logic. Use ReadFileToString() instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6365 Test Plan: RUn all existing tests. Differential Revision: D19709399 fbshipit-source-id: 3bac31f3b2471f98a0d2694278b41e9cd34040fe | 04 February 2020, 01:40:49 UTC |
| 36c504b | sdong | 03 February 2020, 22:11:50 UTC | Avoid create directory for every column families (#6358) Summary: A relatively recent regression causes for every CF, create and open directory is called for the DB directory, unless CF has a private directory. This doesn't scale well with large number of column families. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6358 Test Plan: Run all existing tests and see it pass. strace with db_bench --num_column_families and observe it doesn't open directory for number of column families. Differential Revision: D19675141 fbshipit-source-id: da01d9216f1dae3f03d4064fbd88ce71245bd9be | 03 February 2020, 22:13:39 UTC |
| eb4d6af | Huisheng Liu | 03 February 2020, 21:30:13 UTC | Error handler test fix (#6266) Summary: MultiDBCompactionError fails when it verifies the number of files on level 0 and level 1 without waiting for compaction to finish. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6266 Differential Revision: D19701639 Pulled By: riversand963 fbshipit-source-id: e96d511bcde705075f073e0b550cebcd2ecfccdc | 03 February 2020, 21:32:53 UTC |
| 7242dae | Adam Retter | 03 February 2020, 20:28:25 UTC | Improve RocksJava Comparator (#6252) Summary: This is a redesign of the API for RocksJava comparators with the aim of improving performance. It also simplifies the class hierarchy. **NOTE**: This breaks backwards compatibility for existing 3rd party Comparators implemented in Java... so we need to consider carefully which release branches this goes into. Previously when implementing a comparator in Java the developer had a choice of subclassing either `DirectComparator` or `Comparator` which would use direct and non-direct byte-buffers resepectively (via `DirectSlice` and `Slice`). In this redesign there we have eliminated the overhead of using the Java Slice classes, and just use `ByteBuffer`s. The `ComparatorOptions` supplied when constructing a Comparator allow you to choose between direct and non-direct byte buffers by setting `useDirect`. In addition, the `ComparatorOptions` now allow you to choose whether a ByteBuffer is reused over multiple comparator calls, by setting `maxReusedBufferSize > 0`. When buffers are reused, ComparatorOptions provides a choice of mutex type by setting `useAdaptiveMutex`. --- [JMH benchmarks previously indicated](https://github.com/facebook/rocksdb/pull/6241#issue-356398306) that the difference between C++ and Java for implementing a comparator was ~7x slowdown in Java. With these changes, when reusing buffers and guarding access to them via mutexes the slowdown is approximately the same. However, these changes offer a new facility to not reuse mutextes, which reduces the slowdown to ~5.5x in Java. We also offer a `thread_local` mechanism for reusing buffers, which reduces slowdown to ~5.2x in Java (closes https://github.com/facebook/rocksdb/pull/4425). These changes also form a good base for further optimisation work such as further JNI lookup caching, and JNI critical. --- These numbers were captured without jemalloc. With jemalloc, the performance improves for all tests, and the Java slowdown reduces to between 4.8x and 5.x. ``` ComparatorBenchmarks.put native_bytewise thrpt 25 124483.795 ± 2032.443 ops/s ComparatorBenchmarks.put native_reverse_bytewise thrpt 25 114414.536 ± 3486.156 ops/s ComparatorBenchmarks.put java_bytewise_non-direct_reused-64_adaptive-mutex thrpt 25 17228.250 ± 1288.546 ops/s ComparatorBenchmarks.put java_bytewise_non-direct_reused-64_non-adaptive-mutex thrpt 25 16035.865 ± 1248.099 ops/s ComparatorBenchmarks.put java_bytewise_non-direct_reused-64_thread-local thrpt 25 21571.500 ± 871.521 ops/s ComparatorBenchmarks.put java_bytewise_direct_reused-64_adaptive-mutex thrpt 25 23613.773 ± 8465.660 ops/s ComparatorBenchmarks.put java_bytewise_direct_reused-64_non-adaptive-mutex thrpt 25 16768.172 ± 5618.489 ops/s ComparatorBenchmarks.put java_bytewise_direct_reused-64_thread-local thrpt 25 23921.164 ± 8734.742 ops/s ComparatorBenchmarks.put java_bytewise_non-direct_no-reuse thrpt 25 17899.684 ± 839.679 ops/s ComparatorBenchmarks.put java_bytewise_direct_no-reuse thrpt 25 22148.316 ± 1215.527 ops/s ComparatorBenchmarks.put java_reverse_bytewise_non-direct_reused-64_adaptive-mutex thrpt 25 11311.126 ± 820.602 ops/s ComparatorBenchmarks.put java_reverse_bytewise_non-direct_reused-64_non-adaptive-mutex thrpt 25 11421.311 ± 807.210 ops/s ComparatorBenchmarks.put java_reverse_bytewise_non-direct_reused-64_thread-local thrpt 25 11554.005 ± 960.556 ops/s ComparatorBenchmarks.put java_reverse_bytewise_direct_reused-64_adaptive-mutex thrpt 25 22960.523 ± 1673.421 ops/s ComparatorBenchmarks.put java_reverse_bytewise_direct_reused-64_non-adaptive-mutex thrpt 25 18293.317 ± 1434.601 ops/s ComparatorBenchmarks.put java_reverse_bytewise_direct_reused-64_thread-local thrpt 25 24479.361 ± 2157.306 ops/s ComparatorBenchmarks.put java_reverse_bytewise_non-direct_no-reuse thrpt 25 7942.286 ± 626.170 ops/s ComparatorBenchmarks.put java_reverse_bytewise_direct_no-reuse thrpt 25 11781.955 ± 1019.843 ops/s ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/6252 Differential Revision: D19331064 Pulled By: pdillinger fbshipit-source-id: 1f3b794e6a14162b2c3ffb943e8c0e64a0c03738 | 03 February 2020, 20:30:13 UTC |
| 800d24d | sdong | 31 January 2020, 23:42:34 UTC | Fix DBTest2.ChangePrefixExtractor LITE build (#6356) Summary: DBTest2.ChangePrefixExtractor fails in LITE build because LITE build doesn't support adaptive build. Fix it by removing the stats check but only check correctness. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6356 Test Plan: Run the test with both of LITE and non-LITE build. Differential Revision: D19669537 fbshipit-source-id: 6d7dd6c8a79f18e80ca1636864b9c71922030d8e | 31 January 2020, 23:44:14 UTC |
| 01ab882 | Maysam Yabandeh | 31 January 2020, 23:07:27 UTC | Fix release warning for unused bg_canceled Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6357 Differential Revision: D19670931 Pulled By: maysamyabandeh fbshipit-source-id: d528c4c7f9450f1f38b9d2a36e0d5d0865b39be9 | 31 January 2020, 23:09:10 UTC |
| ec49634 | sdong | 31 January 2020, 19:00:24 UTC | Add a unit test for prefix extractor changes (#6323) Summary: Add a unit test for prefix extractor change, including a check that fails due to a bug. Also comment out the partitioned filter case which will fail the test too. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6323 Test Plan: Run the test and it passes (and fails if the SeekForPrev() part is uncommented) Differential Revision: D19509744 fbshipit-source-id: 678202ca97b5503e9de73b54b90de9e5ba822b72 | 31 January 2020, 19:02:03 UTC |
