| db03172 | mrambacher | 08 December 2020, 04:09:55 UTC | Change ErrorHandler methods to return const Status& (#7539) Summary: This change eliminates the need for a lot of the PermitUncheckedError calls on return from ErrorHandler methods. The calls are no longer needed as the status is returned as a reference rather than a copy. Additionally, this means that the originating status (recovery_error_, bg_error_) is not cleared implicitly as a result of calling one of these methods. For this class, I do not know if the proper behavior should be to call PermitUncheckedError in the destructor or if the checked state should be cleared when the status is cleared. I did tests both ways. Without the code in the destructor, the status will need to be cleared in at least some of the places where it is set to OK. When running tests, I found no instances where this class was destructed with a non-OK, non-checked Status. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7539 Reviewed By: anand1976 Differential Revision: D25340565 Pulled By: pdillinger fbshipit-source-id: 1730c035c81a475875ea745226112030ec25136c | 08 December 2020, 04:11:35 UTC |
| 8a06fe2 | Levi Tamasi | 08 December 2020, 01:35:54 UTC | Do not use ASSERT_OK in child threads in ExternalSstFileTest.PickedLevelBug (#7754) Summary: `googletest` uses exceptions to communicate assertion failures when `GTEST_THROW_ON_FAILURE` is set, which does not go well with `std::thread`s, since an exception escaping the top-level function of an `std::thread` object or an `std::thread` getting destroyed without having been `join`ed or `detach`ed first results in a call to `std::terminate`. The patch fixes this by moving the `Status` assertions of background operations in `ExternalSstFileTest.PickedLevelBug` to the main thread. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7754 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D25383808 Pulled By: ltamasi fbshipit-source-id: 32fb2721e5169ec898d218900bc0d83eead45d03 | 08 December 2020, 01:37:17 UTC |
| ba2a3bf | davkor | 07 December 2020, 22:01:05 UTC | OSS-Fuzz integration and db_fuzzer (#7674) Summary: This PR adds a fuzzer to the project and infrastructure to integrate Rocksdb with OSS-Fuzz. OSS-Fuzz is a service run by Google that performs continuous fuzzing of important open source projects. The LevelDB project is also in being fuzzed by OSS-Fuzz (https://github.com/google/oss-fuzz/tree/master/projects/leveldb). Essentially, OSS-Fuzz will perform the fuzzing for you and email you bug reports, coverage reports etc. All we need is a set of email addresses that will receive this information. For cross-referencing, the PR that adds the OSS-Fuzz logic is here: https://github.com/google/oss-fuzz/pull/4642 The `db_fuzzer` of the PR performs stateful fuzzing of Rocksdb by calling a sequence of Rockdb's APIs with random input in each fuzz iteration. Each fuzz iteration, thus, creates a new instance of Rocksdb and operates on this given instance. The goal is to test diverse states of Rocksdb and ensure no state lead to error conditions, e.g. memory corruption vulnerabilities. The fuzzer is similar (although more complex) to the fuzzer that is currently being used to analyse Leveldb (https://github.com/google/oss-fuzz/blob/master/projects/leveldb/fuzz_db.cc) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7674 Reviewed By: pdillinger Differential Revision: D25238536 Pulled By: cheng-chang fbshipit-source-id: 610331c49a77eb68d3b1d7d5ef1b0ce230ac0630 | 07 December 2020, 22:02:20 UTC |
| 20c7d7c | Akanksha Mahajan | 07 December 2020, 21:42:27 UTC | Handling misuse of snprintf return value (#7686) Summary: Handle misuse of snprintf return value to avoid Out of bound read/write. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7686 Test Plan: make check -j64 Reviewed By: riversand963 Differential Revision: D25030831 Pulled By: akankshamahajan15 fbshipit-source-id: 1a1d181c067c78b94d720323ae00b79566b57cfa | 07 December 2020, 21:43:55 UTC |
| b77569f | Neil Mitchell | 07 December 2020, 18:26:20 UTC | Make the TARGETS file Starlark compliant (#7743) Summary: Buck TARGETS files are sometimes parsed with Python, and sometimes with Starlark - this TARGETS file was not Starlark compliant. In Starlark you can't have a top-level if in a TARGETS file, but you can have a ternary `a if b else c`. Therefore I converted TARGETS, and updated the generator for it. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7743 Reviewed By: pdillinger Differential Revision: D25342587 Pulled By: ndmitchell fbshipit-source-id: 88cbe8632071a45a3ea8675812967614c62c78d1 | 07 December 2020, 18:28:26 UTC |
| 1df8584 | Akanksha Mahajan | 07 December 2020, 18:23:17 UTC | Fix unit test failure ppc64le in travis (#7752) Summary: Added a fix for the failure of DBTest2.PartitionedIndexUserToInternalKey on ppc64le in travis Closes https://github.com/facebook/rocksdb/issues/7746 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7752 Test Plan: Ran travis job multiple times and it passed. Will keep watching the travis job after this patch. Reviewed By: pdillinger Differential Revision: D25373130 Pulled By: akankshamahajan15 fbshipit-source-id: fa0e3f85f75b687415044a506e42cc38ead87975 | 07 December 2020, 18:24:33 UTC |
| eee0af9 | Yanqin Jin | 05 December 2020, 22:17:11 UTC | Add full_history_ts_low to column family (#7740) Summary: Following https://github.com/facebook/rocksdb/issues/7655 and https://github.com/facebook/rocksdb/issues/7657, this PR adds `full_history_ts_low_` to `ColumnFamilyData`. `ColumnFamilyData::full_history_ts_low_` will be used to create `FlushJob` and `CompactionJob`. `ColumnFamilyData::full_history_ts_low` is persisted to the MANIFEST file. An application can only increase its value. Consider the following case: > > The database has a key at ts=950. `full_history_ts_low` is first set to 1000, and then a GC is triggered > and cleans up all data older than 1000. If the application sets `full_history_ts_low` to 900 afterwards, > and tries to read at ts=960, the key at 950 is not seen. From the perspective of the read, the result > is hard to reason. For simplicity, we just do now allow decreasing full_history_ts_low for now. > During recovery, the value of `full_history_ts_low` is restored for each column family if applicable. Note that version edits in the MANIFEST file for the same column family may have `full_history_ts_low` unsorted due to the potential interleaving of `LogAndApply` calls. Only the max will be used to restore the state of the column family. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7740 Test Plan: make check Reviewed By: ltamasi Differential Revision: D25296217 Pulled By: riversand963 fbshipit-source-id: 24acda1df8262cd7cfdc6ce7b0ec56438abe242a | 05 December 2020, 22:18:22 UTC |
| e34b2e9 | Peter Dillinger | 05 December 2020, 07:22:18 UTC | Migrate away from broken macos on Travis (#7745) Summary: Add macos+cmake build on CircleCI instead. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7745 Test Plan: CI Reviewed By: riversand963 Differential Revision: D25352864 Pulled By: pdillinger fbshipit-source-id: 6b0a328cbe715bc3b43d70e919a27c834edcf079 | 05 December 2020, 07:24:27 UTC |
| 61932cd | Levi Tamasi | 05 December 2020, 05:28:26 UTC | Add blob support to DBIter (#7731) Summary: The patch adds iterator support to the integrated BlobDB implementation. Whenever a blob reference is encountered during iteration, the corresponding blob is retrieved by calling `Version::GetBlob`, assuming the `expose_blob_index` (formerly `allow_blob`) flag is *not* set. (Note: the flag is set by the old stacked BlobDB implementation, which has its own blob file handling/blob retrieval logic.) In addition, `DBIter` now uniformly returns `Status::NotSupported` with the error message `"BlobDB does not support merge operator."` when encountering a blob reference while performing a merge (instead of potentially returning a message that implies the database should be opened using the stacked BlobDB's `Open`.) TODO: We can implement support for lazily retrieving the blob value (or in other words, bypassing the retrieval of blob values based on key) by extending the `Iterator` API with a new `PrepareValue` method (similarly to `InternalIterator`, which already supports lazy values). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7731 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D25256293 Pulled By: ltamasi fbshipit-source-id: c39cd782011495a526cdff99c16f5fca400c4811 | 05 December 2020, 05:29:38 UTC |
| e102de7 | Zhichao Cao | 05 December 2020, 04:30:23 UTC | Fix assert(cfd->imm()->NumNotFlushed() > 0) in FlushMemtable (#7744) Summary: In current code base, in FlushMemtable, when `(Flush_reason == FlushReason::kErrorRecoveryRetryFlush && (!cfd->mem()->IsEmpty() || !cached_recoverable_state_empty_.load()))`, we assert that cfd->imm()->NumNotFlushed() > 0. However, there are some corner cases that can fail this assert: 1) if there are multiple CFs, some CF has immutable memtable, some CFs don't. In ResumeImpl, all CFs will call FlushMemtable, which will hit the assert. 2) Regular flush is scheduled and running, the resume thread is waiting. New KVs are inserted and SchedulePendingFlush is called. Regular flush will continue call MaybeScheduleFlushAndCompaction until all the immutable memtables are flushed. When regular flush ends and auto resume thread starts to schedule new flushes, cfd->imm()->NumNotFlushed() can be 0. Remove the assert and added the comments. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7744 Test Plan: make check and pass the stress test Reviewed By: riversand963 Differential Revision: D25340573 Pulled By: zhichao-cao fbshipit-source-id: eac357bdace660247c197f01a9ff6857e3c97672 | 05 December 2020, 04:31:39 UTC |
| ee4bd47 | Adam Retter | 04 December 2020, 23:21:18 UTC | Fix compilation on Apple Silicon (#7714) Summary: Closes - https://github.com/facebook/rocksdb/issues/7710 I tested this on an Apple DTK (Developer Transition Kit) with an Apple A12Z Bionic CPU and macOS Big Sur (11.0.1). Previously the arm64 specific CRC optimisations were limited to Linux only OS... Well now Apple Silicon is also arm64 but runs macOS ;-) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7714 Reviewed By: ltamasi Differential Revision: D25287349 Pulled By: pdillinger fbshipit-source-id: 639b168bf0ac2652907531e9604936ac4974b577 | 04 December 2020, 23:22:33 UTC |
| eb5a8c0 | Zhichao Cao | 04 December 2020, 22:57:29 UTC | Fix the thread wait case in error_handler (#7700) Summary: In error_handler auto recovery case, if recovery_in_prog_ is false, the recover is finished or failed. In this case, the auto recovery thread should finish its execution so recovery_thread_ should be null. However, in some cases, it is not null, the caller should not directly returned. Instead, it should wait for a while and create a new thread to execute the new recovery. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7700 Test Plan: make check, error_handler_fs_test Reviewed By: anand1976 Differential Revision: D25098233 Pulled By: zhichao-cao fbshipit-source-id: 5a1cba234ca18f6dd5d1be88e02d66e1d5ce931b | 04 December 2020, 22:58:37 UTC |
| 70f2e09 | Cheng Chang | 04 December 2020, 03:21:08 UTC | Write min_log_number_to_keep to MANIFEST during atomic flush under 2 phase commit (#7570) Summary: When 2 phase commit is enabled, if there are prepared data in a WAL, the WAL should be kept, the minimum log number for such a WAL is written to MANIFEST during flush. In atomic flush, such information is not written to MANIFEST. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7570 Test Plan: Added a new unit test `DBAtomicFlushTest.ManualFlushUnder2PC`, this test fails in atomic flush without this PR, after this PR, it succeeds. Reviewed By: riversand963 Differential Revision: D24394222 Pulled By: cheng-chang fbshipit-source-id: 60ce74b21b704804943be40c8de01b41269cf116 | 04 December 2020, 03:22:24 UTC |
| ac2f90d | Ramkumar Vadivelu | 03 December 2020, 20:44:07 UTC | add 6.15.fb to check_format_compatible.sh (#7738) Summary: Update check_format_compatible.sh with 6.15.fb Pull Request resolved: https://github.com/facebook/rocksdb/pull/7738 Reviewed By: ajkr Differential Revision: D25307717 Pulled By: ramvadiv fbshipit-source-id: 49f5c6366e8c8a2ade9697975453c9c65e919f1b | 03 December 2020, 20:45:14 UTC |
| 29e8f6a | Zhichao Cao | 03 December 2020, 02:22:05 UTC | Add kManifestWriteNoWAL to BackgroundErrorReason to handle Flush IO Error when WAL is disabled (#7693) Summary: In the current code base, all the manifest writes with IO error will be set with reason: BackgroundErrorReason::kManifestWrite, which will be mapped to the kHardError if the IO Error is retryable. However, if the system does not use the WAL, all the retryable IO error should be mapped to kSoftError. Create this PR to handle is special case by adding kManifestWriteNoWAL to BackgroundErrorReason. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7693 Test Plan: make check, add new testing cases to error_handler_fs_test Reviewed By: anand1976 Differential Revision: D25066204 Pulled By: zhichao-cao fbshipit-source-id: d59553896c2eac3fb37c05238544d2b265379462 | 03 December 2020, 02:24:01 UTC |
| 3b9bfe8 | Peter Dillinger | 02 December 2020, 23:15:17 UTC | Skip minimum rate check in Sandcastle (#7728) Summary: The minimum rate check in RateLimiterTest.Rate can fail in Facebook's CI system Sandcastle, presumably due to heavily loaded machines. This change disables the minimum rate check for Sandcastle runs, and cleans up the code disabling it on other CI environments. (The amount of conditionally compiled code shall be minimized.) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7728 Test Plan: try new test with and without setting envvar SANDCASTLE=1 Reviewed By: ltamasi Differential Revision: D25247642 Pulled By: pdillinger fbshipit-source-id: d786233af37af9a874adbb3a9e2707ec52c27a5a | 02 December 2020, 23:16:49 UTC |
| 7fec715 | Jay Zhuang | 02 December 2020, 20:59:23 UTC | Make CompactRange and GetApproximateSizes work with timestamp (#7684) Summary: Add timestamp to the `CompactRange()` and `GetApproximateSizes` range keys if needed. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7684 Test Plan: make check Reviewed By: riversand963 Differential Revision: D25015421 Pulled By: jay-zhuang fbshipit-source-id: 51ca0756087eb053a3b11801e5c7ce1c6e2d38a9 | 02 December 2020, 21:00:53 UTC |
| e062a71 | Yanqin Jin | 02 December 2020, 17:29:50 UTC | Fix assertion failure in bg flush (#7362) Summary: https://github.com/facebook/rocksdb/issues/7340 reports and reproduces an assertion failure caused by a combination of the following: - atomic flush is disabled. - a column family can appear multiple times in the flush queue at the same time. This behavior was introduced in release 5.17. Consequently, it is possible that two flushes race with each other. One bg flush thread flushes all memtables. The other thread calls `FlushMemTableToOutputFile()` afterwards, and hits the assertion error below. ``` assert(cfd->imm()->NumNotFlushed() != 0); assert(cfd->imm()->IsFlushPending()); ``` Fix this by reverting the behavior. In non-atomic-flush case, a column family can appear in the flush queue at most once at the same time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7362 Test Plan: make check Also run stress test successfully for 10 times. ``` make crash_test ``` Reviewed By: ajkr Differential Revision: D25172996 Pulled By: riversand963 fbshipit-source-id: f1559b6366cc609e961e3fc83fae548f1fad08ce | 02 December 2020, 17:31:14 UTC |
| 9e16404 | Jay Zhuang | 01 December 2020, 22:05:19 UTC | Exclude timestamp from prefix extractor (#7668) Summary: Timestamp should not be included in prefix extractor, as we discussed here: https://github.com/facebook/rocksdb/pull/7589#discussion_r511068586 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7668 Test Plan: added unittest Reviewed By: riversand963 Differential Revision: D24966265 Pulled By: jay-zhuang fbshipit-source-id: 0dae618c333d4b7942a40d556535a1795e060aea | 01 December 2020, 22:07:15 UTC |
| b937be3 | Adam Retter | 01 December 2020, 19:20:10 UTC | Fix Compilation on ppc64le using Clang 11 (#7713) Summary: Closes https://github.com/facebook/rocksdb/issues/7691 The optimised CRC code for PPC64le which was originally imported in https://github.com/facebook/rocksdb/pull/2353 is not compatible with Clang 11. It looks like the code most likely originated from https://github.com/antonblanchard/crc32-vpmsum. The code relied on a GCC header file `ppc-asm.h` which is not available in Clang. To solve this, I have taken the same approach as the the upstream project from which the CRC code came https://github.com/antonblanchard/crc32-vpmsum/commit/ffc8018efc1e4f05d22a9fc8dde57109dd09368b#diff-ec3e62c56fbcddeb07230f2a4673c1abd7f0f1cc8e48a2aa560056cfc1b25d60 and simply imported a copy of the GCC header file into our code-base which will be used when Clang is the compiler on pcc64le. **NOTE**: The new file `util/ppc-asm.h` may have licensing implications which I guess need to be approved by RocksDB/Facebook before this is merged Pull Request resolved: https://github.com/facebook/rocksdb/pull/7713 Reviewed By: jay-zhuang Differential Revision: D25222645 Pulled By: pdillinger fbshipit-source-id: e3fec9136f26ce1eb7a027048bcf77a6cb3c769c | 01 December 2020, 19:21:44 UTC |
| 0b06af9 | Peter Dillinger | 01 December 2020, 18:15:36 UTC | Warn about practically unfixable TSAN warnings in stack trace (#7723) Summary: TSAN reports that our stack trace handler makes unsafe calls during a signal handler. I just tried fixing some of them and I don't think it's fixable unless we can get away from using FILE stdio. Even if we can use lower level functions only, I'm not sure it's fixed. I also tried suppressing the reports with function and file level TSAN suppression, but that doesn't seem to work, perhaps because the violation is reported on the callee, not the caller. So I added a warning to be printed whenever these violations would be reported that they are practically ignorable. Internal ref: T77844138 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7723 Test Plan: run external_sst_file_test with seeded abort(), with TSAN (TSAN warnings + new warning) and without TSAN (no warning, just stack trace). Reviewed By: akankshamahajan15 Differential Revision: D25228011 Pulled By: pdillinger fbshipit-source-id: 3eda1d6e7ca3cdc64076cf99ae954168837d2818 | 01 December 2020, 18:17:56 UTC |
| eb65d67 | Andrew Kryczka | 01 December 2020, 02:10:18 UTC | Fix kPointInTimeRecovery handling of truncated WAL (#7701) Summary: WAL may be truncated to an incomplete record due to crash while writing the last record or corruption. In the former case, no hole will be produced since no ACK'd data was lost. In the latter case, a hole could be produced without this PR since we proceeded to recover the next WAL as if nothing happened. This PR changes the record reading code to always report a corruption for incomplete records in `kPointInTimeRecovery` mode, and the upper layer will only ignore them if the next WAL has consecutive seqnum (i.e., we are guaranteed no hole). While this solves the hole problem for the case of incomplete records, the possibility is still there if the WAL is corrupted by truncation to an exact record boundary. This PR also regresses how much data can be recovered when writes are mixed with/without `WriteOptions::disableWAL`, as then we can not distinguish between a seqnum gap caused by corruption and a seqnum gap caused by a `disableWAL` write. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7701 Test Plan: Interestingly there already was a test for this case (`DBWALTestWithParams.kPointInTimeRecovery`); it just had a typo bug in the verification that prevented it from noticing holes in recovery. Reviewed By: anand1976 Differential Revision: D25111765 Pulled By: ajkr fbshipit-source-id: 5e330b13b1ee2b5be096cea9d0ff6075843e57b6 | 01 December 2020, 02:11:38 UTC |
| cc431ec | Steve Yen | 30 November 2020, 20:08:23 UTC | Fix merge operator docs typo (#7716) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7716 Reviewed By: pdillinger Differential Revision: D25214340 Pulled By: zhichao-cao fbshipit-source-id: 143a8e7d076917e60bbe6993d60ec55f33e2ab56 | 30 November 2020, 20:09:39 UTC |
| 51a8dc6 | Levi Tamasi | 24 November 2020, 05:07:01 UTC | Integrated blob garbage collection: relocate blobs (#7694) Summary: The patch adds basic garbage collection support to the integrated BlobDB implementation. Valid blobs residing in the oldest blob files are relocated as they are encountered during compaction. The threshold that determines which blob files qualify is computed based on the configuration option `blob_garbage_collection_age_cutoff`, which was introduced in https://github.com/facebook/rocksdb/issues/7661 . Once a blob is retrieved for the purposes of relocation, it passes through the same logic that extracts large values to blob files in general. This means that if, for instance, the size threshold for key-value separation (`min_blob_size`) got changed or writing blob files got disabled altogether, it is possible for the value to be moved back into the LSM tree. In particular, one way to re-inline all blob values if needed would be to perform a full manual compaction with `enable_blob_files` set to `false`, `enable_blob_garbage_collection` set to `true`, and `blob_file_garbage_collection_age_cutoff` set to `1.0`. Some TODOs that I plan to address in separate PRs: 1) We'll have to measure the amount of new garbage in each blob file and log `BlobFileGarbage` entries as part of the compaction job's `VersionEdit`. (For the time being, blob files are cleaned up solely based on the `oldest_blob_file_number` relationships.) 2) When compression is used for blobs, the compression type hasn't changed, and the blob still qualifies for being written to a blob file, we can simply copy the compressed blob to the new file instead of going through decompression and compression. 3) We need to update the formula for computing write amplification to account for the amount of data read from blob files as part of GC. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7694 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D25069663 Pulled By: ltamasi fbshipit-source-id: bdfa8feb09afcf5bca3b4eba2ba72ce2f15cd06a | 24 November 2020, 05:08:22 UTC |
| dd6b7fc | Andrew Kryczka | 24 November 2020, 00:27:46 UTC | Return `Status` from `MemTable` mutation functions (#7656) Summary: This PR updates `MemTable::Add()`, `MemTable::Update()`, and `MemTable::UpdateCallback()` to return `Status` objects, and adapts the client code in `MemTableInserter`. The goal is to prepare these functions for key-value checksum, where we want to verify key-value integrity while adding to memtable. After this PR, the memtable mutation functions can report a failed integrity check by returning `Status::Corruption`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7656 Reviewed By: riversand963 Differential Revision: D24900497 Pulled By: ajkr fbshipit-source-id: 1a7e80581e3774676f2bbba2f0a0b04890f40009 | 24 November 2020, 00:29:04 UTC |
| 0baa505 | Peter Dillinger | 23 November 2020, 03:50:42 UTC | Add Ribbon schema test to bloom_test (#7696) Summary: These new unit tests should ensure that we don't accidentally change the interpretation of bits for what I call Standard128Ribbon filter internally, available publicly as NewExperimentalRibbonFilterPolicy. There is very little intuitive reason for the values we check against in these tests; I just plug in the right expected values upon watching the test fail initially. Most (but not all) of the tests are essentially "whitebox" "round-trip." We create a filter from fixed keys, and first compare the checksum of those filter bytes against a saved value. We also run queries against other fixed keys, comparing which return false positives against a saved set. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7696 Test Plan: test addition and refactoring only Reviewed By: jay-zhuang Differential Revision: D25082289 Pulled By: pdillinger fbshipit-source-id: b5ca646fdcb5a1c2ad2085eda4a1fd44c4287f67 | 23 November 2020, 03:52:04 UTC |
| 1a5fc4f | Yanqin Jin | 21 November 2020, 02:38:47 UTC | Port corruption test to use custom env (#7699) Summary: Allow corruption_test to run on custom env loaded via `Env::LoadEnv()`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7699 Test Plan: ``` make corruption_test ./corruption_test ``` Also run on in-house custom env. Reviewed By: zhichao-cao Differential Revision: D25135525 Pulled By: riversand963 fbshipit-source-id: 7941e7ce342dc88ec2cd63e90f7674a2f57de6b7 | 21 November 2020, 02:40:24 UTC |
| 7c19d43 | anand76 | 20 November 2020, 06:37:55 UTC | Fix initialization order of DBOptions and kHostnameForDbHostId (#7702) Summary: Fix initialization order of DBOptions and kHostnameForDbHostId by making the initialization of the latter static rather than dynamic. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7702 Reviewed By: ajkr Differential Revision: D25111633 Pulled By: anand1976 fbshipit-source-id: 7afad834a66e40bcd8694a43b40d378695212224 | 20 November 2020, 06:39:40 UTC |
| 5c585e1 | Cheng Chang | 19 November 2020, 22:44:48 UTC | Ship the track WAL in MANIFEST feature (#7689) Summary: Updates the option description and HISTORY. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7689 Test Plan: N/A Reviewed By: zhichao-cao Differential Revision: D25056238 Pulled By: cheng-chang fbshipit-source-id: 6af1ef6f8dcf2173cbc0fccadc0e06cefd92bcae | 19 November 2020, 22:45:54 UTC |
| a65e905 | Dylan Wen | 19 November 2020, 21:31:16 UTC | Fix typos in comments (#7687) Summary: Hi there, This PR fixes a few typos in comments in `cache/lru_cache.h`. Thanks Pull Request resolved: https://github.com/facebook/rocksdb/pull/7687 Reviewed By: ajkr Differential Revision: D25064674 Pulled By: jay-zhuang fbshipit-source-id: fe633369d5b82c5aac42d4ee8d551b9d657237d1 | 19 November 2020, 21:32:50 UTC |
| 7169ca9 | Cheng Chang | 19 November 2020, 05:24:32 UTC | Do not track empty WALs (#7697) Summary: An empty WAL won't be backed up by the BackupEngine. So if we track the empty WALs in MANIFEST, then when restoring from a backup, it may report corruption that the empty WAL is missing, which is correct because the WAL is actually in the main DB but not in the backup DB, but missing an empty WAL does not logically break DB consistency. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7697 Test Plan: watch existing tests to pass Reviewed By: pdillinger Differential Revision: D25077194 Pulled By: cheng-chang fbshipit-source-id: 01917b57234b92b6063925f2ee9452c5732bdc03 | 19 November 2020, 05:27:54 UTC |
| 8a97f35 | Cheng Chang | 18 November 2020, 22:53:14 UTC | Call out a bug in HISTORY (#7690) Summary: It's worth mentioning the corner case bug fixed in PR 7621. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7690 Test Plan: N/A Reviewed By: zhichao-cao Differential Revision: D25056678 Pulled By: cheng-chang fbshipit-source-id: 1ab42ec080f3ffe21f5d97acf65ee0af993112ba | 18 November 2020, 22:54:22 UTC |
| 6cacb0d | Akanksha Mahajan | 18 November 2020, 02:18:13 UTC | Add cmake-mingw in circle-build (#7144) Summary: Add cmake-mignw in circle-build Pull Request resolved: https://github.com/facebook/rocksdb/pull/7144 Test Plan: watch circle cmake-mingw build Reviewed By: jay-zhuang Differential Revision: D25039744 Pulled By: akankshamahajan15 fbshipit-source-id: 92584c9d5ad161b93d5e5a1303aac306e7985108 | 18 November 2020, 02:19:50 UTC |
| 8c93b16 | Cheng Chang | 17 November 2020, 23:54:49 UTC | Track WAL in MANIFEST: Update logic for computing min_log_number_to_keep in atomic flush (#7660) Summary: The logic for computing min_log_number_to_keep in atomic flush was incorrect. For example, when all column families are flushed, the min_log_number_to_keep should be the latest new log. But the incorrect logic calls `PrecomputeMinLogNumberToKeepNon2PC` for each column family, and returns the minimum of them. However, `PrecomputeMinLogNumberToKeepNon2PC(cf)` assumes column families other than `cf` are flushed, but in case all column families are flushed, this assumption is incorrect. Without this fix, the WAL referenced by the computed min_log_number_to_keep may actually contain no unflushed data, so the WAL might have actually been deleted from disk on recovery, then an incorrect error `Corruption: missing WAL` will be reported. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7660 Test Plan: run `make crash_test_with_atomic_flush` on devserver added a unit test in `db_flush_test` Reviewed By: riversand963 Differential Revision: D24906265 Pulled By: cheng-chang fbshipit-source-id: 08deda62e71f67f59e3b7925cdd86dd09bd4f430 | 17 November 2020, 23:55:55 UTC |
| 303d283 | Adam Retter | 17 November 2020, 23:28:56 UTC | RocksJava static lib dependencies should support MacOS 10.12+ (#7683) Summary: Expands on https://github.com/facebook/rocksdb/pull/7016 so that when `PORTABLE=1` is set the dependencies for RocksJava static target will also be built with backwards compatibility for MacOS as far back as 10.12 (i.e. 2016). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7683 Reviewed By: ajkr Differential Revision: D25034164 Pulled By: pdillinger fbshipit-source-id: dc9e51828869ed9ec336a8a86683e4d0bfe04f27 | 17 November 2020, 23:34:05 UTC |
| 4c336c6 | Adam Retter | 17 November 2020, 23:27:51 UTC | Fix jemalloc compliation problem on macOS (#7624) Summary: Closes https://github.com/facebook/rocksdb/issues/7269 I have only tested this on macOS, let's see what CI makes of it for the other platforms... Pull Request resolved: https://github.com/facebook/rocksdb/pull/7624 Reviewed By: ajkr Differential Revision: D24834305 Pulled By: pdillinger fbshipit-source-id: ba818d8424297ccebd18ed854b044764c2dbab5f | 17 November 2020, 23:29:35 UTC |
| 699411b | Cheng Chang | 17 November 2020, 20:55:27 UTC | Fuzzing RocksDB (#7685) Summary: This is the initial PR to support adding fuzz tests to RocksDB. It includes the necessary build infrastructure, and includes an example fuzzer. There is also a README serving as the tutorial for how to add more tests. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7685 Test Plan: Manually build and run the fuzz test according to README. Reviewed By: pdillinger Differential Revision: D25013847 Pulled By: cheng-chang fbshipit-source-id: c91e3b337398d7f4d8f769fd5091cd080487b171 | 17 November 2020, 20:56:48 UTC |
| 84a7008 | Yanqin Jin | 17 November 2020, 08:43:20 UTC | Fix the logic of setting read_amp_bytes_per_bit from OPTIONS file (#7680) Summary: Instead of using `EncodeFixed32` which always serialize a integer to little endian, we should use the local machine's endianness when populating a native data structure during options parsing. Without this fix, `read_amp_bytes_per_bit` may be populated incorrectly on big-endian machines. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7680 Test Plan: make check Reviewed By: pdillinger Differential Revision: D24999166 Pulled By: riversand963 fbshipit-source-id: dc603cff6e17f8fa32479ce6df93b93082e6b0c4 | 17 November 2020, 08:44:30 UTC |
| 869f053 | Yanqin Jin | 17 November 2020, 06:07:29 UTC | Clean up after two test failures in db_basic_test (#7682) Summary: In db_basic_test.cc, there are two tests that rely on the underlying system's `LockFile` support to function correctly: DBBasicTest.OpenWhenOpen and DBBasicTest.CheckLock. In both tests, re-opening a db using `DB::Open` is expected to fail because the second open cannot lock the LOCK file. Some distributed file systems, e.g. HDFS do not support the POSIX-style file lock. Therefore, these unit tests will cause assertion failure and the second `Open` will create a db instance. Currently, these db instances are not closed after the assertion failure. Since these db instances are registered with some process-wide, static data structures, e.g. `PeriodicWorkScheduler::Default()`, they can still be accessed after the unit tests. However, the `Env` object created for this db instance is destroyed when the test finishes in `~DBTestBase()`. Consequently, it causes illegal memory access. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7682 Test Plan: Run the following on a distrubited file system: ``` make check ``` Reviewed By: anand1976 Differential Revision: D25004215 Pulled By: riversand963 fbshipit-source-id: f4327d7716c0e72b13bb43737ec9a5d156da4d52 | 17 November 2020, 06:09:01 UTC |
| 9627e34 | anand76 | 17 November 2020, 02:41:43 UTC | Use default FileSystem in GenerateUniqueId (#7672) Summary: Use ```FileSystem::Default``` to read ```/proc/sys/kernel/uuid```, so it works for ```Envs``` with remote ```FileSystem``` as well. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7672 Reviewed By: riversand963 Differential Revision: D24998702 Pulled By: anand1976 fbshipit-source-id: fa95c1d70f0e4ed17561201f047aa055046d06c3 | 17 November 2020, 04:48:13 UTC |
| 1c5f13f | Andrew Kryczka | 17 November 2020, 01:58:59 UTC | Fail early when `merge_operator` not configured (#7667) Summary: An application may accidentally write merge operands without properly configuring `merge_operator`. We should alert them as early as possible that there's an API misuse. Previously RocksDB only notified them when a query or background operation needed to merge but couldn't. With this PR, RocksDB notifies them of the problem before applying the merge operand to the memtable (although it may already be in WAL, which seems it'd cause a crash loop until they enable `merge_operator`). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7667 Reviewed By: riversand963 Differential Revision: D24933360 Pulled By: ajkr fbshipit-source-id: 3a4a2ceb0b7aed184113dd03b8efd735a8332f7f | 17 November 2020, 04:39:01 UTC |
| 7582c56 | jsteemann | 16 November 2020, 23:51:21 UTC | add ArangoDB to USERS.md, and fix typos in that file (#7675) Summary: Add ArangoDB to USERS.md. We are using RocksDB since 2016. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7675 Reviewed By: riversand963 Differential Revision: D24998955 Pulled By: ajkr fbshipit-source-id: 82c656bf56589e52aff8c491bab6fbc19b52cc91 | 17 November 2020, 02:29:51 UTC |
| 1861de4 | Mammo, Mulugeta | 16 November 2020, 21:05:21 UTC | Add arena_block_size flag to db_bench (#7654) Summary: db_bench currently does not allow overriding the default `arena_block_size `calculation ([memtable size/8](https://github.com/facebook/rocksdb/blob/master/db/column_family.cc#L216)). For memtables whose size is in gigabytes, the `arena_block_size` defaults to hundreds of megabytes (affecting performance). Exposing this option in db_bench would allow us to test the workloads with various `arena_block_size` values. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7654 Reviewed By: jay-zhuang Differential Revision: D24996812 Pulled By: ajkr fbshipit-source-id: a5e3d2c83d9f89e1bb8382f2e8dd476c79e33bef | 16 November 2020, 21:06:30 UTC |
| 5bd1258 | Ramkumar Vadivelu | 15 November 2020, 20:36:24 UTC | Update release history to 6.15 (#7673) Summary: Update release history to 6.15 Pull Request resolved: https://github.com/facebook/rocksdb/pull/7673 Test Plan: No code change Reviewed By: ajkr Differential Revision: D24971069 Pulled By: ramvadiv fbshipit-source-id: 5cb3f5cbc1b19beb580ea8095acdef72cc092905 | 15 November 2020, 20:37:24 UTC |
| 1aae417 | Cheng Chang | 13 November 2020, 21:36:00 UTC | Do not track WAL in MANIFEST when fsync is disabled in a test (#7669) Summary: If fsync is disabled in a unit test, then do not track WAL in MANIFEST, because on DB recovery, the WAL might be missing because the directory is not fsynced. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7669 Test Plan: Tests with fsync enabled should pass. Reviewed By: riversand963 Differential Revision: D24941431 Pulled By: cheng-chang fbshipit-source-id: ab3ff0f90769795cfb4e4d6dcf084ea5545d1975 | 13 November 2020, 21:37:14 UTC |
| 9aa1b1d | Yanqin Jin | 13 November 2020, 19:51:24 UTC | 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 | 13 November 2020, 19:52:50 UTC |
| e300ce2 | Akanksha Mahajan | 13 November 2020, 06:08:03 UTC | Update option "allow_data_in_errors" in BuildOptions (#7665) Summary: "allow_data_in_errors" is not updated in BuildOptions. So it would assume default value when BuildOptions is called. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7665 Test Plan: make check -j64 Reviewed By: zhichao-cao Differential Revision: D24929100 Pulled By: akankshamahajan15 fbshipit-source-id: dd6225a6c9f13b20027ff1b6de8e79801b57b3f7 | 13 November 2020, 06:09:17 UTC |
| 60af964 | Peter Dillinger | 13 November 2020, 04:45:02 UTC | Experimental (production candidate) SST schema for Ribbon filter (#7658) Summary: Added experimental public API for Ribbon filter: NewExperimentalRibbonFilterPolicy(). This experimental API will take a "Bloom equivalent" bits per key, and configure the Ribbon filter for the same FP rate as Bloom would have but ~30% space savings. (Note: optimize_filters_for_memory is not yet implemented for Ribbon filter. That can be added with no effect on schema.) Internally, the Ribbon filter is configured using a "one_in_fp_rate" value, which is 1 over desired FP rate. For example, use 100 for 1% FP rate. I'm expecting this will be used in the future for configuring Bloom-like filters, as I expect people to more commonly hold constant the filter accuracy and change the space vs. time trade-off, rather than hold constant the space (per key) and change the accuracy vs. time trade-off, though we might make that available. ### Benchmarking ``` $ ./filter_bench -impl=2 -quick -m_keys_total_max=200 -average_keys_per_filter=100000 -net_includes_hashing Building... Build avg ns/key: 34.1341 Number of filters: 1993 Total size (MB): 238.488 Reported total allocated memory (MB): 262.875 Reported internal fragmentation: 10.2255% Bits/key stored: 10.0029 ---------------------------- Mixed inside/outside queries... Single filter net ns/op: 18.7508 Random filter net ns/op: 258.246 Average FP rate %: 0.968672 ---------------------------- Done. (For more info, run with -legend or -help.) $ ./filter_bench -impl=3 -quick -m_keys_total_max=200 -average_keys_per_filter=100000 -net_includes_hashing Building... Build avg ns/key: 130.851 Number of filters: 1993 Total size (MB): 168.166 Reported total allocated memory (MB): 183.211 Reported internal fragmentation: 8.94626% Bits/key stored: 7.05341 ---------------------------- Mixed inside/outside queries... Single filter net ns/op: 58.4523 Random filter net ns/op: 363.717 Average FP rate %: 0.952978 ---------------------------- Done. (For more info, run with -legend or -help.) ``` 168.166 / 238.488 = 0.705 -> 29.5% space reduction 130.851 / 34.1341 = 3.83x construction time for this Ribbon filter vs. lastest Bloom filter (could make that as little as about 2.5x for less space reduction) ### Working around a hashing "flaw" bloom_test discovered a flaw in the simple hashing applied in StandardHasher when num_starts == 1 (num_slots == 128), showing an excessively high FP rate. The problem is that when many entries, on the order of number of hash bits or kCoeffBits, are associated with the same start location, the correlation between the CoeffRow and ResultRow (for efficiency) can lead to a solution that is "universal," or nearly so, for entries mapping to that start location. (Normally, variance in start location breaks the effective association between CoeffRow and ResultRow; the same value for CoeffRow is effectively different if start locations are different.) Without kUseSmash and with num_starts > 1 (thus num_starts ~= num_slots), this flaw should be completely irrelevant. Even with 10M slots, the chances of a single slot having just 16 (or more) entries map to it--not enough to cause an FP problem, which would be local to that slot if it happened--is 1 in millions. This spreadsheet formula shows that: =1/(10000000*(1 - POISSON(15, 1, TRUE))) As kUseSmash==false (the setting for Standard128RibbonBitsBuilder) is intended for CPU efficiency of filters with many more entries/slots than kCoeffBits, a very reasonable work-around is to disallow num_starts==1 when !kUseSmash, by making the minimum non-zero number of slots 2*kCoeffBits. This is the work-around I've applied. This also means that the new Ribbon filter schema (Standard128RibbonBitsBuilder) is not space-efficient for less than a few hundred entries. Because of this, I have made it fall back on constructing a Bloom filter, under existing schema, when that is more space efficient for small filters. (We can change this in the future if we want.) TODO: better unit tests for this case in ribbon_test, and probably update StandardHasher for kUseSmash case so that it can scale nicely to small filters. ### Other related changes * Add Ribbon filter to stress/crash test * Add Ribbon filter to filter_bench as -impl=3 * Add option string support, as in "filter_policy=experimental_ribbon:5.678;" where 5.678 is the Bloom equivalent bits per key. * Rename internal mode BloomFilterPolicy::kAuto to kAutoBloom * Add a general BuiltinFilterBitsBuilder::CalculateNumEntry based on binary searching CalculateSpace (inefficient), so that subclasses (especially experimental ones) don't have to provide an efficient implementation inverting CalculateSpace. * Minor refactor FastLocalBloomBitsBuilder for new base class XXH3pFilterBitsBuilder shared with new Standard128RibbonBitsBuilder, which allows the latter to fall back on Bloom construction in some extreme cases. * Mostly updated bloom_test for Ribbon filter, though a test like FullBloomTest::Schema is a next TODO to ensure schema stability (in case this becomes production-ready schema as it is). * Add some APIs to ribbon_impl.h for configuring Ribbon filters. Although these are reasonably covered by bloom_test, TODO more unit tests in ribbon_test * Added a "tool" FindOccupancyForSuccessRate to ribbon_test to get data for constructing the linear approximations in GetNumSlotsFor95PctSuccess. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7658 Test Plan: Some unit tests updated but other testing is left TODO. This is considered experimental but laying down schema compatibility as early as possible in case it proves production-quality. Also tested in stress/crash test. Reviewed By: jay-zhuang Differential Revision: D24899349 Pulled By: pdillinger fbshipit-source-id: 9715f3e6371c959d923aea8077c9423c7a9f82b8 | 13 November 2020, 04:46:14 UTC |
| bbbb5a2 | Levi Tamasi | 13 November 2020, 02:57:20 UTC | Add options for integrated blob GC (#7661) Summary: This patch simply adds a couple of options that will enable users to configure garbage collection when using the integrated BlobDB implementation. The actual GC logic will be added in a separate step. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7661 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D24906544 Pulled By: ltamasi fbshipit-source-id: ee0e056a712a4b4475cd90de8b27d969bd61b7e1 | 13 November 2020, 02:58:44 UTC |
| 76ef894 | Yanqin Jin | 13 November 2020, 02:43:30 UTC | Add full_history_ts_low_ to FlushJob (#7655) Summary: https://github.com/facebook/rocksdb/issues/7556 enables `CompactionIterator` to perform garbage collection during compaction according to a lower bound (user-defined) timestamp `full_history_ts_low_`. This PR adds a data member `full_history_ts_low_` of type `std::string` to `FlushJob`, and `full_history_ts_low_` does not change during flush. `FlushJob` will pass a pointer to this data member to the `CompactionIterator` used during flush. Also refactored flush_job_test.cc to re-use some existing code, which is actually the majority of this PR. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7655 Test Plan: make check Reviewed By: ltamasi Differential Revision: D24933340 Pulled By: riversand963 fbshipit-source-id: 2e584bfd0cf6e5c295ab1af264e68e9d6a12fca3 | 13 November 2020, 02:44:34 UTC |
| bb69b4c | Levi Tamasi | 13 November 2020, 01:31:28 UTC | Fix InternalStats::DumpCFStats (#7666) Summary: https://github.com/facebook/rocksdb/pull/7461 accidentally broke `InternalStats::DumpCFStats` by making `DumpCFFileHistogram` overwrite the output of `DumpCFStatsNoFileHistogram` instead of appending to it, resulting in only the file histogram related information getting logged. The patch fixes this by reverting to appending in `DumpCFFileHistogram`. Fixes https://github.com/facebook/rocksdb/issues/7664 . Pull Request resolved: https://github.com/facebook/rocksdb/pull/7666 Test Plan: Ran `make check` and checked the info log of `db_bench`. Reviewed By: riversand963 Differential Revision: D24929051 Pulled By: ltamasi fbshipit-source-id: 636a3d5ebb5ce23de4f3fe4f03ad3f16cb2858f8 | 13 November 2020, 01:33:04 UTC |
| cf9d8e4 | Yanqin Jin | 12 November 2020, 19:40:52 UTC | Add full_history_ts_low_ to CompactionJob (#7657) Summary: https://github.com/facebook/rocksdb/issues/7556 enables `CompactionIterator` to perform garbage collection during compaction according to a lower bound (user-defined) timestamp `full_history_ts_low_`. This PR adds a data member `full_history_ts_low_` of type `std::string` to `CompactionJob`, and `full_history_ts_low_` does not change during compaction. `CompactionJob` will pass a pointer to this data member to the `CompactionIterator` used during compaction. Also refactored compaction_job_test.cc to re-use some existing code, which is actually the majority of this PR. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7657 Test Plan: make check Reviewed By: ltamasi Differential Revision: D24913803 Pulled By: riversand963 fbshipit-source-id: 11ad5329ddac365667152e7b3b02f84182c0ca8e | 12 November 2020, 19:43:24 UTC |
| 0dc437d | Levi Tamasi | 12 November 2020, 16:48:14 UTC | Clean up CompactionProxy (#7662) Summary: `CompactionProxy` is currently both a concrete class used for actual `Compaction`s and a base class that `FakeCompaction` (which is used in `compaction_iterator_test`) is derived from. This is bad from an OO design standpoint, and also results in `FakeCompaction` containing an (uninitialized and unused) `Compaction*` member. The patch fixes this by making `CompactionProxy` a pure interface and introducing a separate concrete class `RealCompaction` for non-test/non-fake compactions. It also removes an unused parameter from the virtual method `level`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7662 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D24907680 Pulled By: ltamasi fbshipit-source-id: c100ecb1beef4b0ada35e799116c5bda71719ee7 | 12 November 2020, 16:49:35 UTC |
| 2400cd6 | Yanqin Jin | 12 November 2020, 16:36:50 UTC | Update HISTORY.md for PR6069 (#7663) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7663 Reviewed By: ajkr Differential Revision: D24913081 Pulled By: riversand963 fbshipit-source-id: 704f427812f2b4f92e16d6cbc93be64d730d1cf9 | 12 November 2020, 16:38:41 UTC |
| ec346da | Andrew Kryczka | 12 November 2020, 04:30:58 UTC | Always apply bottommost_compression_opts when enabled (#7633) Summary: Previously, even when `bottommost_compression_opts`'s `enabled` flag was set, it only took effect when `bottommost_compression` was also set to something other than `kDisableCompressionOption`. This wasn't documented and, if we kept the old behavior, it'd make things complicated like the migration instructions in https://github.com/facebook/rocksdb/issues/7619. We can simplify the API by making `bottommost_compression_opts` always take effect when its `enabled` flag is set. Fixes https://github.com/facebook/rocksdb/issues/7631. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7633 Reviewed By: ltamasi Differential Revision: D24710358 Pulled By: ajkr fbshipit-source-id: bbbdf9c1b53c63a4239d902cc3f5a11da1874647 | 12 November 2020, 04:32:28 UTC |
| c442f68 | mrambacher | 11 November 2020, 23:09:14 UTC | Create a Customizable class to load classes and configurations (#6590) Summary: The Customizable class is an extension of the Configurable class and allows instances to be created by a name/ID. Classes that extend customizable can define their Type (e.g. "TableFactory", "Cache") and a method to instantiate them (TableFactory::CreateFromString). Customizable objects can be registered with the ObjectRegistry and created dynamically. Future PRs will make more types of objects extend Customizable. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6590 Reviewed By: cheng-chang Differential Revision: D24841553 Pulled By: zhichao-cao fbshipit-source-id: d0c2132bd932e971cbfe2c908ca2e5db30c5e155 | 11 November 2020, 23:10:41 UTC |
| 8b6b6ae | Yanqin Jin | 11 November 2020, 15:58:15 UTC | Refactor with VersionEditHandler (#6581) Summary: Added a few classes in the same class hierarchy to remove code duplication and refactor the logic of reading and processing MANIFEST files. New classes are as follows. ``` class VersionEditHandlerBase; class ListColumnFamiliesHandler : VersionEditHandlerBase; class FileChecksumRetriever : VersionEditHandlerBase; class DumpManifestHandler : VersionEditHandler; ``` Classes that already existed before this PR are as follows. ``` class VersionEditHandler : VersionEditHandlerBase; ``` With these classes, refactored functions: `VersionSet::Recover()`, `VersionSet::ListColumnFamilies()`, `VersionSet::DumpManifest()`, `GetFileChecksumFromManifest()`. Test Plan (devserver): ``` make check COMPILE_WITH_ASAN=1 make check ``` These refactored code, especially recovery-related logic, will be tested intensively by all existing unit tests and stress tests. For example, run ``` make crash_test ``` Verified 3 successful runs on devserver. Pull Request resolved: https://github.com/facebook/rocksdb/pull/6581 Reviewed By: ajkr Differential Revision: D20616217 Pulled By: riversand963 fbshipit-source-id: 048c7743aa4be2623ccd0cc3e61c0027e604e78b | 11 November 2020, 16:00:14 UTC |
| c57f914 | Peter Dillinger | 11 November 2020, 07:41:05 UTC | Use NPHash64 in more places (#7632) Summary: Since the hashes should not be persisted in output_validator nor mock_env. Also updated NPHash64 to use 64-bit seed, and comments. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7632 Test Plan: make check, and new build setting that enables modification to NPHash64, to check for behavior depending on specific values. Added that setting to one of the CircleCI configurations. Reviewed By: jay-zhuang Differential Revision: D24833780 Pulled By: pdillinger fbshipit-source-id: 02a57652ccf1ac105fbca79e77875bb7bf7c071f | 11 November 2020, 07:42:13 UTC |
| bcba372 | Yanqin Jin | 11 November 2020, 01:16:04 UTC | Report if unpinnable value encountered during backward iteration (#7618) Summary: There is an undocumented behavior about a certain combination of options and operations. - inplace_update_support = true, and - call `SeekForPrev()`, `SeekToLast()`, and/or `Prev()` on unflushed data. We should stop the backward iteration and report an error of `Status::NotSupported`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7618 Test Plan: make check Reviewed By: pdillinger Differential Revision: D24769619 Pulled By: riversand963 fbshipit-source-id: 81d199fa55ed4739ab10e719cc345a992238ccbb | 11 November 2020, 01:17:39 UTC |
| 18aee7d | Jay Zhuang | 10 November 2020, 22:51:38 UTC | Fix a seek issue with prefix extractor and timestamp (#7644) Summary: During seek, prefix compare should not include timestamp. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7644 Test Plan: added unittest Reviewed By: riversand963 Differential Revision: D24772066 Pulled By: jay-zhuang fbshipit-source-id: 3982655a8bf8da256a738e8497b73b3d9bdac92e | 10 November 2020, 22:53:13 UTC |
| 16d103d | Huisheng Liu | 10 November 2020, 19:13:10 UTC | 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 | 10 November 2020, 19:14:48 UTC |
| 2026051 | Akanksha Mahajan | 10 November 2020, 18:48:46 UTC | Fix crash test to run in DEBUG_LEVEL=0 mode in tmpfs (#7643) Summary: crash tests donot run in DEBUG_MODE=0 on tmpfs when use_direct_reads/use_direct_io_for_flush_and_compaction is set randomly because direct I/O is not supported on tmpfs and tests exit. Fix: Sanitize direct I/O read options in DEBUG_LEVEL=0 so that crash tests can run in tmpfs. When mmap_reads is set, direct I/O reads options are unset so we can sanitize direct I/O reads options in case of tmpfs as well. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7643 Test Plan: 1. export DEBUG_LEVEL=0; export TEST_TMPDIR="/dev/shm"; export CRASH_TEST_EXT_ARGS="--use_direct_reads=1 --mmap_read=0"; make crash_test -j64 2. In DEBUG_LEVEL=1 mode: make crash_test -j64 Reviewed By: jay-zhuang Differential Revision: D24766550 Pulled By: akankshamahajan15 fbshipit-source-id: 021720b2343c12c72004f84b26147625d3991d9e | 10 November 2020, 18:50:34 UTC |
| 9f1c84c | Yanqin Jin | 10 November 2020, 02:19:42 UTC | Fix a bug in compaction iterator with timestamp (#7645) Summary: https://github.com/facebook/rocksdb/issues/7556 introduced support for compaction iterator to perform timestamp-aware garbage collection. However, there was a bug. The comparison between `ikey_.user_key` and `current_user_key_` should happen before `key_ = current_key_.SetInternalKey(key_, &ikey_);` (line 336 of compaction_iterator.cc). Otherwise, after this line, `current_key_` is always the same as `ikey_.user_key`. This PR also re-arranged the order of some data members because some of them are state variables of `CompactionIterator` while others are inputs from callers. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7645 Test Plan: make check Reviewed By: ltamasi Differential Revision: D24845028 Pulled By: riversand963 fbshipit-source-id: c7e79914832701462b86867e8463cd463b6c0c25 | 10 November 2020, 02:23:31 UTC |
| c3911f1 | Cheng Chang | 09 November 2020, 16:42:41 UTC | Track WAL in MANIFEST: Track deleted WALs in MANIFEST after recovering from the WALs (#7649) Summary: After replaying the WALs, the memtables are flushed synchronously to L0 instead of being flushed in background. Currently, we only track WAL obsoletion events in the code path of background flush jobs. This PR tracks these events in RecoverLogFiles. After this change, we can enable `track_and_verify_wal_in_manifest` in `db_stress`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7649 Test Plan: `python tools/db_crashtest.py whitebox` Reviewed By: riversand963 Differential Revision: D24824501 Pulled By: cheng-chang fbshipit-source-id: 207129f7b845c50b333680ce6818a68a2fad54b9 | 09 November 2020, 18:25:43 UTC |
| 5e794b0 | Cheng Chang | 08 November 2020, 05:54:55 UTC | Fix a recovery corner case (#7621) Summary: Consider the following sequence of events: 1. Db flushed an SST with file number N, appended to MANIFEST, and tried to sync the MANIFEST. 2. Syncing MANIFEST failed and db crashed. 3. Db tried to recover with this MANIFEST. In the meantime, no entry about the newly-flushed SST was found in the MANIFEST. Therefore, RocksDB replayed WAL and tried to flush to an SST file reusing the same file number N. This failed because file system does not support overwrite. Then Db deleted this file. 4. Db crashed again. 5. Db tried to recover. When db read the MANIFEST, there was an entry referencing N.sst. This could happen probably because the append in step 1 finally reached the MANIFEST and became visible. Since N.sst had been deleted in step 3, recovery failed. It is possible that N.sst created in step 1 is valid. Although step 3 would still fail since the MANIFEST was not synced properly in step 1 and 2, deleting N.sst would make it impossible for the db to recover even if the remaining part of MANIFEST was appended and visible after step 5. After this PR, in step 3, immediately after recovering from MANIFEST, a new MANIFEST is created, then we find that N.sst is not referenced in the MANIFEST, so we delete it, and we'll not reuse N as file number. Then in step 5, since the new MANIFEST does not contain N.sst, the recovery failure situation in step 5 won't happen. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7621 Test Plan: 1. some tests are updated, because these tests assume that new MANIFEST is created after WAL recovery. 2. a new unit test is added in db_basic_test to simulate step 3. Reviewed By: riversand963 Differential Revision: D24668144 Pulled By: cheng-chang fbshipit-source-id: 90d7487fbad2bc3714f5ede46ea949895b15ae3b | 08 November 2020, 06:23:27 UTC |
| 8b8a2e9 | Peter Dillinger | 08 November 2020, 00:39:14 UTC | Ribbon: major re-work of hashing, seeds, and more (#7635) Summary: * Fully optimized StandardHasher, in terms of efficiently generating Start, CoeffRow, and ResultRow from a stock hash value, with sufficient independence between them to have no measurably degraded behavior. (Degraded behavior would be an FP rate higher than explainable by 2^-b and, if using a 32-bit stock hash function, expected stock hash collisions.) Details in code comments. * Our standard 64-bit and 32-bit hash functions do not exhibit sufficient independence on sequential seeds (for one Ribbon construction attempt to have independent probability from the next). I have worked around this in the Ribbon code by "pre-mixing" "ordinal seeds," sequentially tried and appropriate for storage in persisted metadata, into "raw seeds," ready for application and appropriate for in-memory storage. This way the pre-mixing step (though fast) is only applied on loading or configuring the structure, not on each query or banding add. * Fix a subtle flaw in which backtracking not clearing ResultRow data could lead to elevated FP rate on keys that were backtracked on and should (for generality) exhibit the same FP rate as novel keys. * Added a basic test for PhsfQuery and construction algorithms (map or "retrieval structure" rather than set or filter), and made a few trivial related fixes. * Better random configuration generation in unit tests * Some other minor cleanup / clarification / etc. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7635 Test Plan: unit tests included Reviewed By: jay-zhuang Differential Revision: D24738978 Pulled By: pdillinger fbshipit-source-id: f9d03599d9e2ca3e30e9d3e7d81cd936b56f76f0 | 08 November 2020, 01:22:54 UTC |
| 1e40696 | Cheng Chang | 07 November 2020, 00:30:44 UTC | Track WAL in MANIFEST: LogAndApply WAL events to MANIFEST (#7601) Summary: When a WAL is synced, an edit is written to MANIFEST. After flushing memtables, the obsoleted WALs are piggybacked to MANIFEST while writing the new L0 files to MANIFEST. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7601 Test Plan: `track_and_verify_wals_in_manifest` is enabled by default for all tests extending `DBBasicTest`, and in db_stress_test. Unit test `wal_edit_test`, `version_edit_test`, and `version_set_test` are also updated. Watch all tests to pass. Reviewed By: ltamasi Differential Revision: D24553957 Pulled By: cheng-chang fbshipit-source-id: 66a569ff1bdced38e22900bd240b73113906e040 | 07 November 2020, 01:22:36 UTC |
| 1ce105d | Cheng Chang | 06 November 2020, 22:25:32 UTC | Disable fsync in DBMergeOperatorTest to save test time (#7640) Summary: The test often times out in internal test infra. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7640 Test Plan: watch test to pass internally Reviewed By: anand1976 Differential Revision: D24764928 Pulled By: cheng-chang fbshipit-source-id: 587f2afc97f52909837943fd938a86ca94544b2c | 06 November 2020, 23:24:17 UTC |
| cdc7ba3 | Cheng Chang | 06 November 2020, 22:21:59 UTC | DBTablePropertiesTest often times out in internal test infra (#7639) Summary: In this test, after flushing memtable, it will read directly from the sst files, so `env_do_fsync` was `true` to ensure that the flushed sst files can be read afterwards. Considering that the test does not last long, the data should be available in os buffer even without fsync, so this PR tries to disable fsync to reduce test time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7639 Test Plan: watch the test to pass in internal infra Reviewed By: anand1976 Differential Revision: D24764689 Pulled By: cheng-chang fbshipit-source-id: ef827611a3eaca04201e4280ae801d6c8e60c138 | 06 November 2020, 22:25:14 UTC |
| da42ece | Cheng Chang | 06 November 2020, 22:16:07 UTC | Skip fsync in txn tests (#7641) Summary: The tests often times out in internal infra, skipping fsync should reduce test time. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7641 Test Plan: watch existing tests to pass Reviewed By: anand1976 Differential Revision: D24765098 Pulled By: cheng-chang fbshipit-source-id: c62bf8110361aee901918d632cf4772435d05e8d | 06 November 2020, 22:25:14 UTC |
| 4c2aef0 | Cheng Chang | 06 November 2020, 17:57:09 UTC | ColumnFamilyTest often times out in internal test infra (#7638) Summary: Tries to fix by skipping fsync. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7638 Test Plan: watch the tests to pass Reviewed By: jay-zhuang Differential Revision: D24764355 Pulled By: cheng-chang fbshipit-source-id: 9c21b177709025ca1943066d94da89324ed47655 | 06 November 2020, 18:25:20 UTC |
| 8154336 | Cheng Chang | 06 November 2020, 17:38:02 UTC | Disable fsync in db_range_del_test (#7637) Summary: This test often times out in internal test infra. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7637 Test Plan: watch test to pass Reviewed By: ajkr Differential Revision: D24763939 Pulled By: cheng-chang fbshipit-source-id: 6564ee2ef637e9faf6688d4b6a5d74a72a51c5e8 | 06 November 2020, 18:25:20 UTC |
| 1f62721 | cheng-chang | 05 November 2020, 00:47:42 UTC | Simplify a test case in Java ReadOnlyTest (#7608) Summary: The original test nests a lot of `try` blocks. This PR flattens these blocks into independent blocks, so that each `try` block closes the DB before opening the next DB instance. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7608 Test Plan: watch the existing java tests to pass Reviewed By: zhichao-cao Differential Revision: D24611621 Pulled By: cheng-chang fbshipit-source-id: d486c5d37ac25d4b860d739ef2cdd58e6064d42d | 05 November 2020, 00:49:17 UTC |
| c9c9709 | Xie Yanbo | 05 November 2020, 00:07:39 UTC | Update clang-format-diff.py (#7609) Summary: `llvm-mirror/clang` is archived. Get the `clang-format-diff.py` file from the active source. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7609 Reviewed By: ajkr Differential Revision: D24711608 Pulled By: pdillinger fbshipit-source-id: b115d8765ff23fbb8190290a170de21565daba84 | 05 November 2020, 00:09:01 UTC |
| b6d8e36 | Yanqin Jin | 04 November 2020, 18:43:17 UTC | 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 | 04 November 2020, 18:44:56 UTC |
| fde0cd7 | Yanqin Jin | 04 November 2020, 04:33:45 UTC | 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 | 04 November 2020, 04:34:56 UTC |
| 06a92fc | Akanksha Mahajan | 03 November 2020, 21:54:01 UTC | Add "max_write_buffer_size_to_maintain" to crash test (#7634) Summary: Add "max_write_buffer_size_to_maintain" to crash test Pull Request resolved: https://github.com/facebook/rocksdb/pull/7634 Test Plan: make crash_test -j64 Reviewed By: zhichao-cao Differential Revision: D24710401 Pulled By: akankshamahajan15 fbshipit-source-id: 89e0412aaa56b2ef5a75603971b82f4b0b494ab7 | 03 November 2020, 21:55:18 UTC |
| 746909c | Peter Dillinger | 03 November 2020, 20:45:18 UTC | Ribbon: InterleavedSolutionStorage (#7598) Summary: The core algorithms for InterleavedSolutionStorage and the implementation SerializableInterleavedSolution make Ribbon fast for filter queries. Example output from new unit test: Simple outside query, hot, incl hashing, ns/key: 117.796 Interleaved outside query, hot, incl hashing, ns/key: 42.2655 Bloom outside query, hot, incl hashing, ns/key: 24.0071 Also includes misc cleanup of previous Ribbon code and comments. Some TODOs and FIXMEs remain for futher work / investigation. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7598 Test Plan: unit tests included (integration work and tests coming later) Reviewed By: jay-zhuang Differential Revision: D24559209 Pulled By: pdillinger fbshipit-source-id: fea483cd354ba782aea3e806f2bc96e183d59441 | 03 November 2020, 20:46:36 UTC |
| 0b94468 | Yanqin Jin | 03 November 2020, 17:47:06 UTC | Avoid skipping a test in db_wal_test (#7628) Summary: Recent test report shows that some tests have been skipped. For DBWALTest that inherits from DBTestBase, the following will always be true, since `env_` is an instance of `SpecialEnv`, not `Env::Default()`. Thus the test will always be skipped. ``` if (options.env != Env::Default()) { ROCKSDB_GTEST_SKIP("Test requires default environment"); return; } ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7628 Test Plan: ./db_wal_test --gtest_filter=DBWALTest.TruncateLastLogAfterRecoverWithoutFlush MEM_ENV=1 ./db_wal_test --gtest_filter=DBWALTest.TruncateLastLogAfterRecoverWithoutFlush make check Reviewed By: jay-zhuang Differential Revision: D24693006 Pulled By: riversand963 fbshipit-source-id: 7f2a772492a0f11bff17bbf5e9f493e9e9a1c125 | 03 November 2020, 17:48:16 UTC |
| 881e0dc | Jay Zhuang | 03 November 2020, 17:44:21 UTC | Fix MultiGet unable to query timestamp data issue (#7589) Summary: The filter query key should not contain timestamp. The timestamp is stripped for Get(), but not MultiGet(). Pull Request resolved: https://github.com/facebook/rocksdb/pull/7589 Reviewed By: riversand963 Differential Revision: D24494661 Pulled By: jay-zhuang fbshipit-source-id: fc5ff40f9d683a89a760c6ff0ab3aed05a70c317 | 03 November 2020, 17:45:41 UTC |
| c992eb1 | Yanqin Jin | 03 November 2020, 03:47:00 UTC | Avoid skipping a test in db_test2 (#7629) Summary: Test report shows that this test has been skipped recently due to a condition that will never meet. `env_` is not equal to `Env::Default()` for DBTest2 that inherits from DBTestBase. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7629 Test Plan: make check ./db_test2 --gtest_filter=DBTest2.PinnableSliceAndMmapReads Reviewed By: jay-zhuang Differential Revision: D24693317 Pulled By: riversand963 fbshipit-source-id: b1bbd5c1e05a6fa57c1de0d74462b69e3c2d5215 | 03 November 2020, 03:48:23 UTC |
| 1adbceb | Andrew Kryczka | 03 November 2020, 03:20:15 UTC | Expand effect of dictionary settings in `ColumnFamilyOptions::compression_opts` (#7619) Summary: In dictionary compression's initial implementation, in order to save CPU overhead, we only enabled it for bottom level under the assumption that the vast majority of data is stored there. At that time, there was no such thing as `ColumnFamilyOptions::bottommost_compression_opts`, so we just hardcoded disabling dictionary compression in flush and compactions to non-bottommost level. Now, we have users who generate all their files through flush and are considering using dictionary compression. To support such a use case, this PR expands the scope of `ColumnFamilyOptions::compression_opts` to additionally include flushed files and files generated by compaction to a non-bottommost level. Users can still get the old behavior by moving their dictionary settings to `ColumnFamilyOptions::bottommost_compression_opts` and explicitly enabling both that and `ColumnFamilyOptions::bottommost_compression`. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7619 Reviewed By: ltamasi Differential Revision: D24665610 Pulled By: ajkr fbshipit-source-id: 656b90bce1033fe21c71e09af931ef5bde3e464c | 03 November 2020, 03:21:11 UTC |
| a388c8c | Andrew Kryczka | 30 October 2020, 21:01:52 UTC | Add recent fixes to HISTORY.md (#7617) Summary: The recently reverted behavior changes were released to at least one place internally, so we should mention the reverts in release notes. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7617 Reviewed By: akankshamahajan15 Differential Revision: D24654343 Pulled By: ajkr fbshipit-source-id: eb64b2797d8508cd95a2dc2698122c1be29ce817 | 30 October 2020, 21:03:35 UTC |
| 30beece | mrambacher | 30 October 2020, 01:43:03 UTC | 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, 01:44:24 UTC |
| 6773901 | Akanksha Mahajan | 29 October 2020, 22:50:27 UTC | Add 6.14 branch to check_format_compatible.sh (#7613) Summary: Add 6.14 to check_format_compatible.sh Pull Request resolved: https://github.com/facebook/rocksdb/pull/7613 Test Plan: ./tools/check_format_compatible.sh Reviewed By: riversand963 Differential Revision: D24628535 Pulled By: akankshamahajan15 fbshipit-source-id: a8bf1d5505a1fcc8a5bedc5ff4fdf33a22c3f2e6 | 29 October 2020, 22:51:50 UTC |
| 7eb2824 | mrambacher | 29 October 2020, 20:45:17 UTC | 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 | 29 October 2020, 20:46:26 UTC |
| 394210f | Yanqin Jin | 29 October 2020, 06:20:23 UTC | Remove unused includes (#7604) Summary: This is a PR generated **semi-automatically** by an internal tool to remove unused includes and `using` statements. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7604 Test Plan: make check Reviewed By: ajkr Differential Revision: D24579392 Pulled By: riversand963 fbshipit-source-id: c4bfa6c6b08da1de186690d37eb73d8fff45aecd | 29 October 2020, 06:22:27 UTC |
| 99a0305 | Jermy Li | 29 October 2020, 01:11:52 UTC | java: correct method name RocksDB.GetColumnFamilyMetaData() (#7606) Summary: update GetColumnFamilyMetaData() to getColumnFamilyMetaData() Pull Request resolved: https://github.com/facebook/rocksdb/pull/7606 Reviewed By: zhichao-cao Differential Revision: D24610298 Pulled By: cheng-chang fbshipit-source-id: d24f9b65478da1456f50747637dc95688af874de | 29 October 2020, 01:13:27 UTC |
| ea347d8 | Zhichao Cao | 28 October 2020, 23:46:04 UTC | 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 | 28 October 2020, 23:47:12 UTC |
| 2404f8b | jsteemann | 28 October 2020, 20:45:21 UTC | slightly improve jemalloc allocator API header (#7592) Summary: Fix a few typos and avoid a potential nullptr dereference. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7592 Reviewed By: zhichao-cao Differential Revision: D24582111 Pulled By: riversand963 fbshipit-source-id: 51e9260e8cad1fcdedd310c889f0faeec6efd937 | 28 October 2020, 20:47:12 UTC |
| 248d10f | vdimir | 28 October 2020, 18:09:52 UTC | Fix typo in arena.cc (#7593) Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/7593 Reviewed By: zhichao-cao Differential Revision: D24576218 Pulled By: riversand963 fbshipit-source-id: a3d77191362ca696ae9df643f97f4ab5b7ecff12 | 28 October 2020, 18:11:17 UTC |
| 793e9b7 | darionyaphet | 28 October 2020, 17:47:18 UTC | Remove duplicate close (#7594) Summary: Because `Close()` have called in `Destroy()` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7594 Reviewed By: zhichao-cao Differential Revision: D24576407 Pulled By: riversand963 fbshipit-source-id: eba70d73375fd47dd78ca64c6a1fab3628448276 | 28 October 2020, 17:48:53 UTC |
| 9a690a7 | Ramkumar Vadivelu | 28 October 2020, 17:11:13 UTC | In ParseInternalKey(), include corrupt key info in Status (#7515) Summary: Fixes Issue https://github.com/facebook/rocksdb/issues/7497 When allow_data_in_errors db_options is set, log error key details in `ParseInternalKey()` Have fixed most of the calls. Have few TODOs still pending - because have to make more deeper changes to pass in the allow_data_in_errors flag. Will do those in a separate PR later. Tests: - make check - some of the existing tests that exercise the "internal key too small" condition are: dbformat_test, cuckoo_table_builder_test - some of the existing tests that exercise the corrupted key path are: corruption_test, merge_helper_test, compaction_iterator_test Example of new status returns: - Key too small - `Corrupted Key: Internal Key too small. Size=5` - Corrupt key with allow_data_in_errors option set to false: `Corrupted Key: '<redacted>' seq:3, type:3` - Corrupt key with allow_data_in_errors option set to true: `Corrupted Key: '61' seq:3, type:3` Pull Request resolved: https://github.com/facebook/rocksdb/pull/7515 Reviewed By: ajkr Differential Revision: D24240264 Pulled By: ramvadiv fbshipit-source-id: bc48f5d4475ac19d7713e16df37505b31aac42e7 | 28 October 2020, 17:12:58 UTC |
| 6c2c063 | Andrew Kryczka | 28 October 2020, 16:59:12 UTC | Require only one `Logger::Logv()` implementation (#7605) Summary: A user who extended `Logger` recently pointed out it is unusual to require they implement the two-argument `Logv()` overload when they've already implemented the three-argument `Logv()` overload. I agree with that and think we can fix it by only calling the two-argument overload from the default implementation of the three-argument overload. Then when the three-argument overload is overridden, RocksDB would not rely on the two-argument overload. Only `Logger::LogHeader()` needed adjustment to achieve this. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7605 Reviewed By: riversand963 Differential Revision: D24584749 Pulled By: ajkr fbshipit-source-id: 9aabe040ac761c4c0dbebc4be046967403ecaf21 | 28 October 2020, 17:00:51 UTC |
| 0e2e675 | Peter Dillinger | 27 October 2020, 18:56:51 UTC | Give instructions instead of broken 2to3 for clang-format-diff.py (#7603) Summary: My previous change to use lib2to3 to migrate clang-format-diff.py for Python 2 only works if there's nothing to reformat. Instead, give instructions to download to REPO_ROOT. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7603 Test Plan: Try the instructions on a fresh CentOS 8 devserver Reviewed By: riversand963 Differential Revision: D24569608 Pulled By: pdillinger fbshipit-source-id: 1410ba163e016b226e883dec93fae3df9ed0eab2 | 27 October 2020, 18:58:17 UTC |
| f35f7f2 | mrambacher | 27 October 2020, 17:31:34 UTC | Fix many tests to run with MEM_ENV and ENCRYPTED_ENV; Introduce a MemoryFileSystem class (#7566) Summary: This PR does a few things: 1. The MockFileSystem class was split out from the MockEnv. This change would theoretically allow a MockFileSystem to be used by other Environments as well (if we created a means of constructing one). The MockFileSystem implements a FileSystem in its entirety and does not rely on any Wrapper implementation. 2. Make the RocksDB test suite work when MOCK_ENV=1 and ENCRYPTED_ENV=1 are set. To accomplish this, a few things were needed: - The tests that tried to use the "wrong" environment (Env::Default() instead of env_) were updated - The MockFileSystem was changed to support the features it was missing or mishandled (such as recursively deleting files in a directory or supporting renaming of a directory). 3. Updated the test framework to have a ROCKSDB_GTEST_SKIP macro. This can be used to flag tests that are skipped. Currently, this defaults to doing nothing (marks the test as SUCCESS) but will mark the tests as SKIPPED when RocksDB is upgraded to a version of gtest that supports this (gtest-1.10). I have run a full "make check" with MEM_ENV, ENCRYPTED_ENV, both, and neither under both MacOS and RedHat. A few tests were disabled/skipped for the MEM/ENCRYPTED cases. The error_handler_fs_test fails/hangs for MEM_ENV (presumably a timing problem) and I will introduce another PR/issue to track that problem. (I will also push a change to disable those tests soon). There is one more test in DBTest2 that also fails which I need to investigate or skip before this PR is merged. Theoretically, this PR should also allow the test suite to run against an Env loaded from the registry, though I do not have one to try it with currently. Finally, once this is accepted, it would be nice if there was a CircleCI job to run these tests on a checkin so this effort does not become stale. I do not know how to do that, so if someone could write that job, it would be appreciated :) Pull Request resolved: https://github.com/facebook/rocksdb/pull/7566 Reviewed By: zhichao-cao Differential Revision: D24408980 Pulled By: jay-zhuang fbshipit-source-id: 911b1554a4d0da06fd51feca0c090a4abdcb4a5f | 27 October 2020, 17:33:09 UTC |
| 6134ce6 | Yanqin Jin | 27 October 2020, 01:20:43 UTC | Perform post-flush updates of memtable list in a callback (#6069) Summary: Currently, the following interleaving of events can lead to SuperVersion containing both immutable memtables as well as the resulting L0. This can cause Get to return incorrect result if there are merge operands. This may also affect other operations such as single deletes. ``` time main_thr bg_flush_thr bg_compact_thr compact_thr set_opts_thr 0 | WriteManifest:0 1 | issue compact 2 | wait 3 | Merge(counter) 4 | issue flush 5 | wait 6 | WriteManifest:1 7 | wake up 8 | write manifest 9 | wake up 10 | Get(counter) 11 | remove imm V ``` The reason behind is that: one bg flush thread's installing new `Version` can be batched and performed by another thread that is the "leader" MANIFEST writer. This bg thread removes the memtables from current super version only after `LogAndApply` returns. After the leader MANIFEST writer signals (releasing mutex) this bg flush thread, it is possible that another thread sees this cf with both memtables (whose data have been flushed to the newest L0) and the L0 before this bg flush thread removes the memtables. To address this issue, each bg flush thread can pass a callback function to `LogAndApply`. The callback is responsible for removing the memtables. Therefore, the leader MANIFEST writer can call this callback and remove the memtables before releasing the mutex. Test plan (devserver) ``` $make merge_test $./merge_test --gtest_filter=MergeTest.MergeWithCompactionAndFlush $make check ``` Pull Request resolved: https://github.com/facebook/rocksdb/pull/6069 Reviewed By: cheng-chang Differential Revision: D18790894 Pulled By: riversand963 fbshipit-source-id: e41bd600c0448b4f4b2deb3f7677f95e3076b4ed | 27 October 2020, 01:23:01 UTC |
| a7a04b6 | Levi Tamasi | 26 October 2020, 20:50:03 UTC | Integrate BlobFileBuilder into the compaction process (#7573) Summary: Similarly to how https://github.com/facebook/rocksdb/issues/7345 integrated blob file writing into the flush process, the patch adds support for writing blob files to the compaction logic. Namely, if `enable_blob_files` is set, large values encountered during compaction are extracted to blob files and replaced with blob indexes. The resulting blob files are then logged to the MANIFEST as part of the compaction job's `VersionEdit` and added to the `Version` alongside any table files written by the compaction. Any errors during blob file building fail the compaction job. There will be a separate follow-up patch to perform blob garbage collection during compactions. In addition, the patch continues to chip away at the mess around computing various compaction related statistics by eliminating some code duplication and by making the `num_output_files` and `bytes_written` stats more consistent for flushes, compactions, and recovery. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7573 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D24404696 Pulled By: ltamasi fbshipit-source-id: 21216af3a172ad3ce8f85d11cd30923784ae426c | 26 October 2020, 20:51:55 UTC |
| 25d54c7 | Peter Dillinger | 26 October 2020, 03:43:04 UTC | Ribbon: initial (general) algorithms and basic unit test (#7491) Summary: This is intended as the first commit toward a near-optimal alternative to static Bloom filters for SSTs. Stephan Walzer and I have agreed upon the name "Ribbon" for a PHSF based on his linear system construction in "Efficient Gauss Elimination for Near-Quadratic Matrices with One Short Random Block per Row, with Applications" ("SGauss") and my much faster "on the fly" algorithm for gaussian elimination (or for this linear system, "banding"), which can be faster than peeling while also more compact and flexible. See util/ribbon_alg.h for more detailed introduction and background. RIBBON = Rapid Incremental Boolean Banding ON-the-fly This commit just adds generic (templatized) core algorithms and a basic unit test showing some features, including the ability to construct structures within 2.5% space overhead vs. information theoretic lower bound. (Compare to cache-local Bloom filter's ~50% space overhead -> ~30% reduction anticipated.) This commit does not include the storage scheme necessary to make queries fast, especially for filter queries, nor fractional "result bits", but there is some description already and those implementations will come soon. Nor does this commit add FilterPolicy support, for use in SST files, but that will also come soon. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7491 Reviewed By: jay-zhuang Differential Revision: D24517954 Pulled By: pdillinger fbshipit-source-id: 0119ee597e250d7e0edd38ada2ba50d755606fa7 | 26 October 2020, 03:44:49 UTC |
| 6595267 | Yanqin Jin | 24 October 2020, 05:58:05 UTC | Allow compaction iterator to perform garbage collection (#7556) Summary: Add a threshold timestamp, full_history_ts_low_ of type `std::string*` to `CompactionIterator`, so that RocksDB can also perform garbage collection during compaction. * If full_history_ts_low_ is nullptr, then compaction iterator does not perform GC, preserving all timestamp history for all keys. Compaction iterator will treat user key with different timestamps as different user keys. * If full_history_ts_low_ is not nullptr, then compaction iterator performs GC. GC will look at keys older than `*full_history_ts_low_` and determine their eligibility based on factors including snapshots. Current rules of GC: * If an internal key is in the same snapshot as a previous counterpart with the same user key, and this key is eligible for GC, and the key is not single-delete or merge operand, then this key can be dropped. Note that the previous internal key cannot be a merge operand either. * If a tombstone is the most recent one in the earliest snapshot and it is eligible for GC, and keyNotExistsBeyondLevel() is true, then this tombstone can be dropped. * If a tombstone is the most recent one in a snapshot and it is eligible for GC, and the compaction is at bottommost level, then all other older internal keys of the same user key must also be eligible for GC, thus can be dropped * Single-delete, delete-range and merge are not currently supported. Pull Request resolved: https://github.com/facebook/rocksdb/pull/7556 Test Plan: make check Reviewed By: ltamasi Differential Revision: D24507728 Pulled By: riversand963 fbshipit-source-id: 3c09c7301f41eed76dfcf4d1527e68cf6e0a8bb3 | 24 October 2020, 05:59:46 UTC |
