https://github.com/facebook/rocksdb

sort by:
Revision Author Date Message Commit Date
35d8e36 Bump version and update HISTORY 15 November 2020, 23:45:38 UTC
71e9a1a Hack to load OPTIONS file for read_amp_bytes_per_bit (#7659) Summary: A temporary hack to work around a bug in 6.10, 6.11, 6.12, 6.13 and 6.14. The bug will write out 8 bytes to OPTIONS file from the starting address of BlockBasedTableOptions.read_amp_bytes_per_bit which is actually a uint32. Consequently, the value of read_amp_bytes_per_bit written in the OPTIONS file is wrong. From 6.15, RocksDB will try to parse the read_amp_bytes_per_bit from OPTIONS file as a uint32. To be able to load OPTIONS file generated by affected releases before the fix, we need to manually parse read_amp_bytes_per_bit with this hack. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7659 Test Plan: Generate a db with current 6.14.fb (head at https://github.com/facebook/rocksdb/commit/b6db05dbb5364c658c5401a8078d73697bb5f31d). Maybe use db_stress. Checkout this PR, run ``` ~/rocksdb/ldb --db=. --try_load_options --ignore_unknown_options idump --count_only ``` Expect success, and should not see ``` Failed: Invalid argument: Error parsing read_amp_bytes_per_bit:17179869184 ``` Also make check Reviewed By: anand1976 Differential Revision: D24954752 Pulled By: riversand963 fbshipit-source-id: c7b802fc3e52acd050a4fc1cd475016122234394 15 November 2020, 23:44:09 UTC
ebaf09d fix read_amp_bytes_per_bit field size (#7651) Summary: The field in BlockBasedTableOptions is 4 bytes: // Default: 0 (disabled) uint32_t read_amp_bytes_per_bit = 0; Pull Request resolved: https://github.com/facebook/rocksdb/pull/7651 Reviewed By: ltamasi Differential Revision: D24844994 Pulled By: riversand963 fbshipit-source-id: e2695e55532256ef8996dd6939cad06987a80293 15 November 2020, 23:42:40 UTC
b6db05d Permit unchecked error For GetCurrentTime() call in GetSnapshot, explicitly ignore checking the error. 05 November 2020, 20:25:46 UTC
8472894 Bump version and update HISTORY 05 November 2020, 20:00:18 UTC
1f73513 Remove DB::VerifyFileChecksums 05 November 2020, 19:55:29 UTC
d567357 Compute NeedCompact() after table builder Finish() (#7627) Summary: In `BuildTable()`, we call `builder->Finish()` before evaluating `builder->NeedCompact()`. However, we call `builder->NeedCompact()` before `builder->Finish()` in compaction job. This can be wrong because the table properties collectors may rely on the success of `Finish()` to provide correct result for `NeedCompact()`. Test plan (on devserver): make check Pull Request resolved: https://github.com/facebook/rocksdb/pull/7627 Reviewed By: ajkr Differential Revision: D24728741 Pulled By: riversand963 fbshipit-source-id: 5a0dce244e14eb1106c4f87021e6bebca82b486e 05 November 2020, 19:47:31 UTC
d4b8274 Add API to verify whole sst file checksum (#7578) Summary: Existing API `VerifyChecksum()` allows application to verify sst files' block checksums. Since whole file, user-specified checksum is tracked in MANIFEST, we can expose a new API to verify sst files' file checksums. ``` // Compute table file checksums if applicable and compare with MANIFEST. // Returns OK if no file has mismatching whole-file checksum. Status DB::VerifyFileChecksums(const ReadOptions& /*read_options*/); ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7578 Test Plan: make check Reviewed By: pdillinger Differential Revision: D24436783 Pulled By: riversand963 fbshipit-source-id: 52b51519b842f2b3c4e3351998a97c86cbec85b3 05 November 2020, 19:45:33 UTC
47d839a Updated GenerateOneFileChecksum to use requested_checksum_func_name (#7586) Summary: CreateFileChecksumGenerator may uses requested_checksum_func_name in generator context to decide which generator will be used. GenerateOneFileChecksum has not being updated to use it, which will always get the generator when the name is empty. Fix it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7586 Test Plan: make check Reviewed By: riversand963 Differential Revision: D24491989 Pulled By: zhichao-cao fbshipit-source-id: d9fdfdd431240f0a9a2e781ddbd48a7d6c609aad 05 November 2020, 19:40:35 UTC
8b298e7 update HISTORY.md and bump version 30 October 2020, 18:20:12 UTC
c87c42f Return NotFound from TableFactory configuration errors during options loading (#7615) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7615 Reviewed By: riversand963 Differential Revision: D24637054 Pulled By: ajkr fbshipit-source-id: 7da20d44289eaa2387af4edf8c3c48057425cc1c 30 October 2020, 17:48:38 UTC
e5686db Revert LoadLatestOptions handling of ignore_unknown_options if versions differ (#7612) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7612 Reviewed By: zhichao-cao Differential Revision: D24627054 Pulled By: riversand963 fbshipit-source-id: 451b4da742e3e84c7442bc7cc4959d39089b89d0 30 October 2020, 17:46:33 UTC
91f3b72 Update History.md and version.h for 6.14.2 Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: 22 October 2020, 03:29:57 UTC
3ddd79e Bug fix to remove function calling in assert statement (#7581) Summary: Remove function calling in assert statement as assert is a no op in opt build and that function might not be called. This causes hang in closing RocksDB when refit level is set. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7581 Test Plan: make check -j64 Reviewed By: riversand963 Differential Revision: D24466420 Pulled By: akankshamahajan15 fbshipit-source-id: 97db4ec5a95ae693c3290e176a3c12a9b1ad2f6d 22 October 2020, 03:25:30 UTC
59ddf78 Revert `Status`es returned from pre-`Configurable` options functions (#7563) Summary: Further refinement of the earlier PR. Now the Status is NotFound with a subcode of PathNotFound. Also the existing functions for options parsing/loading are reverted to return InvalidArgument no matter in which way the user-provided arguments are deemed invalid. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7563 Reviewed By: zhichao-cao Differential Revision: D24422491 Pulled By: ajkr fbshipit-source-id: ba6b237cd0584d3f925c5ba0d349aeb8c250af67 20 October 2020, 19:06:25 UTC
d043387 Test for LoadLatestOptions (#7554) Summary: Make LoadLatestOptions return PathNotFound if the options file does not exist. Added tests for the LoadOptions related methods. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7554 Reviewed By: akankshamahajan15 Differential Revision: D24298985 Pulled By: zhichao-cao fbshipit-source-id: c9ae3cb12fc4a5bbef07743e1c1300f98a2441b3 15 October 2020, 18:28:49 UTC
e402de8 update HISTORY.md and version.h for 6.14.1 13 October 2020, 17:05:00 UTC
668033f Fix bug in pinned partitioned indexes with some reads bypassing block cache Backports part of 75d3b6fdf0aa1007c4d26382f65be0adf4519a37. 13 October 2020, 17:03:08 UTC
2cea469 Fix bug in pinned partitioned user key indexes Backports part of 75d3b6fdf0aa1007c4d26382f65be0adf4519a37. 13 October 2020, 17:02:39 UTC
94c019a Add missing regression test/release note for bug fix Bug fix was in 750817555867a43f0e7b73dffa44756a9136c808. The unit test comes from https://github.com/facebook/rocksdb/pull/7521. 13 October 2020, 17:01:03 UTC
353745d Add missing release note 13 October 2020, 16:56:08 UTC
9dd2548 Update release history 6.14 (#7525) Summary: Update release history for 6.14 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7525 Test Plan: No code change Reviewed By: jay-zhuang Differential Revision: D24224690 Pulled By: akankshamahajan15 fbshipit-source-id: 95441aefde96672fea5a6af5d7e67cdafb1ebdd2 09 October 2020, 23:05:22 UTC
82d4260 Enable paranoid_file_checks in crash test (#7489) Summary: Cover paranoid_file_checks in crash test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7489 Test Plan: Run crash tests for hours and didn't see any failure. Reviewed By: ajkr Differential Revision: D24063868 fbshipit-source-id: 7b48b110e66ce78ae5d0c99a9f32af86edd34c1e 09 October 2020, 15:32:22 UTC
24498ab Add few unit test cases in ASSERT_STATUS_CHECKED (#7500) Summary: Add status enforcement for following tests: 1. import_column_family_test 2. memory_test 3. table_test Pull Request resolved: https://github.com/facebook/rocksdb/pull/7500 Reviewed By: zhichao-cao Differential Revision: D24095887 Pulled By: akankshamahajan15 fbshipit-source-id: db8e1ec595852df143fad78a0c07bfdd27dc3c84 08 October 2020, 18:22:44 UTC
810ab34 Do not rely on the two-argument std::pair constructor being constexpr (#7519) Summary: The `std::pair(const T1& x, const T2& y);` constructor is `constexpr` only starting from C++14; relying on this breaks compilation on certain compilers/platforms we need to support. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7519 Test Plan: `make check` Reviewed By: ajkr Differential Revision: D24195747 Pulled By: ltamasi fbshipit-source-id: 665e8fbc9747675bb49c5d895aad3dcf2714750f 08 October 2020, 17:49:40 UTC
98c1333 Disable a known flaky test: RandomAccessUniqueIDDeletes (#7511) Summary: It's a known issue, which is tracked in https://github.com/facebook/rocksdb/issues/7405, https://github.com/facebook/rocksdb/issues/7470. Disable it for now. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7511 Reviewed By: zhichao-cao Differential Revision: D24145075 Pulled By: jay-zhuang fbshipit-source-id: 1858497972f2baba617867aaeac30d93b8305c80 08 October 2020, 16:40:59 UTC
4146276 Add ldb_cmd_test to ASSERT_STATUS_CHECKED list (#7499) Summary: Add ldb_cmd_test to ASSERT_STATUS_CHECKED list Pull Request resolved: https://github.com/facebook/rocksdb/pull/7499 Test Plan: pass ASSERT_STATUS_CHECKED=1 make -j48 ldb_cmd_test Reviewed By: cheng-chang Differential Revision: D24086203 Pulled By: zhichao-cao fbshipit-source-id: 29592202b1d4335e566de15e7937269d98d57841 08 October 2020, 07:00:48 UTC
002b30c Fix clang analyzer (#7518) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7518 Test Plan: ``` $USE_CLANG=1 make analyze ``` Reviewed By: zhichao-cao Differential Revision: D24175390 Pulled By: riversand963 fbshipit-source-id: c70121652908cf5d450120c38ab65cc595332ca7 08 October 2020, 03:11:06 UTC
1f84611 Clean up BlobLogReader and rename it to BlobLogSequentialReader (#7517) Summary: The patch does some cleanup in and around the legacy `BlobLogReader` class: * It renames the class to `BlobLogSequentialReader` to emphasize that it is for sequentially iterating through blobs in a blob file, as opposed to doing random point reads using `BlobIndex`es (which is `BlobFileReader`'s jurisdiction). * It removes some dead code from the old BlobDB implementation that references `BlobLogReader` (namely the method `BlobFile::OpenRandomAccessReader`). * It cleans up some `#include`s and forward declarations. * It fixes some incorrect/outdated comments related to the reader class. * It adds a few assertions to the `Read` methods of the class. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7517 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D24172611 Pulled By: ltamasi fbshipit-source-id: 43e2ae1eba5c3dd30c1070cb00f217edc45bd64f 08 October 2020, 00:48:16 UTC
22655a3 Introduce a blob file reader class (#7461) Summary: The patch adds a class called `BlobFileReader` that can be used to retrieve blobs using the information available in blob references (e.g. blob file number, offset, and size). This will come in handy when implementing blob support for `Get`, `MultiGet`, and iterators, and also for compaction/garbage collection. When a `BlobFileReader` object is created (using the factory method `Create`), it first checks whether the specified file is potentially valid by comparing the file size against the combined size of the blob file header and footer (files smaller than the threshold are considered malformed). Then, it opens the file, and reads and verifies the header and footer. The verification involves magic number/CRC checks as well as checking for unexpected header/footer fields, e.g. incorrect column family ID or TTL blob files. Blobs can be retrieved using `GetBlob`. `GetBlob` validates the offset and compression type passed by the caller (because of the presence of the header and footer, the specified offset cannot be too close to the start/end of the file; also, the compression type has to match the one in the blob file header), and retrieves and potentially verifies and uncompresses the blob. In particular, when `ReadOptions::verify_checksums` is set, `BlobFileReader` reads the blob record header as well (as opposed to just the blob itself) and verifies the key/value size, the key itself, as well as the CRC of the blob record header and the key/value pair. In addition, the patch exposes the compression type from `BlobIndex` (both using an accessor and via `DebugString`), and adds a blob file read latency histogram to `InternalStats` that can be used with `BlobFileReader`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7461 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D23999219 Pulled By: ltamasi fbshipit-source-id: deb6b1160d251258b308d5156e2ec063c3e12e5e 07 October 2020, 22:44:53 UTC
38d0a36 Add Stats for MultiGet (#7366) Summary: Add following stats for MultiGet in Histogram to get more insight on MultiGet. 1. Number of index and filter blocks read from file as part of MultiGet request per level. 2. Number of data blocks read from file per level. 3. Number of SST files loaded from file system per level. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7366 Reviewed By: anand1976 Differential Revision: D24127040 Pulled By: akankshamahajan15 fbshipit-source-id: e63a003056b833729b277edc0639c08fb432756b 07 October 2020, 20:28:48 UTC
8891e9a Disallow trivial move if BottommostLevelCompaction is kForce* (#7368) Summary: If `BottommostLevelCompaction.kForce*` is set, compaction should avoid trivial move and always compact the sst to the target size. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7368 Reviewed By: ajkr Differential Revision: D23629525 Pulled By: jay-zhuang fbshipit-source-id: 79f23c79ecb31587e0593b28cce43131107bbcd0 07 October 2020, 20:19:31 UTC
60649aa Fix wrong comments about function TruncateToPageBoundary. (#6975) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6975 Reviewed By: ajkr Differential Revision: D22914797 Pulled By: riversand963 fbshipit-source-id: 5be2cd322d41447f638dba1336e96dcdc090f9dd 07 October 2020, 19:34:34 UTC
a242a58 Enable ASSERT_STATUS_CHECKED for db_universal_compaction_test (#7460) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7460 Reviewed By: riversand963 Differential Revision: D24057636 Pulled By: anand1976 fbshipit-source-id: bfb13da6993a5e407be20073e4d6751dfb38e442 06 October 2020, 21:42:12 UTC
1bcef3d Make sure assert(false) handles failures too (#7483) Summary: In opt mode, assertions are just no-ops. Therefore, we need to report errors instead of just doing an `assert(false)`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7483 Test Plan: make check Reviewed By: anand1976 Differential Revision: D24142725 Pulled By: riversand963 fbshipit-source-id: 5629556dbe29f00dd09e30a7d5df5e6cf09ee435 06 October 2020, 20:52:42 UTC
5308903 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 06 October 2020, 19:44:20 UTC
2496d3d Avoid to suppress status code in ~BlockBasedTableBuilder (#7507) Summary: We just used a hacky way to fix db_basic_test: suppress status code in ~BlockBasedTableBuilder. Rather, we should pass them back in Finish() and suppress them in Abandon(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7507 Test Plan: Watch existing tests to succeed. Reviewed By: riversand963 Differential Revision: D24119527 fbshipit-source-id: 71c4d4a81c0fd1c5595224692275f20f7759973a 05 October 2020, 21:58:49 UTC
5d16325 Revert "Return error if Get() fails in Prefetching Filter blocks (#7463)" (#7505) Summary: This reverts commit 7d503e66a9a9d9bdf17e2d35038bfe8f3e12f5dc. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7505 Reviewed By: ajkr Differential Revision: D24100875 Pulled By: ltamasi fbshipit-source-id: 8705e3e6e8be4b4fd175ffdb031baa6530b61151 04 October 2020, 01:38:04 UTC
758ead5 Enforce status check for corruption_test (#7453) Summary: Enforce status check for corruption_test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7453 Test Plan: ``` ASSERT_STATUS_CHECKED=1 make corruption_test ./corruption_test ``` Reviewed By: jay-zhuang Differential Revision: D24006862 Pulled By: riversand963 fbshipit-source-id: 664677caf4c3007a25cf565cec3d677f2dcea130 03 October 2020, 05:11:00 UTC
7d503e6 Return error if Get() fails in Prefetching Filter blocks (#7463) Summary: Right now all I/O failures under PartitionFilterBlock::CacheDependencies() is swallowed. Return error in case prefetch fails. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7463 Test Plan: make check -j64 Reviewed By: anand1976 Differential Revision: D24008226 Pulled By: akankshamahajan15 fbshipit-source-id: b65d63b2d01465db92500b78de7ad58650ec9b3b 03 October 2020, 02:40:43 UTC
668ee08 Fix prefix_test for status check (#7495) Summary: Fix prefix_test so that it passes when ASSERT_STATUS_CHECKED=1 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7495 Test Plan: Run the test with the option Reviewed By: anand1976 Differential Revision: D24069715 fbshipit-source-id: 54f74b58575a1b49dbdee9ea2d24751fa956b620 03 October 2020, 00:01:15 UTC
b7062f0 Status check enforcement for error_handler_fs_test (#7342) Summary: Added status check enforcement for error_test_fs_test Pull Request resolved: https://github.com/facebook/rocksdb/pull/7342 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 error_test_fs_test Reviewed By: akankshamahajan15 Differential Revision: D23972231 Pulled By: zhichao-cao fbshipit-source-id: fa41bfe440012e0c55f2c9507c1d0104e5e93f84 02 October 2020, 23:41:13 UTC
7cd760d Add status check enforcement for column_family_test.cc (#7484) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7484 Reviewed By: jay-zhuang Differential Revision: D24037616 Pulled By: akankshamahajan15 fbshipit-source-id: 0f63281f81046bcb1b95a7578783285cc6346ece 02 October 2020, 20:35:15 UTC
48d5aa9 Enable status check for db_secondary_test (#7487) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7487 Test Plan: ASSERT_STATUS_CHECKED=1 make db_secondary_test ./db_secondary_test Reviewed By: zhichao-cao Differential Revision: D24071038 Pulled By: riversand963 fbshipit-source-id: e6600c0aecab71c1326b22af263e92bddee5f7ac 02 October 2020, 18:23:36 UTC
29ed766 add Status check enforcement for stats_history_test (#7496) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7496 Reviewed By: zhichao-cao Differential Revision: D24070007 Pulled By: ajkr fbshipit-source-id: 4320413a4d7707774ee23a7e6232714d7ee7a57f 02 October 2020, 15:25:30 UTC
1e00909 Periodically flush info log out of application buffer (#7488) Summary: This PR schedules a background thread (shared across all DB instances) to flush info log every ten seconds. This improves debuggability in case of RocksDB hanging since it ensures the log messages leading up to the hang will eventually become visible in the log. The bulk of this PR is moving monitoring/stats_dump_scheduler* to db/periodic_work_scheduler* and making the corresponding name changes since now the scheduler handles info log flushing, not just stats dumping. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7488 Reviewed By: riversand963 Differential Revision: D24065165 Pulled By: ajkr fbshipit-source-id: 339c47a0ff43b79fdbd055fbd9fefbb6f9d8d3b5 02 October 2020, 02:14:14 UTC
94fc676 Fix db_properties_test for ASSERT_STATUS_CHECKED (#7490) Summary: Add all status handling in db_properties_test so that it can pass ASSERT_STATUS_CHECKED. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7490 Test Plan: Run the test with ASSERT_STATUS_CHECKED Reviewed By: jay-zhuang Differential Revision: D24065382 fbshipit-source-id: e008916155196891478c964df0226545308ca71d 02 October 2020, 00:47:09 UTC
685cabd Add trace_analyzer_test to ASSERT_STATUS_CHECKED list (#7480) Summary: Add trace_analyzer_test to ASSERT_STATUS_CHECKED list Pull Request resolved: https://github.com/facebook/rocksdb/pull/7480 Test Plan: ASSERT_STATUS_CHECKED=1 make -j48 trace_analyzer_test Reviewed By: riversand963 Differential Revision: D24033768 Pulled By: zhichao-cao fbshipit-source-id: b415045e6fab01d6193448650772368c21c6dba6 01 October 2020, 22:58:52 UTC
f5e22ce fix dummy collector's name (#7442) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7442 Reviewed By: jay-zhuang Differential Revision: D24011201 Pulled By: ajkr fbshipit-source-id: 54f24c29875f7575612a780f15a42cda918d6641 01 October 2020, 22:22:47 UTC
9082771 Add is_full_compaction to CompactionJobStats, cleanup (#7451) Summary: This exposes to the listener interface whether a compaction was full or not. Also cleaned up API comment for CompactionJobInfo::stats, which is not of a nullable type. And since CompactionJob is always created with non-null CompactionJobStats, removed conditionals on it being nullptr and instead assert non-null. TODO later: update C and Java interfaces Pull Request resolved: https://github.com/facebook/rocksdb/pull/7451 Test Plan: updated existing unit tests to check new field, make check Reviewed By: ltamasi Differential Revision: D23977796 Pulled By: pdillinger fbshipit-source-id: 1ae7e26cb949631c2b2fb9e696710daf53cc378d 01 October 2020, 19:52:58 UTC
786c1a2 Reduce the number of iterations in DBTest.FileCreationRandomFailure (#7481) Summary: `DBTest.FileCreationRandomFailure` frequently times out during our continuous test runs. (It's a case of "stress test posing as unit test.") The patch reduces the number of iterations to avoid this. Note that the lower numbers are still sufficient to trigger both flushes and compactions, so test coverage is still the same. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7481 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D24034712 Pulled By: ltamasi fbshipit-source-id: 8731a9446e5a121a1041b00f0df473b9f714935a 01 October 2020, 17:42:58 UTC
7508175 Introduce options.check_flush_compaction_key_order (#7467) Summary: Introduce an new option options.check_flush_compaction_key_order, by default set to true, which checks key order of flush and compaction, and fail the operation if the order is violated. Also did minor refactor hash checking code, which consolidates the hashing logic to a vlidation class, where the key ordering logic is added. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7467 Test Plan: Add unit tests to validate the check can catch reordering in flush and compaction, and can be properly disabled. Reviewed By: riversand963 Differential Revision: D24010683 fbshipit-source-id: 8dd6292d2cda8006054e9ded7cfa4bf405f0527c 01 October 2020, 17:10:26 UTC
3e74505 Fix MSVC-related build issues (#7439) Summary: This PR addresses some build and functional issues on MSVC targets, as a step towards an eventual goal of having RocksDB build successfully for Windows on ARM64. Addressed issues include: - BitsSetToOne and CountTrailingZeroBits do not compile on non-x64 MSVC targets. A fallback implementation of BitsSetToOne when Intel intrinsics are not available is added, based on the C++20 `<bit>` popcount implementation in Microsoft's STL. - The implementation of FloorLog2 for MSVC targets (including x64) gives incorrect results. The unit test easily detects this, but CircleCI is currently configured to only run a specific set of tests for Windows CMake builds, so this seems to have been unnoticed. - AsmVolatilePause does not use YieldProcessor on Windows ARM64 targets, even though it is available. - When CondVar::TimedWait calls Microsoft STL's condition_variable::wait_for, it can potentially trigger a bug (just recently fixed in the upcoming VS 16.8's STL) that deadlocks various tests that wait for a timer to execute, since `Timer::Run` doesn't get a chance to execute before being blocked by the test function acquiring the mutex. - In c_test, `GetTempDir` assumes a POSIX-style temp path. - `NormalizePath` did not eliminate consecutive POSIX-style path separators on Windows, resulting in test failures in e.g., wal_manager_test. - Various other test failures. In a followup PR I hope to modify CircleCI's config.yml to invoke all RocksDB unit tests in Windows CMake builds with CTest, instead of the current use of `run_ci_db_test.ps1` which requires individual tests to be specified and is missing many of the existing tests. Notes from peterd: FloorLog2 is not yet used in production code (it's for something in progress). I also added a few more inexpensive platform-dependent tests to Windows CircleCI runs. And included facebook/folly#1461 as requested Pull Request resolved: https://github.com/facebook/rocksdb/pull/7439 Reviewed By: jay-zhuang Differential Revision: D24021563 Pulled By: pdillinger fbshipit-source-id: 0ec2027c0d6a494d8a0fe38d9667fc2f7e29f7e7 01 October 2020, 16:23:04 UTC
2f2e6e1 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 01 October 2020, 05:50:32 UTC
2af46f1 Move break into block (#7468) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7468 Reviewed By: riversand963 Differential Revision: D24028736 Pulled By: ajkr fbshipit-source-id: bd2b4b8d069491a16373d3d2705fddf7ebfe6723 01 October 2020, 03:24:23 UTC
e04a509 Change ParseInternalKey() to return Status instead of bool (#7457) Summary: Fixes https://github.com/facebook/rocksdb/issues/7430 Change ParseInternalKey() to return Status instead of bool. db_bench (seekrandom) based before/after results with value size of 100 bytes and 16 bytes can be found at (tests ran on an udb server): https://www.dropbox.com/s/47bwamdy5ozngph/PIK_ret_Status_results.xlsx?dl=0 ![db_bench_results](https://user-images.githubusercontent.com/62277872/94642825-2a21a800-029a-11eb-88f2-124136c83fd3.png) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7457 Reviewed By: ajkr Differential Revision: D24002433 Pulled By: ramvadiv fbshipit-source-id: ac253ecf577a29044c47c3fe254a01e71404c44c 01 October 2020, 02:16:47 UTC
e127fe1 Fix TSAN failure for backupable_db_test (#7478) Summary: It's a transient failure, but can be reproduce with running the test 100 times: https://app.circleci.com/pipelines/github/facebook/rocksdb/3760/workflows/de909685-f22b-45ba-a8f3-6ebb78a54e96/jobs/37039 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7478 Test Plan: re-run the test 100 times Reviewed By: ajkr Differential Revision: D24035758 Pulled By: jay-zhuang fbshipit-source-id: 6b31983d5c3f7faa8d5481306098513485d0d69d 01 October 2020, 00:22:56 UTC
718e192 Fix flaky intra-L0 consistency failure regression tests (#7477) Summary: Do not assert the number of files after intra-L0 compaction is eligible to run since it could complete (and reduce the number of files) before the assertion executes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7477 Reviewed By: pdillinger Differential Revision: D24032049 Pulled By: ajkr fbshipit-source-id: e838ac7a24651ebd643b9e5a9d39d2e789c46929 30 September 2020, 23:50:24 UTC
58905f3 Remove a gtest-parallel workaround (#7433) Summary: As the issue is fixed in official repo: https://github.com/google/gtest-parallel/pull/79 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7433 Reviewed By: ajkr Differential Revision: D24023271 Pulled By: jay-zhuang fbshipit-source-id: 0babf5e4c59cd61ded5a64cf9aa2d457deeeaa47 30 September 2020, 23:08:09 UTC
aedcaae Stress test to support paranoid_file_checks (#7473) Summary: It's important to make sure no false positive is reported when options.paranoid_file_checks is used. Add it to stress test and a place holder in crash test. It is disabled in crash test as there appears to be a bug causing false positive. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7473 Test Plan: Run crash test Reviewed By: ajkr Differential Revision: D24026939 fbshipit-source-id: 89102acb45cf041776775ce44a4eef4b0f3a380c 30 September 2020, 21:41:33 UTC
5997f6b Add Status check enforcement for unit tests (#7464) Summary: Add status check for unit tests : block_based_filter_block_test, block_fetcher_test, full_filter_block_test and partitioned_filter_block_test Pull Request resolved: https://github.com/facebook/rocksdb/pull/7464 Reviewed By: zhichao-cao Differential Revision: D24011309 Pulled By: akankshamahajan15 fbshipit-source-id: d814803f94e8bb8b811ef170d20b22d52c1a3ff2 30 September 2020, 19:48:05 UTC
ddbc5da Enable force_consistency_checks by default (#7446) Summary: This has been running in production on some key workloads, so we believe it to be safe and extremely low cost. Nevertheless, I've added code to ensure that "force_consistency_checks" is mentioned in any corruption reports so that people know how to disable in case of false positive corruption reports. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7446 Test Plan: make check, CI, temporary debug print new message with ./version_builder_test Reviewed By: ajkr Differential Revision: D23972101 Pulled By: pdillinger fbshipit-source-id: 9623e400f3752577c0ecf977e6d0915562cf9968 30 September 2020, 18:57:32 UTC
5f33436 Revert an uncessary status code check skipping (#7458) Summary: https://github.com/facebook/rocksdb/pull/7452 added an uncessary skip for status code checking. Revert it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7458 Test Plan: Watch CI to finish Reviewed By: jay-zhuang Differential Revision: D23994390 fbshipit-source-id: a2b50a6326d8073db3386bff3d32acc5a6666e9b 30 September 2020, 18:38:39 UTC
9d212d3 Provide users with option to opt-in to get corrupt data in logs/messages (#7420) Summary: Add a new Option "allow_data_in_errors". When it's set by users, it allows them to opt-in to get error messages containing corrupted keys/values. Corrupt keys, values will be logged in the messages, logs, status etc. that will help users with the useful information regarding affected data. By default value is set false to prevent users data to be exposed in the messages. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7420 Test Plan: 1. make check -j64 2. Add a new test case Reviewed By: ajkr Differential Revision: D23835028 Pulled By: akankshamahajan15 fbshipit-source-id: 8d2eba8fb898e79fcf1fccc07295065a75eb59b1 30 September 2020, 06:17:45 UTC
1bdaef7 Status check enforcement for timestamp_basic_test (#7454) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7454 Reviewed By: riversand963 Differential Revision: D23981719 Pulled By: jay-zhuang fbshipit-source-id: 01073f73e54c17067b886c4a2f179b2804198399 30 September 2020, 01:23:27 UTC
8115eb5 add Status check assertions for repair_test (#7455) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7455 Reviewed By: riversand963 Differential Revision: D23985283 Pulled By: ajkr fbshipit-source-id: 5dd2be62350f6e31d13a1e7821cb848a37699c93 29 September 2020, 23:30:08 UTC
bd5d9e2 Fix misspelling of PartitionedIndexIterator (#7450) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7450 Reviewed By: zhichao-cao Differential Revision: D23992811 Pulled By: anand1976 fbshipit-source-id: 71bd898aafce6a3add3c8cd86d9f0e0fb63860cf 29 September 2020, 23:28:13 UTC
1600aac Flush info log for warning and higher severity (#7462) Summary: After unclean crash, the tail of the log could look as follows due to block buffering, even when the call to `ROCKSDB_LOG_ERROR()` finished. ``` 2020/09/29-13:54:39.596710 7f67025fe700 [ERROR] [/db_impl/db_impl_compaction_flush.cc:2500] Waiting after background compaction err ``` This PR forces the flush while logging warning severity or higher to prevent that case. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7462 Reviewed By: riversand963 Differential Revision: D24000154 Pulled By: ajkr fbshipit-source-id: 3bf5f1e69a62ee10e84095cebc88937a8f81b4ad 29 September 2020, 23:06:14 UTC
07dc955 Report error of GetChildren (#7459) Summary: As title Pull Request resolved: https://github.com/facebook/rocksdb/pull/7459 Test Plan: make check Reviewed By: anand1976 Differential Revision: D23999393 Pulled By: riversand963 fbshipit-source-id: 09df8e1637f4df3616c63ee314de397b35be4e4a 29 September 2020, 22:27:00 UTC
12ede5e Remove invalid assertion in compaction_picker_universal.cc (#7421) Summary: The assertion checks that there is no overlap in sequence numbers across levels in universal compaction. However, this assumption doesn't hold when there is a delete triggered compaction or a trivial move, as they operate on a subset of a level. Tests - make check Pull Request resolved: https://github.com/facebook/rocksdb/pull/7421 Reviewed By: ajkr Differential Revision: D23872672 Pulled By: anand1976 fbshipit-source-id: c386deab8e01a5746ca996ff1f4ebcae3b15b7d2 29 September 2020, 20:29:58 UTC
d08a900 Make db_basic_test pass assert status checked (#7452) Summary: Add db_basic_test status check list. Some of the warnings are suppressed. It is possible that some of them are due to real bugs. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7452 Test Plan: See CI tests pass. Reviewed By: zhichao-cao Differential Revision: D23979764 fbshipit-source-id: 6151570c2a9b931b0fbb3fe939a94b2bd1583cbe 29 September 2020, 16:49:04 UTC
5e221a9 Support injecting read errors for RandomAccessFile when using FaultInjectionTestEnv (#7447) Summary: The patch adds support for injecting errors when reading from `RandomAccessFile` using `FaultInjectionTestEnv`. (This functionality was curiously missing w/r/t `RandomAccessFile`, even though it was implemented for `RandomRWFile`.) The patch also fixes up a test case in `blob_db_test` which uses `FaultInjectionTestEnv` but has so far relied on reads from `RandomAccessFile`s succeeding even after deactivating the filesystem. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7447 Test Plan: `make check` Reviewed By: zhichao-cao Differential Revision: D23971740 Pulled By: ltamasi fbshipit-source-id: 8492736cb64b1ee138c658822535f3ff4fe560c6 29 September 2020, 00:32:06 UTC
8c7bac6 Check status for file_reader_writer_test (#7449) Summary: Check the status for file_reader_writer_test. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7449 Test Plan: ``` ASSERT_STATUS_CHECKED=1 make -j20 file_reader_writer_test ./file_reader_writer_test ``` Reviewed By: zhichao-cao Differential Revision: D23975609 Pulled By: riversand963 fbshipit-source-id: a468eb04b386967fcc0478a56e4f0a19bdf81cdf 28 September 2020, 23:05:11 UTC
7a23a21 enable Status check assertions for sst_file_reader_test (#7448) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7448 Reviewed By: jay-zhuang Differential Revision: D23973281 Pulled By: ajkr fbshipit-source-id: 90fe55f1db904f1a236f3f7994d0806b8d24282c 28 September 2020, 22:13:15 UTC
d71cfe0 Add flush_job_test to the list of ASSERT_STATUS_CHECKED tests (#7445) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7445 Test Plan: pass ASSERT_STATUS_CHECKED=1 make -j48 flush_job_test Reviewed By: akankshamahajan15 Differential Revision: D23969372 Pulled By: zhichao-cao fbshipit-source-id: 498ff45ef84e07ec27a8f35d0874d3371412afe9 28 September 2020, 21:59:02 UTC
1abbc56 Add version_builder_test to the list of ASSERT_STATUS_CHECKED tests (#7444) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7444 Test Plan: `ASSERT_STATUS_CHECKED=1 make version_builder_test -j24` Reviewed By: jay-zhuang Differential Revision: D23965793 Pulled By: ltamasi fbshipit-source-id: 8beaf66548379f21146189cda699d5f6fbb35a1b 28 September 2020, 19:12:40 UTC
c203e01 reset refitting_level_ flag to false in error paths (#7403) Summary: Reset refitting_level_ flag to false in error paths in DBImpl::ReFitLevel() Pull Request resolved: https://github.com/facebook/rocksdb/pull/7403 Reviewed By: ajkr Differential Revision: D23909028 Pulled By: ramvadiv fbshipit-source-id: 521ad9aadc1b734bef9ef9119d1e1ee1fa8126e9 28 September 2020, 18:37:00 UTC
08552b1 Genericize and clean up FastRange (#7436) Summary: A generic algorithm in progress depends on a templatized version of fastrange, so this change generalizes it and renames it to fit our style guidelines, FastRange32, FastRange64, and now FastRangeGeneric. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7436 Test Plan: added a few more test cases Reviewed By: jay-zhuang Differential Revision: D23958153 Pulled By: pdillinger fbshipit-source-id: 8c3b76101653417804997e5f076623a25586f3e8 28 September 2020, 18:35:00 UTC
8f82640 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:44:43 UTC
fa92b9d Fix TSAN build and re-enable the tests (#7386) Summary: Resolve TSAN build warnings and re-enable disabled TSAN tests. Not sure if it's a compiler issue or TSAN check issue. Switching from conditional operator to if-else mitigated the problem. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7386 Test Plan: run TSAN check 10 times in circleci. ``` WARNING: ThreadSanitizer: data race (pid=27735) Atomic write of size 8 at 0x7b54000005e8 by thread T32: #0 __tsan_atomic64_store <null> (db_test+0x4cee95) https://github.com/facebook/rocksdb/issues/1 std::__atomic_base<unsigned long>::store(unsigned long, std::memory_order) /usr/bin/../lib/gcc/x86_64-linux-gnu/5.4.0/../../../../include/c++/5.4.0/bits/atomic_base.h:374:2 (db_test+0x78460e) https://github.com/facebook/rocksdb/issues/2 rocksdb::VersionSet::SetLastSequence(unsigned long) /home/circleci/project/./db/version_set.h:1058:20 (db_test+0x78460e) ... Previous read of size 8 at 0x7b54000005e8 by thread T31: #0 bool rocksdb::DBImpl::MultiCFSnapshot<std::unordered_map<unsigned int, rocksdb::DBImpl::MultiGetColumnFamilyData, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, rocksdb::DBImpl::MultiGetColumnFamilyData> > > >(rocksdb::ReadOptions const&, rocksdb::ReadCallback*, std::function<rocksdb::DBImpl::MultiGetColumnFamilyData* (std::unordered_map<unsigned int, rocksdb::DBImpl::MultiGetColumnFamilyData, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, rocksdb::DBImpl::MultiGetColumnFamilyData> > >::iterator&)>&, std::unordered_map<unsigned int, rocksdb::DBImpl::MultiGetColumnFamilyData, std::hash<unsigned int>, std::equal_to<unsigned int>, std::allocator<std::pair<unsigned int const, rocksdb::DBImpl::MultiGetColumnFamilyData> > >*, unsigned long*) /home/circleci/project/db/db_impl/db_impl.cc (db_test+0x715087) ``` Reviewed By: ltamasi Differential Revision: D23725226 Pulled By: jay-zhuang fbshipit-source-id: a6d662a5ea68111246cd32ec95f3411a25f76bc6 25 September 2020, 21:46:28 UTC
c8a12aa EnableFileDeletions only read field while holding mutex (#7435) Summary: Possible fix for a TSAN issue reported in EnableFileDeletions. disable_delete_obsolete_files_ should only be accessed holding the db mutex, but for logging it was being accessed outside holding the mutex, now fixed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7435 Test Plan: existing tests, watch for recurrence Reviewed By: ltamasi Differential Revision: D23917578 Pulled By: pdillinger fbshipit-source-id: 8573025bca3f6fe169b24b87bbfc4ce9667b0482 25 September 2020, 20:34:36 UTC
1a24f4d Track WAL in MANIFEST: add method to check WAL consistency (#7236) Summary: Add a method `CheckWals` in `WalSet` to check the logs on disk. See `CheckWals`'s comments. This method will be used to check consistency of WALs during DB recovery. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7236 Test Plan: a set of tests are added to wal_edit_test.cc. Reviewed By: riversand963 Differential Revision: D23036505 Pulled By: cheng-chang fbshipit-source-id: 5b1d6857ac173429b00f950c32c4a5b8d063a732 25 September 2020, 20:25:54 UTC
8c9fff9 MultiGet() with timestamp should respect snapshot (#7404) Summary: Similar to PR https://github.com/facebook/rocksdb/issues/7227, add read callback to filter out rows with with higher sequence number. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7404 Reviewed By: riversand963 Differential Revision: D23790762 Pulled By: jay-zhuang fbshipit-source-id: bce854307612f1a22f985ffc934da627d0a139c2 25 September 2020, 16:42:01 UTC
c5b3128 Add ASSERT_STATUS_CHECKED flag support (#7332) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7332 Test Plan: `cmake .. -DASSERT_STATUS_CHECKED=1`, then run tests Reviewed By: zhichao-cao Differential Revision: D23437128 Pulled By: jay-zhuang fbshipit-source-id: 66fce57ae5d126e7a8b91c16a73c81438cf3b65e 25 September 2020, 16:08:03 UTC
30fb9dd Introduce a helper method UncompressData (#7434) Summary: The patch introduces a helper method in `util/compression.h` called `UncompressData` that dispatches calls to the correct uncompression method based on type, and changes `UncompressBlockContentsForCompressionType` and `Benchmark::Uncompress` in `db_bench` so they are implemented in terms of the new method. This eliminates some code duplication. (`Benchmark::Compress` is also updated to use the previously introduced `CompressData` helper.) In addition, the patch brings the implementation of `Snappy_Uncompress` into sync with the other uncompression methods by making the method compute the buffer size and allocate the buffer itself. Finally, the patch eliminates some potentially risky back-and-forth conversions between various unsigned and signed integer types by exposing the size of the allocated buffer as a `size_t` instead of an `int`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7434 Test Plan: `make check` `./db_bench -benchmarks=compress,uncompress --compression_type ...` Reviewed By: riversand963 Differential Revision: D23900011 Pulled By: ltamasi fbshipit-source-id: b25df63ceec4639889be94acb22eb53e530c54e0 25 September 2020, 16:01:45 UTC
9a63bbd Add few unit test cases in ASSERT_STATUS_CHECKED build (#7427) Summary: Fix few test cases and add them in ASSERT_STATUS_CHECKED build. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7427 Test Plan: 1. ASSERT_STATUS_CHECKED=1 make -j48 check, 2. travis build for ASSERT_STATUS_CHECKED, 3. Without ASSERT_STATUS_CHECKED: make check -j64, CircleCI build and travis build Reviewed By: pdillinger Differential Revision: D23909983 Pulled By: akankshamahajan15 fbshipit-source-id: 42d7e4aea972acb9fcddb7ca73fcb82f93272434 25 September 2020, 04:48:57 UTC
0ce9b3a 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, 02:02:26 UTC
98ac6b6 Add IO Tracer Parser (#7333) Summary: Implement a parsing tool io_tracer_parser that takes IO trace file (binary file) with command line argument --io_trace_file and output file with --output_file and dumps the IO trace records in outputfile in human readable form. Also added unit test cases that generates IO trace records and calls io_tracer_parse to parse those records. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7333 Test Plan: make check -j64, Add unit test cases. Reviewed By: anand1976 Differential Revision: D23772360 Pulled By: akankshamahajan15 fbshipit-source-id: 9c20519c189362e6663352d08863326f3e496271 23 September 2020, 22:50:26 UTC
31d1cea 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:26:17 UTC
0698598 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:24:03 UTC
6980fff CircleCI build improvements (#7392) Summary: * add slack integration * parallelize `check_some` tests with gtest-parallel * upgrade build image to the latest one: ubuntu-1604:202007-01 * clean up the config Pull Request resolved: https://github.com/facebook/rocksdb/pull/7392 Reviewed By: pdillinger Differential Revision: D23845379 Pulled By: jay-zhuang fbshipit-source-id: f3af82dc2daeba6441f61c858e08d3936c6ccd10 23 September 2020, 21:42:16 UTC
249f2b5 build: make it compile with @mode/win (#7406) Summary: While rocksdb can compile on both macOS and Linux with Buck, it couldn't be compiled on Windows. The only way to compile it on Windows was with the CMake build. To keep the multi-platform complexity low, I've simply included all the Windows bits in the TARGETS file, and added large #if blocks when not on Windows, the same was done on the posix specific files. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7406 Test Plan: On my devserver: buck test //rocksdb/... On Windows: buck build mode/win //rocksdb/src:rocksdb_lib Reviewed By: pdillinger Differential Revision: D23874358 Pulled By: xavierd fbshipit-source-id: 8768b5d16d7e8f44b5ca1e2483881ca4b24bffbe 23 September 2020, 19:55:54 UTC
ac1734d 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:34:48 UTC
b005f96 db_iter.cc: DBIter::Next(): minor improve (#7407) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7407 Reviewed By: ajkr Differential Revision: D23817122 Pulled By: jay-zhuang fbshipit-source-id: 62bf43e4d780fad8c682edd750b4800b5b8f4a77 23 September 2020, 16:53:24 UTC
5d6ff69 Fix valgrind issues with configurable_test (#7424) Summary: Valgrind was reporting a problem with the configurable_test in some GTEST code. This problem was caused by using a std::function as a GTEST parameter. This change changes the test to use a string as a function parameter (backed by a map) and fixes the valgrind issue. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7424 Reviewed By: ajkr Differential Revision: D23855540 Pulled By: pdillinger fbshipit-source-id: 2f2be03f7f92d96644aa9fa6481e4f37f2cfa5f5 23 September 2020, 16:34:01 UTC
00ee89b Track WAL in MANIFEST: update WalMetadata for WAL syncing (#7414) Summary: There are some tricky behaviors related to WAL sync: - When creating a WAL, the WAL might not be synced, if the WAL directory is not synced, the WAL file's metadata may not even be synced to disk, so during recovery, when listing the WAL directory, the WAL may not even show up. - During each DB::Write, the WriteOption can control whether the WAL should be synced, so a WAL previously not synced on creation can be synced during Write. For each `SyncWAL`, we'll track the synced status and the current WAL size. Previously, we only track the WAL size on closing. During recovery, we check that the on-disk WAL size is >= the last synced size. So this PR introduces `synced_size` and `closed` to `WalMetadata` for the above design update. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7414 Test Plan: - updated wal_edit_test - updated version_edit_test Reviewed By: riversand963 Differential Revision: D23796127 Pulled By: cheng-chang fbshipit-source-id: 5498ab80f537c48a10157e71a4745716aef5cf30 22 September 2020, 21:35:14 UTC
cd72f89 Allow mutex to be released in GetAggregatedIntProperty (#7412) Summary: Current implementation holds db mutex while calling `GetAggregatedIntProperty()`. For property kEstimateTableReadersMem, this can be expensive, especially if the number of table readers is high. We can release and re-acquire db mutex if property_info.need_out_of_mutex is true. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7412 Test Plan: make check COMPILE_WITH_ASAN=1 make check COMPILE_WITH_TSAN=1 make check Also test internally on a shadow host. Used bpf to verify the excessively long db mutex holding no longer exists when applications call GetApproximateMemoryUsageByType(). Reviewed By: jay-zhuang Differential Revision: D23794824 Pulled By: riversand963 fbshipit-source-id: 6bc02a59fd25613d343a62cf817467c7122c9721 22 September 2020, 19:37:16 UTC
29f7bbe Fix RocksDB SIGILL error on Raspberry PI 4 (#7233) Summary: Issue:https://github.com/facebook/rocksdb/issues/7042 No PMULL runtime check will lead to SIGILL on a Raspberry pi 4. Leverage 'getauxval' to get Hardware-Cap to detect whether target platform does support PMULL or not in runtime. Consider the condition that the target platform does support crc32 but not support PMULL. In this condition, the code should leverage the crc32 instruction rather than skip all hardware crc32 instruction. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7233 Reviewed By: jay-zhuang Differential Revision: D23790116 fbshipit-source-id: a3ebd821fbd4a38dd2f59064adbb7c3013ee8140 22 September 2020, 17:41:19 UTC
3591da3 Add a release build with RTTI in CircleCI (#7364) Summary: Release build RTTI is not covered in CI. Add one. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7364 Test Plan: Watch the build results. Reviewed By: jay-zhuang Differential Revision: D23602157 fbshipit-source-id: f0bb0f918632c5ee009db9d0a47a771f3c98195b 22 September 2020, 17:28:01 UTC
6727259 Possible fix to flaky db_write_test (#7418) Summary: Make the test robust to spurious wakeups on condition variable, and clear sync points to ensure no use-after-free. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7418 Test Plan: repeated runs on updated test, watch CircleCI for recurrence Reviewed By: jay-zhuang Differential Revision: D23828823 Pulled By: pdillinger fbshipit-source-id: af85117d9c02602541a90252840e0e5a6996de5b 22 September 2020, 16:57:05 UTC
back to top