https://github.com/facebook/rocksdb

sort by:
Revision Author Date Message Commit Date
3f6b2d7 Redesign block cache pinning API (#7520) Summary: The old flag-based APIs (`BlockBasedTableOptions::pin_l0_filter_and_index_blocks_in_cache` and `BlockBasedTableOptions::pin_top_level_index_and_filter`) were insufficient for our needs. For example, it was impossible to pin only unpartitioned meta-blocks, which could prevent block cache contention when turning on dictionary compression or during a migration to partitioned indexes/filters. It was also impossible to pin all meta-blocks in memory while having predictable memory usage via block cache. If we had continued adding flags to address these scenarios, they would have had significant overlap causing confusion. Instead, this PR deprecates the flags and starts a new API with non-overlapping options. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7520 Test Plan: - new unit test - added new options to stress/crash test and ran for a while: `$ python tools/db_crashtest.py blackbox --simple --max_key=1000000 -write_buffer_size=1048576 -target_file_size_base=1048576 -max_bytes_for_level_base=4194304 --interval=10 -value_size_mult=33 -column_families=1 -reopen=0` Reviewed By: pdillinger Differential Revision: D24200034 Pulled By: ajkr fbshipit-source-id: 3fa7cfc71e7960f7a867511dd6ae5834dd73b13e 19 October 2020, 21:18:57 UTC
97bf787 Update HISTORY.md and version.h for 6.13.3 14 October 2020, 16:29:38 UTC
0bda0e3 Fix StallWrite crash with mixed of slowdown/no_slowdown writes (#7508) Summary: `BeginWriteStall()` removes no_slowdown write from the write list and updates `link_newer`, which makes `CreateMissingNewerLinks()` thought all write list has valid `link_newer` and failed to create link for all writers. It caused flaky test and SegFault for release build. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7508 Test Plan: Add unittest to reproduce the issue. Reviewed By: anand1976 Differential Revision: D24126601 Pulled By: jay-zhuang fbshipit-source-id: f8ac5dba653f7ee1b0950296427d4f5f8ee34a06 14 October 2020, 00:44:02 UTC
950b72c Update HISTORY.md and version.h for 6.13.2 13 October 2020, 16:41:35 UTC
6eccfbb Fix bug when `DeleteRange()` used with `paranoid_file_checks == true` Backports part of 750817555867a43f0e7b73dffa44756a9136c808. The unit test comes from https://github.com/facebook/rocksdb/pull/7521. 13 October 2020, 16:40:12 UTC
ed5c64e update HISTORY.md and version.h for 6.13.1 12 October 2020, 21:06:52 UTC
0dd184c fix TARGETS ran `python3 buckifier/buckify_rocksdb.py` 12 October 2020, 21:06:28 UTC
6676d31 Fix bug in pinned partitioned indexes with some reads bypassing block cache Backports part of 75d3b6fdf0aa1007c4d26382f65be0adf4519a37. 12 October 2020, 20:55:59 UTC
82c5a09 Fix bug in pinned partitioned user key indexes Backports part of 75d3b6fdf0aa1007c4d26382f65be0adf4519a37. 12 October 2020, 20:54:35 UTC
ac39f41 Add missing release note 12 October 2020, 20:54:11 UTC
d6cd5d1 Add a rocksdb lib target with link_whole=True (#7466) Summary: We would like to build a shared library with all fbcode dependencies statically linked within. This resulting .so should not drop any symbols definitions in the building process. To ensure that, we use `link_whole=True` according to https://buck.build/rule/cxx_library.html#link_whole. Since `link_whole` is `False` by default, adding a `link_whole=False` to existing libraries won't change any behavior. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7466 Test Plan: build a .so and test internally. Reviewed By: pdillinger Differential Revision: D24009780 Pulled By: riversand963 fbshipit-source-id: d18804d495da7195ed72a2040e1a5de4fd336519 05 October 2020, 18:26:16 UTC
5a86c2e Re-add extra compiler flags when building unittests (#7437) Summary: Re-add extra_compiler_flags when building unit tests for fbcode. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7437 Test Plan: Integrate with buck and run internal tests. Reviewed By: pdillinger Differential Revision: D23943924 Pulled By: riversand963 fbshipit-source-id: b92b7ad003e06e0860c45efc5f7f9684233d0c55 25 September 2020, 23:48:46 UTC
e29a510 Add Unreleased to HISTORY.md for patches 24 September 2020, 15:21:36 UTC
41a2789 Add AppendWithVerify and PositionedAppendWithVerify to Env and FileSystem (#7419) Summary: Add new AppendWithVerify and PositionedAppendWithVerify APIs to Env and FileSystem to bring the data verification information (data checksum information) from upper layer (e.g., WritableFileWriter) to the storage layer. This PR only include the API definition, no functional codes are added to unblock other developers which depend on these APIs. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7419 Test Plan: make -j32 Reviewed By: pdillinger Differential Revision: D23883196 Pulled By: zhichao-cao fbshipit-source-id: 94676c26bc56144cc32e3661f84f21eccd790411 24 September 2020, 03:44:00 UTC
2e5f8bd Fix scope of `ReadOptions` in `SstFileReader` (#7432) Summary: a4a4a2dabdb9fc8dbd8567abaa50042de84a44f4 changed the contract of `TableReader::NewIterator()` to require `ReadOptions` outlive the returned iterator. But I didn't notice that `SstFileReader` violates the new contract and needs to be adapted. The unit test provided here exposes the problem when run under ASAN. ``` $ ./sst_file_reader_test --gtest_filter=SstFileReaderTest.ReadOptionsOutOfScope Note: Google Test filter = SstFileReaderTest.ReadOptionsOutOfScope [==========] Running 1 test from 1 test case. [----------] Global test environment set-up. [----------] 1 test from SstFileReaderTest [ RUN ] SstFileReaderTest.ReadOptionsOutOfScope ================================================================= ==3238048==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffd6189e158 at pc 0x000001298350 bp 0x7ffd6189c280 sp 0x7ffd6189c278 READ of size 8 at 0x7ffd6189e158 thread T0 #0 0x129834f in rocksdb::BlockBasedTableIterator::InitDataBlock() table/block_based/block_based_table_iterator.cc:236 https://github.com/facebook/rocksdb/issues/1 0x12b01f7 in rocksdb::BlockBasedTableIterator::SeekImpl(rocksdb::Slice const*) table/block_based/block_based_table_iterator.cc:77 https://github.com/facebook/rocksdb/issues/2 0x844d28 in rocksdb::IteratorWrapperBase<rocksdb::Slice>::SeekToFirst() table/iterator_wrapper.h:116 https://github.com/facebook/rocksdb/issues/3 0x844d28 in rocksdb::DBIter::SeekToFirst() db/db_iter.cc:1352 https://github.com/facebook/rocksdb/issues/4 0x52482b in rocksdb::SstFileReaderTest_ReadOptionsOutOfScope_Test::TestBody() table/sst_file_reader_test.cc:150 https://github.com/facebook/rocksdb/issues/5 0x5f433c in void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3899 https://github.com/facebook/rocksdb/issues/6 0x5f433c in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3935 https://github.com/facebook/rocksdb/issues/7 0x5cc2de in testing::Test::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3973 https://github.com/facebook/rocksdb/issues/8 0x5cc988 in testing::Test::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3965 https://github.com/facebook/rocksdb/issues/9 0x5cc988 in testing::TestInfo::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:4149 https://github.com/facebook/rocksdb/issues/10 0x5cce9a in testing::TestInfo::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:4124 https://github.com/facebook/rocksdb/issues/11 0x5cce9a in testing::TestCase::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:4267 https://github.com/facebook/rocksdb/issues/12 0x5ce696 in testing::TestCase::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:4253 https://github.com/facebook/rocksdb/issues/13 0x5ce696 in testing::internal::UnitTestImpl::RunAllTests() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:6633 https://github.com/facebook/rocksdb/issues/14 0x5f541c in bool testing::internal::HandleSehExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3899 https://github.com/facebook/rocksdb/issues/15 0x5f541c in bool testing::internal::HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>(testing::internal::UnitTestImpl*, bool (testing::internal::UnitTestImpl::*)(), char const*) third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:3935 https://github.com/facebook/rocksdb/issues/16 0x5cee74 in testing::UnitTest::Run() third-party/gtest-1.8.1/fused-src/gtest/gtest-all.cc:6242 https://github.com/facebook/rocksdb/issues/17 0x4c0332 in RUN_ALL_TESTS() third-party/gtest-1.8.1/fused-src/gtest/gtest.h:22104 https://github.com/facebook/rocksdb/issues/18 0x4c0332 in main table/sst_file_reader_test.cc:213 https://github.com/facebook/rocksdb/issues/19 0x7fb0263281a5 in __libc_start_main (/usr/local/fbcode/platform007/lib/libc.so.6+0x211a5) https://github.com/facebook/rocksdb/issues/20 0x523e56 (/data/users/andrewkr/rocksdb/sst_file_reader_test+0x523e56) Address 0x7ffd6189e158 is located in stack of thread T0 at offset 568 in frame #0 0x52428f in rocksdb::SstFileReaderTest_ReadOptionsOutOfScope_Test::TestBody() table/sst_file_reader_test.cc:131 This frame has 9 object(s): [32, 40) 'reader' [96, 104) '<unknown>' [160, 168) '<unknown>' [224, 232) 'iter' [288, 304) 'gtest_ar' [352, 368) '<unknown>' [416, 440) 'keys' [480, 512) '<unknown>' [544, 680) 'ropts' <== Memory access at offset 568 is inside this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) AddressSanitizer: stack-use-after-scope table/block_based/block_based_table_iterator.cc:236 in rocksdb::BlockBasedTableIterator::InitDataBlock() ... ``` The fix is to use `ArenaWrappedDBIter` which has support for holding a `ReadOptions` in an `Arena` whose lifetime is tied to the iterator. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7432 Test Plan: verified the provided unit test no longer fails Reviewed By: pdillinger Differential Revision: D23880043 Pulled By: ajkr fbshipit-source-id: 9464c37408f7bd7c9c4a90ceffb04d9f0ca7a494 23 September 2020, 22:28:50 UTC
e041a40 Add and fix clang -Wshift-sign-overflow (#7431) Summary: This option is apparently used by some teams within Facebook (internal ref T75998621) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7431 Test Plan: USE_CLANG=1 make check before (fails) and after Reviewed By: jay-zhuang Differential Revision: D23876584 Pulled By: pdillinger fbshipit-source-id: abb8b67a1f1aac75327944d266e284b2b6727191 23 September 2020, 22:28:47 UTC
74be8f3 Fix/minimize mock_time_env.h dependencies (#7426) Summary: (a) own copy of kMicrosInSecond (b) out-of-line sync point code Pull Request resolved: https://github.com/facebook/rocksdb/pull/7426 Test Plan: FB internal Reviewed By: ajkr Differential Revision: D23861363 Pulled By: pdillinger fbshipit-source-id: de6b1621dca2f7391c5ff72bad04a7613dc27527 23 September 2020, 18:42:15 UTC
cda8a74 Less I/O for incremental backups, slightly better corruption detection (#7413) Summary: Two relatively simple functional changes to incremental backup behavior, integrated with a minor refactoring to reduce code redundancy and improve error/log message. There are nuances to the impact of these changes, but I believe they are fundamentally good and generally safe. Those functional changes: * Incremental backups no longer read DB table files that are already saved to a shared part of the backup directory, unless `share_files_with_checksum` is used with `kLegacyCrc32cAndFileSize` naming (discouraged) where crc32c full file checksums are needed to determine file naming. * Justification: incremental backups should not need to read the whole DB, especially without rate limiting. (Although other BackupEngine reads are not rate limited either, other non-trivial reads are generally limited by a corresponding write, as in copying files.) Also, the fact that this is not already fixed was arguably a bug/oversight in the implementation of https://github.com/facebook/rocksdb/issues/7110. * When considering whether a table file is already backed up in a shared part of backup directory, BackupEngine would already query the sizes of source (DB) and pre-existing destination (backup) files. BackupEngine now uses these file sizes to detect corruption, as at least one of (a) old backup, (b) backup in progress, or (c) current DB is corrupt if there's a size mismatch. * Justification: a random related fix that also helps to cover a small hole in corruption checking uncovered by the other functional change: * For `share_table_files` without "checksum" (not recommended), the other change regresses in detecting fundamentally unsafe use of this option combination: when you might generate different versions of same SST file number. As demonstrated by `BackupableDBTest.FailOverwritingBackups,` this regression is greatly mitigated by the new file size checking. Nevertheless, almost no reason to use `share_files_with_checksum=false` should remain, and comments are updated appropriately. Also, this change renames internal function `CalculateChecksum` to `ReadFileAndComputeChecksum` to make the performance impact of this function clear in code reviews. It is not clear what 'same_path' is for in backupable_db.cc, and I suspect it cannot be true for a DB with unique file names (like DBImpl). Nevertheless, I've tried to keep its functionality intact when `true` to minimize risk for now, despite having no unit tests for which it is true. Select impact details (much more in unit tests): For `share_files_with_checksum`, I am confident there is no regression (vs. pre-6.12) in detecting DB or backup corruption at backup creation time, mostly because the old design did not leverage this extra checksum computation for detecting inconsistencies at backup creation time. (With computed checksums in names, a recently corrupted file just looked like a different file vs. what was already backed up.) Even in the hypothetical case of DB session id collision (~100 bits entropy collision), file size in name and/or our file size check add an extra layer of protection against false success in creating an accurate new backup. (Unit test included.) `DB::VerifyChecksum` and `BackupEngine::VerifyBackup` with checksum checking are still able to catch corruptions that `CreateNewBackup` does not. Note that when custom file checksum support is added to BackupEngine, that will essentially give the same power as `DB::VerifyChecksum` into `CreateNewBackup`. We could add options for `CreateNewBackup` to cover some of what would be caught by `VerifyBackup` with checksum checking. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7413 Test Plan: Two new unit tests included, both of which fail without these changes. Although we don't test the I/O improvement directly, we test it indirectly in DB corruption detection power that was inadvertently unlocked with new backup file naming PLUS computing current content checksums (now removed). (I don't think that case of DB corruption detection justifies reading the whole DB on incremental backup.) Reviewed By: zhichao-cao Differential Revision: D23818480 Pulled By: pdillinger fbshipit-source-id: 148aff16f001af5b9fd4b22f155311c2461f1bac 22 September 2020, 17:20:22 UTC
fb98398 Postponing custom checksum support in BackupEngine (#7411) Summary: This change reverts BackupEngine to 6.12 state to accommodate a higher-priority fix that does not easily merge with this custom checksum support. We intend to reinstate this support soon, by merging a revert of this change. For backupable_db_test, I've removed the tests depending on this feature. I've also removed relevant HISTORY.md entry. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7411 Test Plan: unit tests Reviewed By: ajkr Differential Revision: D23793835 Pulled By: pdillinger fbshipit-source-id: 7e861436539584799b13d1a8ae559b81b6d08052 22 September 2020, 17:18:30 UTC
2dbb90a Update HISTORY.md for #7346 (#7417) Summary: Copied from Andrew's entry for 6.12.3. Inserted here retroactive to 6.12 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7417 Test Plan: no code change Reviewed By: jay-zhuang Differential Revision: D23815980 Pulled By: pdillinger fbshipit-source-id: 3c8a052cdb61be1215d311556c9487f9ea5c8cb0 21 September 2020, 16:49:45 UTC
b4f29f7 Restore file size in backup table file names (and other cleanup) (#7400) Summary: Prior to 6.12, backup files using share_files_with_checksum had the file size encoded in the file name, after the last '\_' and before the last '.'. We considered this an implementation detail subject to change, and indeed removed this information from the file name (with an option to use old behavior) because it was considered ineffective/inefficient for file name uniqueness. However, some downstream RocksDB users were relying on this information since the file size is not explicitly in the backup manifest file. This primary purpose of this change is "retrofitting" the 6.12 release (not yet a public release) to simultaneously support the benefits of the new naming scheme (I/O performance and data correctness at scale) and preserve the file size information, both as default behaviors. With this change, we are essentially making the file size information encoded in the file name an official, though obscure, extension of the backup meta file format. We preserve an option (kLegacyCrc32cAndFileSize) to use the original "legacy" naming scheme, with its caveats, and make it easy to omit the file size information (no kFlagIncludeFileSize), for more compact file names. But note that changing the naming scheme used on an existing db and backup directory can lead to transient space amplification, as some files will be stored under two names in the shared_checksum directory. Because some backups were saved using the original 6.12 naming scheme, we offer two ways of dealing with those files: SST files generated by older 6.12 versions can either use the default naming scheme in effect when the SST files were generated (kFlagMatchInterimNaming, default, no transient space amplification) or can use a new naming scheme (no kFlagMatchInterimNaming, potential space amplification because some already stored files getting a new name). We don't have a natural way to detect which files were generated by previous 6.12 versions, but this change hacks one in by changing DB session ids to now use a more concise encoding, reducing file name length, saving ~dozen bytes from SST files, and making them visually distinct from DB ids so that they are less likely to be mixed up. Two final auxiliary notes: Recognizing that the backup file names have become a de facto part of the backup meta schema, this change makes them easier to parse and extend by putting a distinct marker, 's', before DB session ids embedded in the name. When we extend this to allow custom checksums in the name, they can get their own marker to ensure safe parsing. For backward compatibility, file size does not get a marker but is assumed for `_[0-9]+[.]` Another change from initial 6.12 default behavior is never including file custom checksum in the file name. Looking ahead to 6.13, we do not want the default behavior to cause backup space amplification for someone turning on file custom checksum checking in BackupEngine; we want that to be an easy decision. When implemented, including file custom checksums in backup file names will be a non-default option. Actual file name patterns and priorities, as regexes: kLegacyCrc32cAndFileSize OR pre-6.12 SST file -> [0-9]+_[0-9]+_[0-9]+[.]sst kFlagMatchInterimNaming set (default) AND early 6.12 SST file -> [0-9]+_[0-9a-fA-F-]+[.]sst kUseDbSessionId AND NOT kFlagIncludeFileSize -> [0-9]+_s[0-9A-Z]{20}[.]sst kUseDbSessionId AND kFlagIncludeFileSize (default) -> [0-9]+_s[0-9A-Z]{20}_[0-9]+[.]sst We might add opt-in options for more '\_' separated data in the name, but embedded file size, if present, will always be after last '\_' and before '.sst'. This change was originally applied to version 6.12. (See https://github.com/facebook/rocksdb/issues/7390) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7400 Test Plan: unit tests included. Sync point callbacks are used to mimic previous version SST files. Reviewed By: ajkr Differential Revision: D23759587 Pulled By: pdillinger fbshipit-source-id: f62d8af4e0978de0a34f26288cfbe66049b70025 21 September 2020, 15:05:23 UTC
731f022 fix the flaky test failure (#7415) Summary: Fix the flaky test failure in error_handler_fs_test. Add the sync point, solve the dependency. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7415 Test Plan: make asan_check, ~/gtest-parallel/gtest-parallel -r 100 ./error_handler_fs_test Reviewed By: siying Differential Revision: D23804330 Pulled By: zhichao-cao fbshipit-source-id: 5175108651f7652e47e15978f2a9c1669ef59d80 21 September 2020, 14:59:48 UTC
a02495c Map retryable IO error during Flush without WAL to soft error and no switch memtable during resume (#7310) Summary: In the current implementation, any retryable IO error happens during Flush is mapped to a hard error. In this case, DB is stopped and write is stalled unless the background error is cleaned. In this PR, if WAL is DISABLED, the retryable IO error during FLush is mapped to a soft error. Such that, the memtable can continue receive the writes. At the same time, if auto resume is triggered, SwtichMemtable will not be called during Flush when resuming the DB to avoid to many small memtables. Testing cases are added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7310 Test Plan: adding new unit test, pass make check. Reviewed By: anand1976 Differential Revision: D23710892 Pulled By: zhichao-cao fbshipit-source-id: bc4ca50d11c6b23b60d2c0cb171d86d542b038e9 21 September 2020, 14:59:19 UTC
5a0cef9 Update HISTORY.md with IO fencing error code (#7402) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7402 Reviewed By: pdillinger Differential Revision: D23761689 Pulled By: anand1976 fbshipit-source-id: 59e10f0aaa80f6c0f5a46dc99467138c4cee0511 17 September 2020, 18:52:23 UTC
bb95ed2 Add a new IOStatus subcode to indicate that writes are fenced off (#7374) Summary: In a distributed file system, directory ownership is enforced by fencing off the previous owner once they've been preempted by a new owner. This PR adds a IOStatus subcode for ```StatusCode::IOError``` to indicate this. Once this error is returned for a file write, the DB is put in read-only mode and not allowed to resume in read-write mode. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7374 Test Plan: Add new unit tests in ```error_handler_fs_test``` Reviewed By: riversand963 Differential Revision: D23687777 Pulled By: anand1976 fbshipit-source-id: bef948642089dc0af399057864d9a8ca339e8b2f 17 September 2020, 18:51:31 UTC
ecc8ffe Update master to version 6.13 (#7378) Summary: for release fork Pull Request resolved: https://github.com/facebook/rocksdb/pull/7378 Test Plan: make check + CI Reviewed By: jay-zhuang Differential Revision: D23669163 Pulled By: pdillinger fbshipit-source-id: 14cbf95b32717c28418c71cc8e10f06733bbc49f 12 September 2020, 20:18:09 UTC
205e577 Cancel tombstone skipping during bottommost compaction (#7356) Summary: During bottommost compaction, RocksDB cannot simply drop a tombstone if this tombstone is not in the earliest snapshot. The current behavior is: RocksDB skips other internal keys (of the same user key) in the same snapshot range. In the meantime, RocksDB should check for the `shutting_down` flag. Otherwise, it is possible for a bottommost compaction that has already started running to take a long time to finish, even if the application has tried to cancel all background jobs. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7356 Test Plan: make check Reviewed By: ltamasi Differential Revision: D23663241 Pulled By: riversand963 fbshipit-source-id: 25f8e9b51bc3bfa3353cdf87557800f9d90ee0b5 12 September 2020, 00:45:43 UTC
be8445e Assert valid linked list for write group (#7375) Summary: We've seen some segfaults in db_write_test, with at least one suggesting corruption of a write group linked list. Adding an assertion to have this fail in a more specific way if that is the broken invariant. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7375 Test Plan: make check Reviewed By: jay-zhuang Differential Revision: D23638477 Pulled By: pdillinger fbshipit-source-id: a76fd677cad60a3a516bd363947bfd9ce418edc1 11 September 2020, 14:58:31 UTC
c4e2066 Fix cf_consistency_stress for backup/restore, harmonize (#7373) Summary: We can only check key on restored backup if in a stress test configuration locking the key. (Fixes mismatch seen in backup/restore with atomic flush.) TestCheckpoint used a very ugly solution to the same problem: copy-paste dozens of lines of code with some changes and removals. I removed the unnecessary implementation and made the existing one simply adaptive, like TestBackupRestore. Also made TestBackupRestore clean up dead backup/restore directories on success. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7373 Test Plan: blackbox_crash_test_with_atomic_flush for a while, blackbox_crash_test for a while, with backup and checkpoint 1 in 5k and only 1k max_keys to stress this area Reviewed By: ajkr Differential Revision: D23629057 Pulled By: pdillinger fbshipit-source-id: d7fe7e2be75aaf3cf974be9540a7c5c5de8b371b 11 September 2020, 05:55:06 UTC
92639b9 Fix checkpoint file deletion race with avoid_unnecessary_blocking_io (#7369) Summary: https://github.com/facebook/rocksdb/issues/3341 guaranteed that upon return of `GetSortedWalFiles` after `DisableFileDeletions`, all pending purges of previously obsolete WAL files will have finished. However, the addition of avoid_unnecessary_blocking_io in https://github.com/facebook/rocksdb/issues/5043 opened a hole in the code making that assurance, which can lead to files to be copied for checkpoint or backup going missing before being copied, with that option enabled. This change patches the hole. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7369 Test Plan: apparent fix to backups in crash test observed. Will work on a unit test for another commit Reviewed By: ajkr Differential Revision: D23620258 Pulled By: pdillinger fbshipit-source-id: bea36b461a5b719c3e3ef802f967bc3e8ae71614 11 September 2020, 05:35:25 UTC
5ce246c Expose the start of the expiration range for TTL blob files through LiveFileMetaData (#7365) Summary: The patch adds support for exposing the start of the expiration range for TTL blob files through the `GetLiveFilesMetaData` API. This can be used for monitoring purposes, i.e. to make sure TTL blob files are deleted in a timely manner. The patch also fixes a couple of uninitialized variable issues. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7365 Test Plan: `make check` Reviewed By: pdillinger Differential Revision: D23605465 Pulled By: ltamasi fbshipit-source-id: 97a9612bf5f4b058423debdd3f28f576bb23a70f 10 September 2020, 18:33:33 UTC
ec5add3 Implement Java API for ConcurrentTaskLimiter class and compaction_thread_limiter field in ColumnFamilyOptions (#7347) Summary: as title Pull Request resolved: https://github.com/facebook/rocksdb/pull/7347 Test Plan: unit tests included Reviewed By: jay-zhuang Differential Revision: D23592552 Pulled By: pdillinger fbshipit-source-id: 1c3571b6f42bfd0cfd723ff49d01fbc02a1be45b 09 September 2020, 19:44:20 UTC
5c39d8d Add getters to the C API for flush, write, cache and compact options (#7321) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7321 Reviewed By: ajkr Differential Revision: D23590160 fbshipit-source-id: 35d106e732ac37f674222759cdb1dbb31e005ca7 09 September 2020, 18:45:27 UTC
e314935 More backup/restore stress test fixes (#7361) Summary: (a) Missed a case in updating handling of rand_keys (b) Only opening restored db with DB::Open so don't (yet) attempt to open restored BlobDB or TransactionDB. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7361 Test Plan: better than being broken Reviewed By: ajkr Differential Revision: D23592570 Pulled By: pdillinger fbshipit-source-id: dd1d999bcc0c852ee77cb6041964ec4abc0fd4fd 09 September 2020, 18:19:05 UTC
7b1d6c4 Fix the handling of the case when a blob file with a lower number gets added in VersionBuilder (#7349) Summary: When multiple background jobs are generating blob files in parallel, it is actually possible for a blob file to be added with a file number that is lower than the highest one in the base version. (This is a harmless race condition.) The patch fixes the handling of this case and adds a unit test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7349 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23542453 Pulled By: ltamasi fbshipit-source-id: 4ff6f3654bc58c391d10b9870e1cc40b5e3fa8e4 09 September 2020, 17:25:12 UTC
a7fde87 Fix platform_dependent in Travis, rebalance groups (#7360) Summary: Was broken by https://github.com/facebook/rocksdb/issues/6660 Travis times before this change, after 6660: platform_dependent: 17 min group 1: 15 min group 2: 44 min (often timeout on non-x86 or non-Linux) group 3: 31 min group 4: 21 min After this change: TODO Pull Request resolved: https://github.com/facebook/rocksdb/pull/7360 Test Plan: CI inspection Reviewed By: ajkr Differential Revision: D23586917 Pulled By: pdillinger fbshipit-source-id: 4c67cf33180b0b833c39a817e6c1f128727941d2 09 September 2020, 16:49:24 UTC
a6ac51b Fix db_bench_tool_test. Fixes 7341 (#7344) Summary: 1. Failed to compile because of use of FileSystem* instead of Env* to some methods; 2. Failed to compile with addition of ConfigOptions to some methods 3. Failed to run successfully because the database and/or db_bench would change some of the options, invalidating the comparison 4. Failed to run successfully if Snappy was not available. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7344 Reviewed By: siying Differential Revision: D23501093 Pulled By: jay-zhuang fbshipit-source-id: 81fd947e95fff9db8a4c5ff419d69d4c36bef23f 09 September 2020, 16:07:16 UTC
f1e99b3 tests need linked with third_party libs (#7351) Summary: To fix the cmake build with third_party libs, like: `mkdir build && cd build && cmake .. -DWITH_SNAPPY=1 && make` Error: ``` Undefined symbols for architecture x86_64: "snappy::RawCompress(char const*, unsigned long, char*, unsigned long*)" ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7351 Reviewed By: pdillinger Differential Revision: D23553705 Pulled By: jay-zhuang fbshipit-source-id: 19b45c6763c7256107583e8af4c01d370ca06128 09 September 2020, 16:02:34 UTC
9de912d Fix some errors showing up in Travis builds (#7359) Summary: Also enables a pull request to trigger all the Travis configurations by writing FULL_CI in the commit message. (See what I did there?) First issue make: *** No rule to make target 'jl/util/crc32c_ppc_asm.o', needed by 'rocksdbjava'. Stop. Second issue tools/db_bench_tool.cc:5514:38: error: ‘gen_exp.rocksdb::Benchmark::GenerateTwoTermExpKeys::keyrange_size_’ may be used uninitialized in this function Pull Request resolved: https://github.com/facebook/rocksdb/pull/7359 Test Plan: CI Reviewed By: zhichao-cao Differential Revision: D23582132 Pulled By: pdillinger fbshipit-source-id: 06d794673fd522ba11cf6398385387e6bd97ef89 08 September 2020, 22:11:47 UTC
0de335e Use FSRandomRWFilePtr Object to call underlying file system. (#7198) Summary: Replace FSRandomRWFile pointer with FSRandomRWFilePtr object in the rocksdb internal code. This new object wraps FSRandomRWFile pointer. Objective: If tracing is enabled, FSRandomRWFile object returns FSRandomRWFileTracingWrapper pointer that includes all necessary information in IORecord and calls underlying FileSystem and invokes IOTracer to dump that record in a binary file. If tracing is disabled then, underlying FileSystem pointer is returned directly. FSRandomRWFilePtr wrapper class is added to bypass the FSRandomRWFileWrapper when tracing is disabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7198 Test Plan: make check -j64 Reviewed By: anand1976 Differential Revision: D23421116 Pulled By: akankshamahajan15 fbshipit-source-id: 8a5ba0e7d9c1ba34c3a6f29829b107c5f09ab6a3 08 September 2020, 19:21:58 UTC
8a8a01c Fix compile error for old gcc-4.8 (#7358) Summary: gcc-4.8 returns error when using the constructor. Not sure if it's a compiler bug/limitation or code issue: ``` table/block_based/block_based_table_reader.cc:3183:67: error: use of deleted function ‘rocksdb::WritableFileStringStreamAdapter::WritableFileStringStreamAdapter(rocksdb::WritableFileStringStreamAdapter&&)’ ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7358 Reviewed By: pdillinger Differential Revision: D23577651 Pulled By: jay-zhuang fbshipit-source-id: b0197e3d3538da61a6f3866410d88d2047fb9695 08 September 2020, 19:09:34 UTC
8307d44 Update HISTORY.md for PR7329 (#7355) Summary: As title. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7355 Reviewed By: pdillinger Differential Revision: D23566635 Pulled By: riversand963 fbshipit-source-id: f8d846bcff637e7617b764b7bfb9a948ea18d195 08 September 2020, 18:10:25 UTC
b175ece Store FSWritableFilePtr object in WritableFileWriter (#7193) Summary: Replace FSWritableFile pointer with FSWritableFilePtr object in WritableFileWriter. This new object wraps FSWritableFile pointer. Objective: If tracing is enabled, FSWritableFile Ptr returns FSWritableFileTracingWrapper pointer that includes all necessary information in IORecord and calls underlying FileSystem and invokes IOTracer to dump that record in a binary file. If tracing is disabled then, underlying FileSystem pointer is returned directly. FSWritableFilePtr wrapper class is added to bypass the FSWritableFileWrapper when tracing is disabled. Test Plan: make check -j64 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7193 Reviewed By: anand1976 Differential Revision: D23355915 Pulled By: akankshamahajan15 fbshipit-source-id: e62a27a13c1fd77e36a6dbafc7006d969bed25cf 08 September 2020, 17:56:08 UTC
4e258d3 Fix backup/restore in stress/crash test (#7357) Summary: (1) Skip check on specific key if restoring an old backup (small minority of cases) because it can fail in those cases. (2) Remove an old assertion about number of column families and number of keys passed in, which is broken by atomic flush (cf_consistency) test. Like other code (for better or worse) assume a single key and iterate over column families. (3) Apply mock_direct_io to NewSequentialFile so that db_stress backup works on /dev/shm. Also add more context to output in case of backup/restore db_stress failure. Also a minor fix to BackupEngine to report first failure status in creating new backup, and drop another clue about the potential source of a "Backup failed" status. Reverts "Disable backup/restore stress test (https://github.com/facebook/rocksdb/issues/7350)" Pull Request resolved: https://github.com/facebook/rocksdb/pull/7357 Test Plan: Using backup_one_in=10000, "USE_CLANG=1 make crash_test_with_atomic_flush" for 30+ minutes "USE_CLANG=1 make blackbox_crash_test" for 30+ minutes And with use_direct_reads with TEST_TMPDIR=/dev/shm/rocksdb Reviewed By: riversand963 Differential Revision: D23567244 Pulled By: pdillinger fbshipit-source-id: e77171c2e8394d173917e36898c02dead1c40b77 08 September 2020, 17:50:19 UTC
423d051 Clean up SubcompactionState a bit (#7322) Summary: The patch cleans up a few things in `CompactionJob::SubcompactionState`: * Instead of using both the member initializer list and in-class initializers (and sometimes both at the same time for the same member), the struct now uniformly uses the latter to initialize integer members. * The default parameter value for the constructor parameter `size` is removed. * The explicitly deleted copy operations are removed, since they are implicitly deleted anyways because of the `unique_ptr` members. * The handwritten move operations, which did not move the member `c_iter` and were not declared `nothrow`, are removed. Note that with the user-declared copy operations gone (see the previous item), we can rely on the compiler to (correctly) generate these methods. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7322 Test Plan: `make check` Reviewed By: siying Differential Revision: D23382408 Pulled By: ltamasi fbshipit-source-id: a4ae5af150161c50ff7bdc07fa145482d0150bfe 08 September 2020, 16:24:23 UTC
ab202e8 Add a new stats level to exclude tickers (#7329) Summary: Currently, application may pass a statistics object to db but later wants to reduce stats tracking overhead by setting stats level to kExceptHistogramOrTimers (the current lowest level). Tickers will still be incremented, causing up to 1% CPU. We can add a new lowest stats level `kExceptTickers` to disable ticker incrementing as well, thus reducing CPU cycles spent on tickers. Test Plan (devserver): ``` make check make clean DEBUG_LEVEL=0 make db_bench ./db_bench -perf_level=1 -stats_level=0 -statistics -benchmarks=fillseq,readrandom -duration=120 ``` Measure CPU util (%) before and after change: CPU util by rocksdb::RecordTick: 1.1 vs (<0.1) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7329 Reviewed By: pdillinger Differential Revision: D23434014 Pulled By: riversand963 fbshipit-source-id: 72ff0f02a192ac476d4b0044b9f37fd4a22ff0d4 05 September 2020, 06:25:03 UTC
27aa443 Add sst_file_dumper status check (#7315) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7315 Test Plan: `ASSERT_STATUS_CHECKED=1 make sst_dump_test && ./sst_dump_test` And manually run `./sst_dump --file=*.sst` before and after the change. Reviewed By: pdillinger Differential Revision: D23361669 Pulled By: jay-zhuang fbshipit-source-id: 5bf51a2a90ee35c8c679e5f604732ec2aef5949a 05 September 2020, 02:26:42 UTC
ef32f11 Disable backup/restore stress test (#7350) Summary: Seems it's causing some tests failures. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7350 Reviewed By: riversand963 Differential Revision: D23544109 Pulled By: jay-zhuang fbshipit-source-id: 798a0ca374a20b6c2d0f29582729ff101c6a2e99 04 September 2020, 18:58:18 UTC
06ad5dd Add file checksum to stress/crash test (#7343) Summary: This change has the crash test randomly select from a few file checksum implementations, or nullptr, for DB file_checksum_gen_factory. For compatibility across runs on same DB, each non-null factory can understand all the other functions, but the default changes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7343 Test Plan: 'make blackbox_crash_test' for a while, including with some debug output to ensure code is being exercised. Reviewed By: zhichao-cao Differential Revision: D23494580 Pulled By: pdillinger fbshipit-source-id: 73bbc7ca32c1adaf619134c0c830f12894880b8a 04 September 2020, 06:50:33 UTC
3f9b756 Fix wrong level args (#7346) Summary: The level args should be output level instead of input levels. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7346 Test Plan: make check Reviewed By: ajkr Differential Revision: D23506373 Pulled By: cheng-chang fbshipit-source-id: b2f701d44c13581c5c10c4dbebded4fcd354d641 04 September 2020, 06:17:37 UTC
499c944 Fix, enable, and enhance backup/restore in db_stress (#7348) Summary: Although added to db_stress, testing of backup/restore was never integrated into the crash test, originally concerned about performance. I've enabled it now and to address the peformance concern, testing backup/restore is always skipped once the db exceeds a certain size threshold, default 100MB. This should provide sufficient opportunity for testing BackupEngine without bogging down everything else with heavier and heavier operations. Also fixed backup/restore in db_stress by making sure PurgeOldBackups can remove manifest files, which are normally kept around for db_stress. Added more coverage of backup options, and up to three backups being saved in one backup directory (in some cases). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7348 Test Plan: ran 'make blackbox_crash_test' for a while, with heightened probabilitly of taking backups (1/10k). Also confirmed with some debug output that the code is being covered, TestBackupRestore only takes a few seconds to complete when triggered, and even at 1/10k and ~50MB database, there's <,~ 1 thread testing backups at any time. Reviewed By: ajkr Differential Revision: D23510835 Pulled By: pdillinger fbshipit-source-id: b6b8735591808141f81f10773ac31634cf03b6c0 04 September 2020, 03:13:15 UTC
5746767 add `ldb unsafe_remove_sst_file` subcommand (#7335) Summary: This is adapted from https://github.com/facebook/rocksdb/issues/6678 but takes a different approach, avoiding opening a read-write DB and avoiding the `DeleteFile()` API. First, this PR refactors how options variables are initialized in `ldb` so it can be reused in a subcommand that doesn't open a DB: - Separated remaining option initialization logic out of `OpenDB()`. The new `PrepareOptions()` function initializes the full options state. - Fixed an old TODO about applying the subcommand CF option overrides to the proper `ColumnFamilyOptions` object. Second, this PR adds the `ldb unsafe_remove_sst_file` subcommand. It uses the `VersionSet`-level APIs to remove the file with the specified number. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7335 Test Plan: played with interactive python and this file removal command. Verified openability/correct results in case of multiple column families, multiple levels, etc. Reviewed By: pdillinger Differential Revision: D23454575 Pulled By: ajkr fbshipit-source-id: 039b7a8cbfc42fd123dcb25821eef51d61148afe 03 September 2020, 23:54:51 UTC
40e97b0 add warning on `DeleteFile()` API (#7337) Summary: Since we can't land https://github.com/facebook/rocksdb/issues/7336 until the next major release, added a strong warning against the `DeleteFile()` API in the meantime. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7337 Reviewed By: pdillinger Differential Revision: D23459728 Pulled By: ajkr fbshipit-source-id: 326cb9b18190386080c35c761a8736d8a877dafb 03 September 2020, 23:42:01 UTC
af54c40 fix SstFileWriter with dictionary compression (#7323) Summary: In block-based table builder, the cut-over from buffered to unbuffered mode involves sampling the buffered blocks and generating a dictionary. There was a bug where `SstFileWriter` passed zero as the `target_file_size` causing the cutover to happen immediately, so there were no samples available for generating the dictionary. This PR changes the meaning of `target_file_size == 0` to mean buffer the whole file before cutting over. It also adds dictionary compression support to `sst_dump --command=recompress` for easy evaluation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7323 Reviewed By: cheng-chang Differential Revision: D23412158 Pulled By: ajkr fbshipit-source-id: 3b232050e70ef3c2ee85a4b5f6fadb139c569873 03 September 2020, 22:49:57 UTC
5b1ccdc Expose rocksdb_open_column_families_with_ttl C function (#7314) Summary: This PR creates `rocksdb_open_column_families_with_ttl` which allows C API users to open a DBWithTLL with column families. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7314 Reviewed By: cheng-chang Differential Revision: D23430287 Pulled By: ajkr fbshipit-source-id: 307aa21d170d1402653263a91f6f832ef76afba0 03 September 2020, 21:39:58 UTC
d0c1a01 Avoid converting MERGES to PUTS when allow_ingest_behind is true (#7166) Summary: - Closes https://github.com/facebook/rocksdb/issues/6490 - Currently MERGEs are converted to PUTs at bottom or compaction has reached the beginning of the key, this can wrongly cover a PUT future base case. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7166 Test Plan: - Automated: `make all check` - Manual: With `allow_ingest_behind = true`, add Merge operations to a key then run compaction. Then run ingesting external files to make sure the base case is probably compacted with existing Merges. Reviewed By: cheng-chang Differential Revision: D23325425 Pulled By: ajkr fbshipit-source-id: 3eb415eb7b381b5453e45245393566153b1abb68 03 September 2020, 21:39:58 UTC
679a413 Close databases on benchmark error exits in db_bench (#7327) Summary: Delete database instances to make sure there are no loose threads running before exit(). This fixes segfaults seen when running workloads through CompositeEnvs with custom file systems. For further background on the issues arising when using CompositeEnvs, see the discussion in: https://github.com/facebook/rocksdb/pull/6878 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7327 Reviewed By: cheng-chang Differential Revision: D23433244 Pulled By: ajkr fbshipit-source-id: 4e19cf2067e3fe68c2a3fe1823f24b4091336bbe 03 September 2020, 21:36:30 UTC
c4d8838 New bit manipulation functions and 128-bit value library (#7338) Summary: These new functions and 128-bit value bit operations are expected to be used in a forthcoming Bloom filter alternative. No functional changes to production code, just new code only called by unit tests, cosmetic changes to existing headers, and fix an existing function for a yet-unused template instantiation (BitsSetToOne on something signed and smaller than 32 bits). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7338 Test Plan: Unit tests included. Works with and without TEST_UINT128_COMPAT=1 to check compatibility with and without __uint128_t. Also added that parameter to the CircleCI build build-linux-shared_lib-alt_namespace-status_checked. Reviewed By: jay-zhuang Differential Revision: D23494945 Pulled By: pdillinger fbshipit-source-id: 5c0dc419100d9df5d4d9abb153b2855d5aea39e8 03 September 2020, 16:32:59 UTC
a09c3cf Add options for forcing AVX and AVX2 instructions (#7334) Summary: This PR is set up to merge into master, but it would be great to get this into a patch release if possible. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7334 Reviewed By: jay-zhuang Differential Revision: D23476624 Pulled By: pdillinger fbshipit-source-id: c6cc02ce06e779e1e174ab0f4748e557d2ce7bc6 03 September 2020, 15:03:51 UTC
61d5a13 Fix typo: rename "bounary" to "boundary" in block.cc (#7328) Summary: Fix typo in comment for SeekForGetImpl(). Rename "bounary" to "boundary" Pull Request resolved: https://github.com/facebook/rocksdb/pull/7328 Reviewed By: riversand963 Differential Revision: D23439748 Pulled By: zhichao-cao fbshipit-source-id: 83a34c417c71a3210ce54a090d76c4d5571313f3 03 September 2020, 03:47:18 UTC
177f8bd Bound L0->Lbase fanout in dynamic leveled compaction (#7325) Summary: L0 score is based on size target and number of files. The size target used is `max_bytes_for_level_base`. However, the base level's size can dynamically expand in write burst mode. In fact, it can expand so much that L0->Lbase becomes the highest fanout in target sizes. This doesn't make sense from an efficiency perspective, so this PR bounds the L0->Lbase fanout to the smoothed level multiplier. The L0 scoring based on file count remains unchanged. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7325 Test Plan: contrived benchmark that exhibits the problem: ``` $ TEST_TMPDIR=/data/users/andrewkr/ ./db_bench -benchmarks=filluniquerandom,readrandom -write_buffer_size=1048576 -target_file_size_base=1048576 -max_bytes_for_level_base=4194304 -level0_file_num_compaction_trigger=4 -level_compaction_dynamic_level_bytes=true -compression_type=none -max_background_jobs=12 -rate_limiter_bytes_per_sec=104857600 -benchmark_write_rate_limit=10485760 -num=100000000 ``` Results: - "Burst W-Amp" is the write-amp near the end of the fillrandom benchmark - "Total W-Amp" is the write-amp after readrandom has run a while and all levels no longer need compaction Branch | Burst W-Amp | Total W-Amp | fillrandom (MB/s) -- | -- | -- | -- master | 20.2 | 21.5 | 4.7 dynamic-l0-score | 12.6 | 14.1 | 7.2 Reviewed By: siying Differential Revision: D23412935 Pulled By: ajkr fbshipit-source-id: f91f2067188e432dd39deab02f1c56f195057a0e 02 September 2020, 02:34:01 UTC
8d44d79 Make examples work on Windows (#7304) Summary: Quick fixes to examples to make it easier to get familiar with RocksDB for Windows users: - Set proper temporary directory path on Windows for all examples (with C++17 we should start using std::filesystem) - Fixed typo and got rid of warnings treated as errors in multi_processes_example.cc - Get number of available cores on Windows in c_simple_example.c - Add command to remove DB directory for Windows in compaction_filter_example.cc (print error, but carry on with example upon error, because error code is returned if there is no such directory on Windows) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7304 Reviewed By: jay-zhuang Differential Revision: D23450900 Pulled By: pdillinger fbshipit-source-id: 4256134deb6ae6bb267ed1bd69f814842b95f60f 02 September 2020, 01:03:50 UTC
55bf42a Recompress blobs during GC if compression changed (#7331) Summary: Recompress blobs if compression type is changed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7331 Test Plan: `make check` Reviewed By: ltamasi Differential Revision: D23437102 Pulled By: jay-zhuang fbshipit-source-id: bb699ebdad137721d422e42e331d4de8a82a7c5f 02 September 2020, 01:03:50 UTC
792d2f9 Log info about generated blob files in BlobFileBuilder (#7324) Summary: The patch adds a log message to `BlobFileBuilder` that is logged upon generating a blob file, similarly to how we log the generation of table files during flush and compaction. The log message contains the column family name, job id, blob file number, and the number and total size of blobs in the new file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7324 Test Plan: Ran `make check` and checked the actual log messages using a custom `db_bench`. Reviewed By: riversand963 Differential Revision: D23402229 Pulled By: ltamasi fbshipit-source-id: ca42beb4db284b783d1eb2651f321032a45d0c5f 31 August 2020, 20:24:12 UTC
963314f Add unit test for max_write_buffer_size_to_maintain (#7311) Summary: Add a unit test case to check memory usage when max_write_buffer_size_to_maintain is set if flushed immutable memtables are trimmed timely or not. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7311 Test Plan: Compared the results with before bug fix. Reviewed By: ltamasi Differential Revision: D23321702 Pulled By: akankshamahajan15 fbshipit-source-id: da04ee21137d641a07fd499a9e2749eb036fcb1e 29 August 2020, 00:38:05 UTC
1e59800 Use standard variables for installing/uninstalling with make (#7187) Summary: Fixes https://github.com/facebook/rocksdb/issues/7185. Standard for GNU and FreeBSD. See https://www.freebsd.org/doc/en/books/porters-handbook/porting-prefix.html https://www.gnu.org/prep/standards/html_node/DESTDIR.html#DESTDIR https://www.gnu.org/prep/standards/html_node/Directory-Variables.html Pull Request resolved: https://github.com/facebook/rocksdb/pull/7187 Reviewed By: cheng-chang Differential Revision: D23333233 Pulled By: ajkr fbshipit-source-id: f704d23852c4516cf5fa00df73ff57687b2ddffb 28 August 2020, 21:47:31 UTC
c2485f2 Add buffer prefetch support for non directIO usecase (#7312) Summary: A new file interface `SupportPrefetch()` is added. When the user overrides it to `false`, an internal prefetch buffer will be used for readahead. Useful for non-directIO but FS doesn't have readahead support. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7312 Reviewed By: anand1976 Differential Revision: D23329847 Pulled By: jay-zhuang fbshipit-source-id: 71cd4ce6f4a820840294e4e6aec111ab76175527 28 August 2020, 01:16:53 UTC
5043960 Add a blob file builder class that can be used in background jobs (#7306) Summary: The patch adds a class called `BlobFileBuilder` that can be used to build and cut blob files in background jobs (flushes/compactions). The class enforces a value size threshold (`min_blob_size`; smaller blobs will be inlined in the LSM tree itself), and supports specifying a blob file size limit (`blob_file_size`), as well as compression (`blob_compression_type`) and checksums for blob files. It also keeps track of the generated blob files and their associated `BlobFileAddition` metadata, which can be applied as part of the background job's `VersionEdit`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7306 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23298817 Pulled By: ltamasi fbshipit-source-id: 38f35d81dab1ba81f15236240612ec173d7f21b5 27 August 2020, 18:55:54 UTC
8e0df90 Store FSRandomAccessPtr object in RandomAccessFileReader (#7192) Summary: Replace FSRandomAccessFile pointer with FSRandomAccessFilePtr object in RandomAccessFileReader. This new object wraps FSRandomAccessFile pointer. Objective: If tracing is enabled, FSRandomAccessFile Ptr returns FSRandomAccessFileTracingWrapper pointer that includes all necessary information in IORecord and calls underlying FileSystem and invokes IOTracer to dump that record in a binary file. If tracing is disabled then, underlying FileSystem pointer is returned directly. FSRandomAccessFilePtr wrapper class is added to bypass the FSRandomAccessFileWrapper when tracing is disabled. Test Plan: make check -j64 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7192 Reviewed By: anand1976 Differential Revision: D23356867 Pulled By: akankshamahajan15 fbshipit-source-id: 48f31168166a17a7444b40be44a9a9d4a5c7182c 27 August 2020, 18:21:52 UTC
9aad24d Real fix for race in backup custom checksum checking (#7309) Summary: This is a "real" fix for the issue worked around in https://github.com/facebook/rocksdb/issues/7294. To get DB checksum info for live files, we now read the manifest file that will become part of the checkpoint/backup. This requires a little extra handling in taking a custom checkpoint, including only reading the manifest file up to the size prescribed by the checkpoint. This moves GetFileChecksumsFromManifest from backup code to file_checksum_helper.{h,cc} and removes apparently unnecessary checking related to column families. Updated HISTORY.md and warned potential future users of DB::GetLiveFilesChecksumInfo() Pull Request resolved: https://github.com/facebook/rocksdb/pull/7309 Test Plan: updated unit test, before and after Reviewed By: ajkr Differential Revision: D23311994 Pulled By: pdillinger fbshipit-source-id: 741e30a2dc1830e8208f7648fcc8c5f000d4e2d5 26 August 2020, 17:39:20 UTC
722814e Get() to fail with underlying failures in PartitionIndexReader::CacheDependencies() (#7297) Summary: Right now all I/O failures under PartitionIndexReader::CacheDependencies() is swallowed. This doesn't impact correctness but we've made a decision that any I/O error in read path now should be returned to users for awareness. Return errors in those cases instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7297 Test Plan: Add a new unit test that ingest errors in this code path and see Get() fails. Only one I/O path is hit in PartitionIndexReader::CacheDependencies(). Several option changes are attempt but not able to got other pread paths triggered. Not sure whether other failure cases would be even possible. Would rely on continuous stress test to validate it. Reviewed By: anand1976 Differential Revision: D23257950 fbshipit-source-id: 859dbc92fa239996e1bb378329344d3d54168c03 26 August 2020, 02:01:05 UTC
cecdd5d Parameterize DBBasicTest.CompactBetweenSnapshots (#7301) Summary: DBBasicTest.CompactBetweenSnapshots can time-out in some slow-I/O hosts. Parameterize it so that single test runs shorter. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7301 Test Plan: Run the test and see see different runs are of different configerations in a hacky way. Reviewed By: ltamasi Differential Revision: D23277733 fbshipit-source-id: 1f717b4131322d175abf9e211131fe7e9b1ef758 25 August 2020, 22:42:11 UTC
d51f88c Pass SST file checksum information through OnTableFileCreated (#7108) Summary: When SST file is created, application is able to know the file information through OnTableFileCreated callback in LogAndNotifyTableFileCreationFinished. Since file checksum information can be useful for application when the SST file is created, we add file_checksum and file_checksum_func_name information to TableFileCreationInfo, which will be passed through OnTableFileCreated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7108 Test Plan: make check, listener_test. Reviewed By: ajkr Differential Revision: D22470240 Pulled By: zhichao-cao fbshipit-source-id: 92c20344d9b986eadfe3480f3769bf4add0dbaae 25 August 2020, 17:46:11 UTC
416943b Eliminates a no-op compaction upon snapshot release when disabling auto compactions (#7267) Summary: After releasing a snapshot, it checks whether it is suitable to trigger bottom compactions. When disabling auto compactions, it may still schedule compaction when releasing a snapshot. Whereas no compaction job will be actually handled, so the state of LSM is not changed and compaction will be triggered again and again every time releasing a snapshot. Too frequent compactions lead to high CPU usage and high db_mutex lock contention which affects foreground write duration finally. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7267 Test Plan: - make check - manual test Reviewed By: akankshamahajan15 Differential Revision: D23252880 Pulled By: ajkr fbshipit-source-id: 4431e071a35d9912a2a3592875db27bae521434b 25 August 2020, 05:06:45 UTC
b7e1c52 Add some simulator cache and block tracer tests to ASSERT_STATUS_CHECKED (#7305) Summary: More tests now pass. When in doubt, I added a TODO comment to check what should happen with an ignored error. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7305 Reviewed By: akankshamahajan15 Differential Revision: D23301262 Pulled By: ajkr fbshipit-source-id: 5f120edc7393560aefc0633250277bbc7e8de9e6 24 August 2020, 23:43:31 UTC
e653af7 DBWithTTL::Open() param ttls: vector<int32_t> to const vector<int32_t>& (#7196) Summary: fix DBWithTTL::Open() param ttls: vector<int32_t> to const vector<int32_t> Pull Request resolved: https://github.com/facebook/rocksdb/pull/7196 Reviewed By: akankshamahajan15 Differential Revision: D23277772 Pulled By: ajkr fbshipit-source-id: bf69834b5c2062c7e166dab21fbfd40416c7872d 24 August 2020, 23:24:16 UTC
5aacef9 Disable fsync in SeqAdvanceConcurrentTest (#7302) Summary: SeqAdvanceConcurrentTest sometimes runs too long on some platforms. Disable fsync to speed it up. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7302 Test Plan: Run the tests and watch CI. Reviewed By: ajkr Differential Revision: D23298192 fbshipit-source-id: 2185eed4e0958c3de5e8a3f94ceed5be5945ed37 24 August 2020, 20:22:06 UTC
21ce018 Disable fsync in some ExternalSSTFileTest tests (#7303) Summary: Some ExternalSSTFileTest runs very long on some places. Disable fsync in some tests to speed them up. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7303 Test Plan: Run these tests. Reviewed By: riversand963 Differential Revision: D23280261 fbshipit-source-id: 0dca862e462f9e6d807f393320a1f82aa5b87e59 24 August 2020, 18:26:09 UTC
ed7ea43 Fix for Regression failure (#7300) Summary: RocksDb regression commands are exiting with error /usr/bin/ar: creating librocksdb.a /usr/bin/ld: ./cache/cache.o: relocation R_X86_64_32 against `.rodata.str1.1' can not be used when making a shared object; recompile with -fPIC Bug: It tries to link the static code into a shared lib. Fix: Added make clean before building shared_lib Pull Request resolved: https://github.com/facebook/rocksdb/pull/7300 Test Plan: make clean make -j$(nproc) static_lib make -j$(nproc) shared_lib Reviewed By: pdillinger Differential Revision: D23276842 Pulled By: akankshamahajan15 fbshipit-source-id: c2e69fa505893ad414786794fc486f3f22f059d5 24 August 2020, 16:11:27 UTC
e500c73 Shutdown timer in destructor (#7292) Summary: Make sure deleting a running timer works fine. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7292 Test Plan: unittest and an invalid benchmark command: `./db_bench --db=/tmp --use_existing_db=false --benchmarks=fred --compression_type=none` Reviewed By: riversand963 Differential Revision: D23248500 Pulled By: jay-zhuang fbshipit-source-id: 04111681b389a9aa23a439db4568d5ca351f1144 21 August 2020, 22:48:52 UTC
3844612 Bug Fix for memtables not trimmed down. (#7296) Summary: When a memtable is trimmed in MemTableListVersion, the memtable is only added to delete list if it is the last reference. However it is not the last reference as it is held by the super version. But the super version would not be switched if the delete list is empty. So the memtable is never destroyed and memory usage increases beyond write_buffer_size + max_write_buffer_size_to_maintain. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7296 Test Plan: 1. ./db_bench -benchmarks=randomtransaction -optimistic_transaction_db=1 -statistics -stats_interval_seconds=1 -duration=90 -num=500000 --max_write_buffer_size_to_maintain=16000000 --transaction_set_snapshot Reviewed By: ltamasi Differential Revision: D23267395 Pulled By: akankshamahajan15 fbshipit-source-id: 3a8d437fe9f4015f851ff84c0e29528aa946b650 21 August 2020, 20:29:05 UTC
187964a Add test function MockTimeEnv.SleepForMicroseconds() (#7293) Summary: And change the internal time value from seconds to microseconds. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7293 Reviewed By: pdillinger Differential Revision: D23253751 Pulled By: jay-zhuang fbshipit-source-id: 36aa9376b8801b85bd10163173590a17cf4f3a3a 21 August 2020, 18:34:37 UTC
a1b5484 Work around a backup bug with DB custom checksums (#7294) Summary: On a read-write DB configured with DBOptions::file_checksum_gen_factory, BackupEngine::CreateNewBackup can fail intermittently, with non-OK status. This is due to a race between GetLiveFiles and GetLiveFilesChecksumInfo in creating backups. For patching 6.12 release (as this commit is intended for, except this is a forward-merged version), we can simply treat files for which we falsely failed to get checksum info as legacy files lacking checksum info. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7294 Test Plan: unit test reproducer included Reviewed By: ajkr Differential Revision: D23253489 Pulled By: pdillinger fbshipit-source-id: 9e4945dad120b776ad3e753be10b962f61f28e14 21 August 2020, 15:16:04 UTC
e9befde Add EnvTestWithParam::OptionsTest to the ASSERT_STATUS_CHECKED passes (#7283) Summary: This test uses database functionality and required more extensive work to get it to pass than the other tests. The DB functionality required for this test now passes the check. When it was unclear what the proper behavior was for unchecked status codes, a TODO was added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7283 Reviewed By: akankshamahajan15 Differential Revision: D23251497 Pulled By: ajkr fbshipit-source-id: 52b79629bdafa0a58de8ead1d1d66f141b331523 21 August 2020, 02:18:35 UTC
b288f01 Add getters for the read options to the C API (#7289) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7289 Reviewed By: akankshamahajan15 Differential Revision: D23252520 Pulled By: ajkr fbshipit-source-id: 85cea485a6dcaa1c67c32a83eb49a1b623966609 20 August 2020, 23:36:19 UTC
ce41923 Track WAL in MANIFEST: minor udpates (#7282) Summary: The updates resolve comments left from https://github.com/facebook/rocksdb/pull/7164. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7282 Test Plan: wal_edit_test Reviewed By: ltamasi Differential Revision: D23196824 Pulled By: cheng-chang fbshipit-source-id: 797f3fef27fc72114c2be777d9eadd3429da5301 20 August 2020, 22:12:00 UTC
327ddb7 Travis: fail fast on install failures (#7239) Summary: A recent build continued with confusing results after failing to "snap install cmake" so ensure failures in installs are fatal. Also upgrade snapd before "snap install" to hopefully avoid this error: error: cannot perform the following tasks: - Mount snap "cmake" (513) (snap "cmake" assumes unsupported features: command-chain (try to update snapd and refresh the core snap)) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7239 Test Plan: watch Travis Reviewed By: akankshamahajan15 Differential Revision: D23244110 Pulled By: pdillinger fbshipit-source-id: 33dbf145f6999d0b90576cdfde484f15c5d1ac19 20 August 2020, 17:39:10 UTC
3e422ce Fix a timer_test deadlock (#7277) Summary: There's a potential deadlock caused by MockTimeEnv time value get to a large number, which causes TimedWait() wait forever. The test misuses the microseconds as seconds, making it more likely to happen. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7277 Reviewed By: pdillinger Differential Revision: D23183873 Pulled By: jay-zhuang fbshipit-source-id: 6fc38ebd40b4125a99551204b271f91a27e70086 20 August 2020, 15:43:13 UTC
2040bb5 Fixed missing space in DeleteScheduler::BackgroundEmptyTrash log message (#7286) Summary: Example of a faulty log message: Rate limiting is enabled with penalty 18203625after deleting file ... Pull Request resolved: https://github.com/facebook/rocksdb/pull/7286 Reviewed By: David132639 Differential Revision: D23215981 Pulled By: freewilll fbshipit-source-id: 8bdbbffea9f2942cc7a652f315a560d61c0f1068 20 August 2020, 09:00:06 UTC
ac7dcfd Add missing ComputeCompactionScore() for a new universal manual compaction (#7281) Summary: Seems it's only causing assert failure during compaction pick, but in production code, the problematic compactions are excluded at a later step. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7281 Reviewed By: akankshamahajan15 Differential Revision: D23228000 Pulled By: jay-zhuang fbshipit-source-id: 2e4055aeebe0f5a2b07e299e0a2d51a1ad2e216d 20 August 2020, 00:42:08 UTC
b9bb59d Add initial set of options for integrated blob write path (#7280) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7280 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23195192 Pulled By: ltamasi fbshipit-source-id: 743b382de391963e62ba86119e9fbd0233ea3b3a 19 August 2020, 01:32:37 UTC
cc24ac1 Store FSSequentialFilePtr object in SequenceFileReader (#7190) Summary: This diff contains following changes: 1. Replace `FSSequentialFile` pointer with `FSSequentialFilePtr` object that wraps `FSSequentialFile` pointer in `SequenceFileReader`. Objective: If tracing is enabled, `FSSequentialFilePtr` returns `FSSequentialFileTracingWrapper` pointer that includes all necessary information in `IORecord` and calls underlying FileSystem and invokes `IOTracer` to dump that record in a binary file. If tracing is disabled then, underlying `FileSystem` pointer is returned directly. `FSSequentialFilePtr` wrapper class is added to bypass the `FSSequentialFileTracingWrapper` when tracing is disabled. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7190 Test Plan: make check -j64 COMPILE_WITH_TSAN=1 make check -j64 Reviewed By: anand1976 Differential Revision: D23059616 Pulled By: akankshamahajan15 fbshipit-source-id: 1564b94dd1297cd0fbfe2ed5c9cc3e20f7395301 18 August 2020, 23:20:54 UTC
e6e2f36 fix doc about kTolerateCorruptedTailRecords recovery (#7270) Summary: - Made it clear only one record in the tail is allowed to have a problem - Added detail about the valid use case instead of calling it legacy behavior Pull Request resolved: https://github.com/facebook/rocksdb/pull/7270 Reviewed By: riversand963 Differential Revision: D23169075 Pulled By: ajkr fbshipit-source-id: 2a4b45aa8641f17efa104523fbad765012a98fb0 18 August 2020, 16:52:25 UTC
7d0ecab Fix some flaky tests in BackupableDBTest with intentional flushing (#7273) Summary: Some tests like BackupableDBTest.FileCollision and ShareTableFilesWithChecksumsNewNaming are intermittently failing, probably due to unpredictable flushing with FillDB. This change should fix the failures seen and help to prevent similar flakiness in future tests in the file. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7273 Test Plan: make check, and with valgrind Reviewed By: siying Differential Revision: D23176947 Pulled By: pdillinger fbshipit-source-id: 654b73a64db475f2b9b065ed53a889a8b9083c59 18 August 2020, 05:07:17 UTC
c073b7f db_bench should be linked with thirdparty libs (#7264) Summary: `db_bench` is not linked with thirdparty libs in cmake, even `-DWITH_*` is specified. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7264 Test Plan: `$ mkdir build; cd build; cmake .. -DWITH_SNAPPY=1; make db_bench; ./db_bench` `$ cmake .. -DWITH_SNAPPY=1 -DWITH_LZ4; make db_bench; ./db_bench -compression_type=lz4` Reviewed By: zhichao-cao Differential Revision: D23165077 Pulled By: jay-zhuang fbshipit-source-id: 9c6fead31c41664a5c75ecd6469f47402fcb7d62 18 August 2020, 01:54:14 UTC
b194c21 Whole DBTest to skip fsync (#7274) Summary: After https://github.com/facebook/rocksdb/pull/7036, we still see extra DBTest that can timeout when running 10 or 20 in parallel. Expand skip-fsync mode in whole DBTest. Still preserve other tests from doing this mode to be conservative. This commit reinstates https://github.com/facebook/rocksdb/issues/7049, whose un-revert was lost in an automatic infrastructure mis-merge. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7274 Test Plan: Run all existing files. Reviewed By: pdillinger Differential Revision: D23177444 fbshipit-source-id: 1f61690b2ac6333c3b2c87176fef6b2cba086b33 18 August 2020, 01:42:25 UTC
5d5ff82 Disable `recycle_log_file_num` with `kTolerateCorruptedTailRecords` (#7271) Summary: The two features are naturally incompatible. WAL recycling expects the recovery to succeed upon encountering a corrupt record at the point where new data ends and recycled data remains at the tail. However, `WALRecoveryMode::kTolerateCorruptedTailRecords` must fail upon encountering any such corrupt record, as it cannot differentiate between this and a real corruption, which would cause committed updates to be truncated. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7271 Reviewed By: riversand963 Differential Revision: D23169923 Pulled By: ajkr fbshipit-source-id: 2cf8a3bcd2c9a0ecb0055a84725047a10fd4db50 18 August 2020, 01:21:10 UTC
92593d5 Add a new EntryType for deletion with timestamp (#7195) Summary: Add `kEntryDeleteWithTimestamp` to `EntryType` which is a public API. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7195 Test Plan: make check Reviewed By: ajkr Differential Revision: D22914704 Pulled By: riversand963 fbshipit-source-id: 886f73c6b70c527cad1c8fc9fc8d3afe60e1ea39 17 August 2020, 23:26:06 UTC
9b083cb Build blob file reader/writer classes in LITE mode as well (#7272) Summary: The patch makes sure that the functionality required for the new integrated BlobDB implementation (most importantly, the classes related to reading and writing blob files) is also built in LITE mode by removing the corresponding `#ifndef`s. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7272 Test Plan: Ran `make check` in both regular and LITE mode. Reviewed By: zhichao-cao Differential Revision: D23173280 Pulled By: ltamasi fbshipit-source-id: 1596bd1a76409a8a6d83d8f1dbfe08bfdea7ffe6 17 August 2020, 22:19:05 UTC
1760637 CompactRange() refit level should confirm destination level is not empty (#7261) Summary: There is potential data race related CompactRange() with level refitting. After the compaction step and refitting step, some automatic compaction could put data to the destination level and cause the DB to be corrupted. Fix the bug by checking the target level to be empty. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7261 Test Plan: Add a unit test, which would fail with "Corruption: L1 have overlapping ranges '666F6F' seq:6, type:1 vs. '626172' seq:2, type:1", and now it succeeds. Reviewed By: ajkr Differential Revision: D23142269 fbshipit-source-id: 28bc14d5ac934c192260b23a4ce3f10a95e3ee91 17 August 2020, 21:21:53 UTC
back to top