1. 09 May, 2020 2 commits
    • Yanqin Jin's avatar
      Fix a few bugs in best-efforts recovery (#6824) · e72e2167
      Yanqin Jin authored
      Summary:
      1. Update column_family_memtables_ to point to latest column_family_set in
         version_set after recovery.
      2. Normalize file paths passed by application so that directories end with '/'
         or '\\'.
      3. In addition to missing files, corrupted files are also ignored in
         best-efforts recovery.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6824
      
      Test Plan: COMPILE_WITH_ASAN=1 make check
      
      Reviewed By: anand1976
      
      Differential Revision: D21463905
      
      Pulled By: riversand963
      
      fbshipit-source-id: c48db8843cc93c8c1c7139c474b64e6f775307d2
      e72e2167
    • Andrew Kryczka's avatar
      prototype status check enforcement (#6798) · 1c846604
      Andrew Kryczka authored
      Summary:
      Tried making Status object enforce that it is checked in some way. In cases it is not checked, `PermitUncheckedError()` must be called explicitly.
      
      Added a way to run tests (`ASSERT_STATUS_CHECKED=1 make -j48 check`) on a
      whitelist. The effort appears significant to get each test to pass with
      this assertion, so I only fixed up enough to get one test (`options_test`)
      working and added it to the whitelist.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6798
      
      Reviewed By: pdillinger
      
      Differential Revision: D21377404
      
      Pulled By: ajkr
      
      fbshipit-source-id: 73236f9c8df38f01cf24ecac4a6d1661b72d077e
      1c846604
  2. 08 May, 2020 7 commits
    • sdong's avatar
      Disable "compression_parallel_threads" in crash test for now (#6816) · 12825894
      sdong authored
      Summary:
      "compressio_parallel_threads" caused several test failure tests. To keep crash test clean, disable it for now.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6816
      
      Test Plan: "make crash_test" to make sure the python script doesn't break
      
      Reviewed By: zhichao-cao
      
      Differential Revision: D21462112
      
      fbshipit-source-id: 9eecc764800da82cd19665dc8b167eacead3310b
      12825894
    • anand76's avatar
      Fix race due to delete triggered compaction in Universal compaction mode (#6799) · 94265234
      anand76 authored
      Summary:
      Delete triggered compaction in universal compaction mode was causing a corruption when scheduled in parallel with other compactions.
      1. When num_levels = 1, a file marked for compaction may be picked along with all older files in L0, without checking if any of them are already being compaction. This can cause unpredictable results like resurrection of older versions of keys or deleted keys.
      2. When num_levels > 1, a delete triggered compaction would not get scheduled if it overlaps with a running regular compaction. However, the reverse is not true. This is due to the fact that in ```UniversalCompactionBuilder::CalculateSortedRuns```, it assumes that entire sorted runs are picked for compaction and only checks the first file in a sorted run to determine conflicts. This is violated by a delete triggered compaction as it works on a subset of a sorted run.
      
      Fix the bug for num_levels > 1, and disable the feature for now when num_levels = 1. After disabling this feature, files would still get marked for compaction, but no compaction would get scheduled.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6799
      
      Reviewed By: pdillinger
      
      Differential Revision: D21431286
      
      Pulled By: anand1976
      
      fbshipit-source-id: ae9f0bdb1d6ae2f10284847db731c23f43af164a
      94265234
    • Andrew Kryczka's avatar
      Fixup HISTORY.md for e9ba4ba3 "validate range tombstone covers positiv… (#6825) · 3730b05d
      Andrew Kryczka authored
      Summary:
      …e range"
      
      Moved it from the wrong section (6.10) to the right section (Unreleased).
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6825
      
      Reviewed By: zhichao-cao
      
      Differential Revision: D21464577
      
      Pulled By: ajkr
      
      fbshipit-source-id: a836b4ab10be2464182826f9411c9c424c933b70
      3730b05d
    • anand76's avatar
      Include options.h in table.h (#6823) · f286fb34
      anand76 authored
      Summary:
      https://github.com/facebook/rocksdb/issues/6389 replaced the #include of options.h in table.h with forward declarations, which is causing some build failures in RocksDB users in 6.10. Remove the forward declarations and #include options.h as recommended by the style guide - https://google.github.io/styleguide/cppguide.html#Forward_Declarations
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6823
      
      Reviewed By: riversand963
      
      Differential Revision: D21464078
      
      Pulled By: anand1976
      
      fbshipit-source-id: 6033ee2544d279690f57bb0db91bc83816cee11d
      f286fb34
    • Peter Dillinger's avatar
      Fix false NotFound from batched MultiGet with kHashSearch (#6821) · b27a1448
      Peter Dillinger authored
      Summary:
      The error is assigning KeyContext::s to NotFound status in a
      table reader for a "not found in this table" case, which skips searching
      in later tables, like only a delete should. (The hash search index iterator
      is the only one that can return status NotFound even if Valid() == false.)
      
      This was detected by intermittent failure in
      MultiThreadedDBTest.MultiThreaded/5, a kHashSearch configuration.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6821
      
      Test Plan: modified existing unit test to reproduce problem
      
      Reviewed By: anand1976
      
      Differential Revision: D21450469
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 7478003684d637dbd491cdac81468041a791be2c
      b27a1448
    • Andrew Kryczka's avatar
      validate range tombstone covers positive range (#6788) · e9ba4ba3
      Andrew Kryczka authored
      Summary:
      We found some files containing nothing but negative range tombstones,
      and unsurprisingly their metadata specified a negative range, which made
      things crash. Time to add a bit of user input validation.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6788
      
      Reviewed By: zhichao-cao
      
      Differential Revision: D21343719
      
      Pulled By: ajkr
      
      fbshipit-source-id: f1c16e4c3e9fa150958c8c866176632a3206fb74
      e9ba4ba3
    • Levi Tamasi's avatar
      Find/purge obsolete blob files (#6807) · ac3ae1df
      Levi Tamasi authored
      Summary:
      The patch extends `FindObsoleteFiles` and `PurgeObsoleteFiles` with
      support for blob files. The behavior is analogous to SST files: obsolete
      blob files are put on the "candidates for deletion" list, while live (and pending)
      files are preserved.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6807
      
      Test Plan: `make check`
      
      Reviewed By: riversand963
      
      Differential Revision: D21406249
      
      Pulled By: ltamasi
      
      fbshipit-source-id: 1948f71c31927564b61e8af394f50ca3964880d9
      ac3ae1df
  3. 07 May, 2020 4 commits
  4. 06 May, 2020 6 commits
    • sdong's avatar
      Slightly expand converage to file consistency check failure (#6800) · c21c4597
      sdong authored
      Summary:
      Current DBCompactionTest.ConsistencyFailTest checks DB fails after L0 inconsitency is found. Add slightly more coverage by introducing DBCompactionTest.ConsistencyFailTest2 which checks non-L0 files too.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6800
      
      Test Plan: Run the new test.
      
      Reviewed By: riversand963
      
      Differential Revision: D21384806
      
      fbshipit-source-id: 36db7b657eed42115283fe2f6afa4c3a31a3b510
      c21c4597
    • mrambacher's avatar
      Add OptionTypeInfo::Enum and related methods (#6423) · 394f2bbd
      mrambacher authored
      Summary:
      Add methods and constructors for handling enums to the OptionTypeInfo.  This change allows enums to be converted/compared without adding a special "type" to the OptionType.
      
      This change addresses a couple of issues:
      - It allows new enumerated types to be added to the options without editing the OptionType base class (and related methods)
      - It standardizes the procedure for adding enumerated types to the options, reducing potential mistakes
      - It moves the enum maps to the location where they are used, allowing them to be static file members rather than global values
      - It reduces the number of types and cases that need to be handled in the various OptionType methods
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6423
      
      Reviewed By: siying
      
      Differential Revision: D21408713
      
      Pulled By: zhichao-cao
      
      fbshipit-source-id: fc492af285d011822578b95d186a0fce25d35626
      394f2bbd
    • Andrew Kryczka's avatar
      fix swallowed error for file deletion consistency check (#6809) · a96461d1
      Andrew Kryczka authored
      Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6809
      
      Reviewed By: pdillinger
      
      Differential Revision: D21411971
      
      Pulled By: ajkr
      
      fbshipit-source-id: 900b6b0370b76e9a3e5e03f968e2ac1bbaab73b8
      a96461d1
    • Peter Dillinger's avatar
      Fix failure to write output in SpecialEnv::GetCurrentTime (#6803) · 2f1700c8
      Peter Dillinger authored
      Summary:
      This very old test code bug was causing a new valgrind failure
      in MultiGetDeadlineExceeded
      
      Also fix hang in MultiGetDeadlineExceeded by unifying with some logic from another test.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6803
      
      Test Plan: run that unit test under valgrind, make check
      
      Reviewed By: siying
      
      Differential Revision: D21388470
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 0ce99d6d5eb8cd3195b17406892c8c5cff5fa5dd
      2f1700c8
    • Cheng Chang's avatar
      Refactor level compaction picker (#6804) · 91bc0130
      Cheng Chang authored
      Summary:
      1. refactor out PickFileToCompact to remove duplicated logic.
      2. remove redundant checks of `start_level_inputs_.empty()`.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6804
      
      Test Plan: make check
      
      Reviewed By: siying
      
      Differential Revision: D21390053
      
      Pulled By: cheng-chang
      
      fbshipit-source-id: 185d5987a08bfdaf63f0f245310c6da69878d415
      91bc0130
    • Yanqin Jin's avatar
      Do not swallow error returned from SaveTo() (#6801) · 5584595f
      Yanqin Jin authored
      Summary:
      With consistency check enabled, VersionBuilder::SaveTo() may return error once
      corruption is detected while building versions. We should handle these errors.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6801
      
      Test Plan: make check
      
      Reviewed By: siying
      
      Differential Revision: D21385045
      
      Pulled By: riversand963
      
      fbshipit-source-id: 98f6424e2a4699b62befa21e9fe00e70a771118e
      5584595f
  5. 05 May, 2020 4 commits
    • Yanqin Jin's avatar
      Fix db_stress when GetLiveFiles() flushes dropped CF (#6805) · 5a61e786
      Yanqin Jin authored
      Summary:
      Current impl. of db_stress will abort verification and report failure if
      GetLiveFiles() causes a dropped column family to be flushed. This is not
      desired.
      To fix, this PR makes the following change:
      In GetLiveFiles, if flush is triggered and returns
      Status::IsColumnFamilyDropped(), then set status to Status::OK().
      This is OK because dropped column families will be skipped during the rest of
      this function, and valid column families will have their live files returned to
      caller.
      
      Test plan (dev server):
      make check
      ./db_stress -ops_per_thread=1000 -get_live_files_one_in=100 -clear_column_family_one_in=100
      ./db_stress -disable_wal=1 -reopen=0 -ops_per_thread=1000 -get_live_files_one_in=100 -clear_column_family_one_in=100
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6805
      
      Reviewed By: ltamasi
      
      Differential Revision: D21390044
      
      Pulled By: riversand963
      
      fbshipit-source-id: de67846b95a4f1b88aa0a30c3d70c43cc68625b9
      5a61e786
    • Levi Tamasi's avatar
      Expose the set of live blob files from Version/VersionSet (#6785) · a00ddf15
      Levi Tamasi authored
      Summary:
      The patch adds logic that returns the set of live blob files from
      `Version::AddLiveFiles` and `VersionSet::AddLiveFiles` (in addition to
      live table files), and also cleans up the code a bit, for example, by
      exposing only the numbers of table files as opposed to the earlier
      `FileDescriptor`s that no clients used. Moreover, the patch extends
      the `GetLiveFiles` API so that it also exposes blob files in the current version.
      Similarly to https://github.com/facebook/rocksdb/pull/6755,
      this is a building block for identifying and purging obsolete blob files.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6785
      
      Test Plan: `make check`
      
      Reviewed By: riversand963
      
      Differential Revision: D21336210
      
      Pulled By: ltamasi
      
      fbshipit-source-id: fc1aede8a49eacd03caafbc5f6f9ce43b6270821
      a00ddf15
    • sdong's avatar
      Avoid Swallowing Some File Consistency Checking Bugs (#6793) · 680c4163
      sdong authored
      Summary:
      We are swallowing some file consistency checking failures. This is not expected. We are fixing two cases: DB reopen and manifest dump.
      More places are not fixed and need follow-up.
      
      Error from CheckConsistencyForDeletes() is also swallowed, which is not fixed in this PR.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6793
      
      Test Plan: Add a unit test to cover the reopen case.
      
      Reviewed By: riversand963
      
      Differential Revision: D21366525
      
      fbshipit-source-id: eb438a322237814e8d5125f916a3c6de97f39ded
      680c4163
    • Mian Qin's avatar
      Fix issues for reproducing synthetic ZippyDB workloads in the FAST20' paper (#6795) · d9e170d8
      Mian Qin authored
      Summary:
      Fix issues for reproducing synthetic ZippyDB workloads in the FAST20' paper using db_bench. Details changes as follows.
      1, add a separate random mode in MixGraph to produce all_random workload.
      2, fix power inverse function for generating prefix_dist workload.
      3, make sure key_offset in prefix mode is always unsigned.
      note: Need to carefully choose key_dist_a/b to avoid aliasing. Power inverse function range should be close to overall key space.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6795
      
      Reviewed By: akankshamahajan15
      
      Differential Revision: D21371095
      
      Pulled By: zhichao-cao
      
      fbshipit-source-id: 80744381e242392c8c7cf8ac3d68fe67fe876048
      d9e170d8
  6. 02 May, 2020 1 commit
  7. 01 May, 2020 11 commits
    • Zhichao Cao's avatar
      Fix multiple CF replay failure in db_bench replay (#6787) · c8643edf
      Zhichao Cao authored
      Summary:
      The multiple CF hash map is not passed to the multi-thread worker. When using multi-thread replay for multiple CFs, it will cause segment fault. Pass the cf_map to the argument.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6787
      
      Test Plan: pass trace replay test.
      
      Reviewed By: yhchiang
      
      Differential Revision: D21339941
      
      Pulled By: zhichao-cao
      
      fbshipit-source-id: 434482b492287e6722c7cd5a706f057c5ec170ce
      c8643edf
    • Yanqin Jin's avatar
      Add Github Action for some basic sanity test of PR (#6761) · 6acbbbf9
      Yanqin Jin authored
      Summary:
      Add Github Action to perform some basic sanity check for PR, inclding the
      following.
      1) Buck TARGETS file.
      On the one hand, The TARGETS file is used for internal buck, and we do not
      manually update it. On the other hand, we need to run the buckifier scripts to
      update TARGETS whenever new files are added, etc. With this Github Action, we
      make sure that every PR does not forget this step. The GH Action uses
      a Makefile target called check-buck-targets. Users can manually run `make
      check-buck-targets` on local machine.
      
      2) Code format
      We use clang-format-diff.py to format our code. The GH Action in this PR makes
      sure this step is not skipped. The checking script build_tools/format-diff.sh assumes that `clang-format-diff.py` is executable.
      On host running GH Action, it is difficult to download `clang-format-diff.py` and make it
      executable. Therefore, we modified build_tools/format-diff.sh to handle the case in which there is a non-executable clang-format-diff.py file in the top-level rocksdb repo directory.
      
      Test Plan (Github and devserver):
      Watch for Github Action result in the `Checks` tab.
      On dev server
      ```
      make check-format
      make check-buck-targets
      make check
      ```
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6761
      
      Test Plan: Watch for Github Action result in the `Checks` tab.
      
      Reviewed By: pdillinger
      
      Differential Revision: D21260209
      
      Pulled By: riversand963
      
      fbshipit-source-id: c646e2f37c6faf9f0614b68aa0efc818cff96787
      6acbbbf9
    • sdong's avatar
      Remove the support of setting CompressionOptions.parallel_threads from string for now (#6782) · 6504ae0c
      sdong authored
      Summary:
      The current way of implementing CompressionOptions.parallel_threads introduces a format change. We plan to change CompressionOptions's serailization format to a new JSON-like format, which would be another format change. We would like to consolidate the two format changes into one, rather than making some users to change twice. Hold CompressionOptions.parallel_threads from being supported by option string for now. Will add it back after the general CompressionOptions's format change.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6782
      
      Test Plan: Run all existing tests.
      
      Reviewed By: zhichao-cao
      
      Differential Revision: D21338614
      
      fbshipit-source-id: bca2dac3cb37d4e6e64b52cbbe8ea749cd848685
      6504ae0c
    • Cheng Chang's avatar
      Make users explicitly be aware of prepare before commit (#6775) · ef0c3eda
      Cheng Chang authored
      Summary:
      In current commit protocol of pessimistic transaction, if the transaction is not prepared before commit, the commit protocol implicitly assumes that the user wants to commit without prepare.
      
      This PR adds TransactionOptions::skip_prepare, the default value is `true` because if set to `false`, all existing users who commit without prepare need to update their code to set skip_prepare to true. Although this does not force the user to explicitly express their intention of skip_prepare, it at least lets the user be aware of the assumption of being able to commit without prepare.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6775
      
      Test Plan: added a new unit test TransactionTest::CommitWithoutPrepare
      
      Reviewed By: lth
      
      Differential Revision: D21313270
      
      Pulled By: cheng-chang
      
      fbshipit-source-id: 3d95b7c9b2d6cdddc09bdd66c561bc4fae8c3251
      ef0c3eda
    • sdong's avatar
      Disallow BlockBasedTableBuilder to set status from non-OK (#6776) · 079e50d2
      sdong authored
      Summary:
      There is no systematic mechanism to prevent BlockBasedTableBuilder's status to be set from non-OK to OK. Adding a mechanism to force this will help us prevent failures in the future.
      
      The solution is to only make it possible to set the status code if the status code to set is not OK.
      
      Since the status code passed to CompressAndVerifyBlock() is changed, a mini refactoring is done too so that the output arguments are changed from reference to pointers, based on Google C++ Style.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6776
      
      Test Plan: Run all existing test.
      
      Reviewed By: pdillinger
      
      Differential Revision: D21314382
      
      fbshipit-source-id: 27000c10f1e4c121661e026548d6882066409375
      079e50d2
    • sdong's avatar
      Flag CompressionOptions::parallel_threads to be experimental (#6781) · 6277e280
      sdong authored
      Summary:
      The feature of CompressionOptions::parallel_threads is still not yet mature. Mention it to be experimental in the comments for now.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6781
      
      Reviewed By: pdillinger
      
      Differential Revision: D21330678
      
      fbshipit-source-id: d7dd7d099fb002a5c6a5d8da689ce5ee08a9eb13
      6277e280
    • anand76's avatar
      Pass a timeout to FileSystem for random reads (#6751) · ab13d43e
      anand76 authored
      Summary:
      Calculate ```IOOptions::timeout``` using ```ReadOptions::deadline``` and pass it to ```FileSystem::Read/FileSystem::MultiRead```. This allows us to impose a tighter bound on the time taken by Get/MultiGet on FileSystem/Envs that support IO timeouts. Even on those that don't support, check in ```RandomAccessFileReader::Read``` and ```MultiRead``` and return ```Status::TimedOut()``` if the deadline is exceeded.
      
      For now, TableReader creation, which might do file opens and reads, are not covered. It will be implemented in another PR.
      
      Tests:
      Update existing unit tests to verify the correct timeout value is being passed
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6751
      
      Reviewed By: riversand963
      
      Differential Revision: D21285631
      
      Pulled By: anand1976
      
      fbshipit-source-id: d89af843e5a91ece866e87aa29438b52a65a8567
      ab13d43e
    • Peter Dillinger's avatar
      Fix assertion that can fail on sst corruption (#6780) · eecd8fba
      Peter Dillinger authored
      Summary:
      An assertion that a char == a CompressionType (unsigned char)
      originally cast from a char can fail if the original value is negative,
      due to numeric promotion.  The assertion should pass even if the value
      is invalid CompressionType, because the callee
      UncompressBlockContentsForCompressionType checks for that and reports
      status appropriately.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6780
      
      Test Plan:
      Temporarily change kZSTD = 0x88 and see tests fail. Make this
      change (in addition), and tests pass.
      
      Reviewed By: siying
      
      Differential Revision: D21328498
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 61caf8d815581ce49261ecb7ab0f396e9ac4bb92
      eecd8fba
    • Levi Tamasi's avatar
      Keep track of obsolete blob files in VersionSet (#6755) · fe238e54
      Levi Tamasi authored
      Summary:
      The patch adds logic to keep track of obsolete blob files. A blob file becomes
      obsolete when the last `shared_ptr` that points to the corresponding
      `SharedBlobFileMetaData` object goes away, which, in turn, happens when the
      last `Version` that contains the blob file is destroyed. No longer needed blob
      files are added to the obsolete list in `VersionSet` using a custom deleter to
      avoid unnecessary coupling between `SharedBlobFileMetaData` and `VersionSet`.
      Obsolete blob files are returned by `VersionSet::GetObsoleteFiles` and stored
      in `JobContext`.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6755
      
      Test Plan: `make check`
      
      Reviewed By: riversand963
      
      Differential Revision: D21233155
      
      Pulled By: ltamasi
      
      fbshipit-source-id: 47757e06fdc0127f27ed57f51abd27893d9a7b7a
      fe238e54
    • Adam Retter's avatar
      Add Slack forum to README (#6773) · cf342464
      Adam Retter authored
      Summary: Pull Request resolved: https://github.com/facebook/rocksdb/pull/6773
      
      Reviewed By: siying
      
      Differential Revision: D21310229
      
      Pulled By: pdillinger
      
      fbshipit-source-id: c0d52d0c51121d307d7d5c1374abc7bf78b0c4cf
      cf342464
    • Ziyue Yang's avatar
      Add an option for parallel compression in for db_stress (#6722) · e619a20e
      Ziyue Yang authored
      Summary:
      This commit adds an `compression_parallel_threads` option in
      db_stress. It also fixes the naming of parallel compression
      option in db_bench to keep it aligned with others.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6722
      
      Reviewed By: pdillinger
      
      Differential Revision: D21091385
      
      fbshipit-source-id: c9ba8c4e5cc327ff9e6094a6dc6a15fcff70f100
      e619a20e
  8. 30 Apr, 2020 4 commits
  9. 29 Apr, 2020 1 commit