1. 16 1月, 2020 1 次提交
    • S
      Fix kHashSearch bug with SeekForPrev (#6297) · d2b4d42d
      sdong 提交于
      Summary:
      When prefix is enabled the expected behavior when the prefix of the target does not exist is for Seek is to seek to any key larger than target and SeekToPrev to any key less than the target.
      Currently. the prefix index (kHashSearch) returns OK status but sets Invalid() to indicate two cases: a prefix of the searched key does not exist, ii) the key is beyond the range of the keys in SST file. The SeekForPrev implementation in BlockBasedTable thus does not have enough information to know when it should set the index key to first (to return a key smaller than target). The patch fixes that by returning NotFound status for cases that the prefix does not exist. SeekForPrev in BlockBasedTable accordingly SeekToFirst instead of SeekToLast on the index iterator.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6297
      
      Test Plan: SeekForPrev of non-exsiting prefix is added to block_test.cc, and a test case is added in db_test2, which fails without the fix.
      
      Differential Revision: D19404695
      
      fbshipit-source-id: cafbbf95f8f60ff9ede9ccc99d25bfa1cf6fcdc3
      d2b4d42d
  2. 15 1月, 2020 1 次提交
  3. 14 1月, 2020 1 次提交
    • S
      Bug when multiple files at one level contains the same smallest key (#6285) · 894c6d21
      sdong 提交于
      Summary:
      The fractional cascading index is not correctly generated when two files at the same level contains the same smallest or largest user key.
      The result would be that it would hit an assertion in debug mode and lower level files might be skipped.
      This might cause wrong results when the same user keys are of merge operands and Get() is called using the exact user key. In that case, the lower files would need to further checked.
      The fix is to fix the fractional cascading index.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6285
      
      Test Plan: Add a unit test which would cause the assertion which would be fixed.
      
      Differential Revision: D19358426
      
      fbshipit-source-id: 39b2b1558075fd95e99491d462a67f9f2298c48e
      894c6d21
  4. 11 1月, 2020 3 次提交
    • Q
      More const pointers in C API (#6283) · 6733be03
      Qinfan Wu 提交于
      Summary:
      This makes it easier to call the functions from Rust as otherwise they require mutable types.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6283
      
      Differential Revision: D19349991
      
      Pulled By: wqfish
      
      fbshipit-source-id: e8da7a75efe8cd97757baef8ca844a054f2519b4
      6733be03
    • S
      Consider all compaction input files to compute the oldest ancestor time (#6279) · cfa58561
      Sagar Vemuri 提交于
      Summary:
      Look at all compaction input files to compute the oldest ancestor time.
      
      In https://github.com/facebook/rocksdb/issues/5992 we changed how creation_time (aka oldest-ancestor-time) table property of compaction output files is computed from max(creation-time-of-all-compaction-inputs) to min(creation-time-of-all-inputs). This exposed a bug where, during compaction, the creation_time:s of only the L0 compaction inputs were being looked at, and all other input levels were being ignored. This PR fixes the issue.
      Some TTL compactions when using Level-Style compactions might not have run due to this bug.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6279
      
      Test Plan: Enhanced the unit tests to validate that the correct time is propagated to the compaction outputs.
      
      Differential Revision: D19337812
      
      Pulled By: sagar0
      
      fbshipit-source-id: edf8a72f11e405e93032ff5f45590816debe0bb4
      cfa58561
    • M
      unordered_write incompatible with max_successive_merges (#6284) · eff5e076
      Maysam Yabandeh 提交于
      Summary:
      unordered_write is incompatible with non-zero max_successive_merges. Although we check this at runtime, we currently don't prevent the user from setting this combination in options. This has led to stress tests to fail with this combination is tried in ::SetOptions.
      The patch fixes that and also reverts the changes performed by https://github.com/facebook/rocksdb/pull/6254, in which max_successive_merges was mistakenly declared incompatible with unordered_write.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6284
      
      Differential Revision: D19356115
      
      Pulled By: maysamyabandeh
      
      fbshipit-source-id: f06dadec777622bd75f267361c022735cf8cecb6
      eff5e076
  5. 10 1月, 2020 1 次提交
  6. 09 1月, 2020 2 次提交
  7. 08 1月, 2020 3 次提交
    • Y
      Fix test in LITE mode (#6267) · a8b1085a
      Yanqin Jin 提交于
      Summary:
      Currently, the recently-added test DBTest2.SwitchMemtableRaceWithNewManifest
      fails in LITE mode since SetOptions() returns "Not supported". I do not want to
      put `#ifndef ROCKSDB_LITE` because it reduces test coverage. Instead, just
      trigger compaction on a different column family. The bg compaction thread
      calling LogAndApply() may race with thread calling SwitchMemtable().
      
      Test Plan (dev server):
      make check
      OPT=-DROCKSDB_LITE make check
      
      or run DBTest2.SwitchMemtableRaceWithNewManifest 100 times.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6267
      
      Differential Revision: D19301309
      
      Pulled By: riversand963
      
      fbshipit-source-id: 88cedcca2f985968ed3bb234d324ffa2aa04ca50
      a8b1085a
    • Y
      Fix error message (#6264) · bce5189f
      Yanqin Jin 提交于
      Summary:
      Fix an error message when CURRENT is not found.
      
      Test plan (dev server)
      ```
      make check
      ```
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6264
      
      Differential Revision: D19300699
      
      Pulled By: riversand963
      
      fbshipit-source-id: 303fa206386a125960ecca1dbdeff07422690caf
      bce5189f
    • C
      Add oldest snapshot sequence property (#6228) · 3e26a94b
      Connor1996 提交于
      Summary:
      Add oldest snapshot sequence property, so we can use `db.GetProperty("rocksdb.oldest-snapshot-sequence")` to get the sequence number of the oldest snapshot.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6228
      
      Differential Revision: D19264145
      
      Pulled By: maysamyabandeh
      
      fbshipit-source-id: 67fbe5304d89cbc475bd404e30d1299f7b11c010
      3e26a94b
  8. 07 1月, 2020 3 次提交
    • Y
      Fix a data race for cfd->log_number_ (#6249) · 1aaa1458
      Yanqin Jin 提交于
      Summary:
      A thread calling LogAndApply may release db mutex when calling
      WriteCurrentStateToManifest() which reads cfd->log_number_. Another thread can
      call SwitchMemtable() and writes to cfd->log_number_.
      Solution is to cache the cfd->log_number_ before releasing mutex in
      LogAndApply.
      
      Test Plan (on devserver):
      ```
      $COMPILE_WITH_TSAN=1 make db_stress
      $./db_stress --acquire_snapshot_one_in=10000 --avoid_unnecessary_blocking_io=1 --block_size=16384 --bloom_bits=16 --bottommost_compression_type=zstd --cache_index_and_filter_blocks=1 --cache_size=1048576 --checkpoint_one_in=1000000 --checksum_type=kxxHash --clear_column_family_one_in=0 --compact_files_one_in=1000000 --compact_range_one_in=1000000 --compaction_ttl=0 --compression_max_dict_bytes=16384 --compression_type=zstd --compression_zstd_max_train_bytes=0 --continuous_verification_interval=0 --db=/dev/shm/rocksdb/rocksdb_crashtest_blackbox --db_write_buffer_size=1048576 --delpercent=5 --delrangepercent=0 --destroy_db_initially=0 --enable_pipelined_write=0  --flush_one_in=1000000 --format_version=5 --get_live_files_and_wal_files_one_in=1000000 --index_block_restart_interval=5 --index_type=0 --log2_keys_per_lock=22 --long_running_snapshots=0 --max_background_compactions=20 --max_bytes_for_level_base=10485760 --max_key=1000000 --max_manifest_file_size=16384 --max_write_batch_group_size_bytes=16 --max_write_buffer_number=3 --memtablerep=skip_list --mmap_read=0 --nooverwritepercent=1 --open_files=500000 --ops_per_thread=100000000 --partition_filters=0 --pause_background_one_in=1000000 --periodic_compaction_seconds=0 --prefixpercent=5 --progress_reports=0 --readpercent=45 --recycle_log_file_num=0 --reopen=20 --set_options_one_in=10000 --snapshot_hold_ops=100000 --subcompactions=2 --sync=1 --target_file_size_base=2097152 --target_file_size_multiplier=2 --test_batches_snapshots=1 --use_direct_io_for_flush_and_compaction=0 --use_direct_reads=0 --use_full_merge_v1=0 --use_merge=0 --use_multiget=1 --verify_checksum=1 --verify_checksum_one_in=1000000 --verify_db_one_in=100000 --write_buffer_size=4194304 --write_dbid_to_manifest=1 --writepercent=35
      ```
      Then repeat the following multiple times, e.g. 100 after compiling with tsan.
      ```
      $./db_test2 --gtest_filter=DBTest2.SwitchMemtableRaceWithNewManifest
      ```
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6249
      
      Differential Revision: D19235077
      
      Pulled By: riversand963
      
      fbshipit-source-id: 79467b52f48739ce7c27e440caa2447a40653173
      1aaa1458
    • Q
      Add range delete function to C-API (#6259) · edaaa1ff
      Qinfan Wu 提交于
      Summary:
      It seems that the C-API doesn't expose the range delete functionality at the moment, so add the API.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6259
      
      Differential Revision: D19290320
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 3f403a4c3446d2042d55f1ece7cdc9c040f40c27
      edaaa1ff
    • M
      Increase max_log_size in FlushJob to 1024 bytes (#6258) · 28e5a9a9
      Maysam Yabandeh 提交于
      Summary:
      When measure_io_stats_ is enabled, the volume of logging is beyond the default limit of 512 size. The patch allows the EventLoggerStream to change the limit, and also sets it to 1024 for FlushJob.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6258
      
      Differential Revision: D19279269
      
      Pulled By: maysamyabandeh
      
      fbshipit-source-id: 3fb5d468dad488f289ac99d713378177eb7504d6
      28e5a9a9
  9. 03 1月, 2020 1 次提交
    • M
      Prevent an incompatible combination of options (#6254) · 48a678b7
      Maysam Yabandeh 提交于
      Summary:
      allow_concurrent_memtable_write is incompatible with non-zero max_successive_merges. Although we check this at runtime, we currently don't prevent the user from setting this combination in options. This has led to stress tests to fail with this combination is tried in ::SetOptions. The patch fixes that.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6254
      
      Differential Revision: D19265819
      
      Pulled By: maysamyabandeh
      
      fbshipit-source-id: 47f2e2dc26fe0972c7152f4da15dadb9703f1179
      48a678b7
  10. 19 12月, 2019 2 次提交
  11. 18 12月, 2019 4 次提交
  12. 17 12月, 2019 3 次提交
    • Y
      Use Env::LoadEnv to create custom Env objects (#6196) · 7678cf2d
      Yanqin Jin 提交于
      Summary:
      As title. Previous assumption was that the underlying lib can always return
      a shared_ptr<Env>. This is too strong. Therefore, we use Env::LoadEnv to relax
      it.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6196
      
      Test Plan: make check
      
      Differential Revision: D19133199
      
      Pulled By: riversand963
      
      fbshipit-source-id: c83a0c02a42610d077054f2de1acfc45126b3a75
      7678cf2d
    • Z
      Merge adjacent file block reads in RocksDB MultiGet() and Add uncompressed block to cache (#6089) · cddd6379
      Zhichao Cao 提交于
      Summary:
      In the current MultiGet, if the KV-pairs do not belong to the data blocks in the block cache, multiple blocks are read from a SST. It will trigger one block read for each block request and read them in parallel. In some cases, if some data blocks are adjacent in the SST, the reads for these blocks can be combined to a single large read, which can reduce the system calls and reduce the read latency if possible.
      
      Considering to fill the block cache, if multiple data blocks are in the same memory buffer, we need to copy them to the heap separately. Therefore, only in the case that 1) data block compression is enabled, and 2) compressed block cache is null, we can do combined read. Otherwise, extra memory copy is needed, which may cause extra overhead. In the current case, data blocks will be uncompressed to a new memory space.
      
      Also, in the case that 1) data block compression is enabled, and 2) compressed block cache is null, it is possible the data block is actually not compressed. In the current logic, these data blocks will not be added to the uncompressed_cache. So if memory buffer is shared and the data block is not compressed, the data block are copied to the head and fill the cache.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6089
      
      Test Plan: Added test case to ParallelIO.MultiGet. Pass make asan_check
      
      Differential Revision: D18734668
      
      Pulled By: zhichao-cao
      
      fbshipit-source-id: 67c5615ed373e51e42635fd74b36f8f3a66d5da4
      cddd6379
    • L
      Fix a data race related to memtable trimming (#6187) · db7c6875
      Levi Tamasi 提交于
      Summary:
      https://github.com/facebook/rocksdb/pull/6177 introduced a data race
      involving `MemTableList::InstallNewVersion` and `MemTableList::NumFlushed`.
      The patch fixes this by caching whether the current version has any
      memtable history (i.e. flushed memtables that are kept around for
      transaction conflict checking) in an `std::atomic<bool>` member called
      `current_has_history_`, similarly to how `current_memory_usage_excluding_last_`
      is handled.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6187
      
      Test Plan:
      ```
      make clean
      COMPILE_WITH_TSAN=1 make db_test -j24
      ./db_test
      ```
      
      Differential Revision: D19084059
      
      Pulled By: ltamasi
      
      fbshipit-source-id: 327a5af9700fb7102baea2cc8903c085f69543b9
      db7c6875
  13. 14 12月, 2019 5 次提交
    • L
      Do not schedule memtable trimming if there is no history (#6177) · bd8404fe
      Levi Tamasi 提交于
      Summary:
      We have observed an increase in CPU load caused by frequent calls to
      `ColumnFamilyData::InstallSuperVersion` from `DBImpl::TrimMemtableHistory`
      when using `max_write_buffer_size_to_maintain` to limit the amount of
      memtable history maintained for transaction conflict checking. Part of the issue
      is that trimming can potentially be scheduled even if there is no memtable
      history. The patch adds a check that fixes this.
      
      See also https://github.com/facebook/rocksdb/pull/6169.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6177
      
      Test Plan:
      Compared `perf` output for
      
      ```
      ./db_bench -benchmarks=randomtransaction -optimistic_transaction_db=1 -statistics -stats_interval_seconds=1 -duration=90 -num=500000 --max_write_buffer_size_to_maintain=16000000 --transaction_set_snapshot=1 --threads=32
      ```
      
      before and after the change. There is a significant reduction for the call chain
      `rocksdb::DBImpl::TrimMemtableHistory` -> `rocksdb::ColumnFamilyData::InstallSuperVersion` ->
      `rocksdb::ThreadLocalPtr::StaticMeta::Scrape` even without https://github.com/facebook/rocksdb/pull/6169.
      
      Differential Revision: D19057445
      
      Pulled By: ltamasi
      
      fbshipit-source-id: dff81882d7b280e17eda7d9b072a2d4882c50f79
      bd8404fe
    • A
      Introduce a new storage specific Env API (#5761) · afa2420c
      anand76 提交于
      Summary:
      The current Env API encompasses both storage/file operations, as well as OS related operations. Most of the APIs return a Status, which does not have enough metadata about an error, such as whether its retry-able or not, scope (i.e fault domain) of the error etc., that may be required in order to properly handle a storage error. The file APIs also do not provide enough control over the IO SLA, such as timeout, prioritization, hinting about placement and redundancy etc.
      
      This PR separates out the file/storage APIs from Env into a new FileSystem class. The APIs are updated to return an IOStatus with metadata about the error, as well as to take an IOOptions structure as input in order to allow more control over the IO.
      
      The user can set both ```options.env``` and ```options.file_system``` to specify that RocksDB should use the former for OS related operations and the latter for storage operations. Internally, a ```CompositeEnvWrapper``` has been introduced that inherits from ```Env``` and redirects individual methods to either an ```Env``` implementation or the ```FileSystem``` as appropriate. When options are sanitized during ```DB::Open```, ```options.env``` is replaced with a newly allocated ```CompositeEnvWrapper``` instance if both env and file_system have been specified. This way, the rest of the RocksDB code can continue to function as before.
      
      This PR also ports PosixEnv to the new API by splitting it into two - PosixEnv and PosixFileSystem. PosixEnv is defined as a sub-class of CompositeEnvWrapper, and threading/time functions are overridden with Posix specific implementations in order to avoid an extra level of indirection.
      
      The ```CompositeEnvWrapper``` translates ```IOStatus``` return code to ```Status```, and sets the severity to ```kSoftError``` if the io_status is retryable. The error handling code in RocksDB can then recover the DB automatically.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/5761
      
      Differential Revision: D18868376
      
      Pulled By: anand1976
      
      fbshipit-source-id: 39efe18a162ea746fabac6360ff529baba48486f
      afa2420c
    • P
      Add useful idioms to Random API (OneInOpt, PercentTrue) (#6154) · 58d46d19
      Peter Dillinger 提交于
      Summary:
      And clean up related code, especially in stress test.
      
      (More clean up of db_stress_test_base.cc coming after this.)
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6154
      
      Test Plan: make check, make blackbox_crash_test for a bit
      
      Differential Revision: D18938180
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 524d27621b8dbb25f6dff40f1081e7c00630357e
      58d46d19
    • L
      Do not create/install new SuperVersion if nothing was deleted during memtable trim (#6169) · 6d54eb3d
      Levi Tamasi 提交于
      Summary:
      We have observed an increase in CPU load caused by frequent calls to
      `ColumnFamilyData::InstallSuperVersion` from `DBImpl::TrimMemtableHistory`
      when using `max_write_buffer_size_to_maintain` to limit the amount of
      memtable history maintained for transaction conflict checking. As it turns out,
      this is caused by the code creating and installing a new `SuperVersion` even if
      no memtables were actually trimmed. The patch adds a check to avoid this.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6169
      
      Test Plan:
      Compared `perf` output for
      
      ```
      ./db_bench -benchmarks=randomtransaction -optimistic_transaction_db=1 -statistics -stats_interval_seconds=1 -duration=90 -num=500000 --max_write_buffer_size_to_maintain=16000000 --transaction_set_snapshot=1 --threads=32
      ```
      
      before and after the change. With the fix, the call chain `rocksdb::DBImpl::TrimMemtableHistory` ->
      `rocksdb::ColumnFamilyData::InstallSuperVersion` -> `rocksdb::ThreadLocalPtr::StaticMeta::Scrape`
      no longer registers in the `perf` report.
      
      Differential Revision: D19031509
      
      Pulled By: ltamasi
      
      fbshipit-source-id: 02686fce594e5b50eba0710e4b28a9b808c8aa20
      6d54eb3d
    • L
      Move out valid blobs from the oldest blob files during compaction (#6121) · 583c6953
      Levi Tamasi 提交于
      Summary:
      The patch adds logic that relocates live blobs from the oldest N non-TTL
      blob files as they are encountered during compaction (assuming the BlobDB
      configuration option `enable_garbage_collection` is `true`), where N is defined
      as the number of immutable non-TTL blob files multiplied by the value of
      a new BlobDB configuration option called `garbage_collection_cutoff`.
      (The default value of this parameter is 0.25, that is, by default the valid blobs
      residing in the oldest 25% of immutable non-TTL blob files are relocated.)
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6121
      
      Test Plan: Added unit test and tested using the BlobDB mode of `db_bench`.
      
      Differential Revision: D18785357
      
      Pulled By: ltamasi
      
      fbshipit-source-id: 8c21c512a18fba777ec28765c88682bb1a5e694e
      583c6953
  14. 13 12月, 2019 3 次提交
    • J
      Support concurrent CF iteration and drop (#6147) · c2029f97
      Jermy Li 提交于
      Summary:
      It's easy to cause coredump when closing ColumnFamilyHandle with unreleased iterators, especially iterators release is controlled by java GC when using JNI.
      
      This patch fixed concurrent CF iteration and drop, we let iterators(actually SuperVersion) hold a ColumnFamilyData reference to prevent the CF from being released too early.
      
      fixed https://github.com/facebook/rocksdb/issues/5982
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6147
      
      Differential Revision: D18926378
      
      fbshipit-source-id: 1dff6d068c603d012b81446812368bfee95a5e15
      c2029f97
    • Fix RangeDeletion bug (#6062) · c4ce8e63
      奏之章 提交于
      Summary:
      Read keys from a snapshot that a range deletion were added after the snapshot  was created and this range deletion was inside an immutable memtable, we will get wrong key set.
      More detail rest in codes.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6062
      
      Differential Revision: D18966785
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 38a60bb1e2d0a1dbfc8ec641617200b6a02b86c3
      c4ce8e63
    • C
      wait pending memtable writes on file ingestion or compact range (#6113) · a8445912
      Connor 提交于
      Summary:
      **Summary:**
      This PR fixes two unordered_write related issues:
      - ingestion job may skip the necessary memtable flush https://github.com/facebook/rocksdb/issues/6026
      - compact range may cause memtable is flushed before pending unordered write finished
          1. `CompactRange` triggers memtable flush but doesn't wait for pending-writes
          2.  there are some pending writes but memtable is already flushed
          3.  the memtable related WAL is removed( note that the pending-writes were recorded in that WAL).
          4.  pending-writes write to newer created memtable
          5. there is a restart
          6. missing the previous pending-writes because WAL is removed but they aren't included in SST.
      
      **How to solve:**
      - Wait pending memtable writes before ingestion job check memtable key range
      - Wait pending memtable writes before flush memtable.
      **Note that: `CompactRange` calls `RangesOverlapWithMemtables` too without waiting for pending waits, but I'm not sure whether it affects the correctness.**
      
      **Test Plan:**
      make check
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6113
      
      Differential Revision: D18895674
      
      Pulled By: maysamyabandeh
      
      fbshipit-source-id: da22b4476fc7e06c176020e7cc171eb78189ecaf
      a8445912
  15. 12 12月, 2019 4 次提交
  16. 10 12月, 2019 3 次提交
    • S
      Apply formatter to some recent commits (#6138) · a68dff5c
      sdong 提交于
      Summary:
      Formatter somehow complains some recent lines changed. Apply them to make the formatter happy.
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6138
      
      Test Plan: See CI passes.
      
      Differential Revision: D18895950
      
      fbshipit-source-id: 7d1696cf3e3a682bc10a30cdca748a23c6565255
      a68dff5c
    • P
      Fix & test rocksdb_filterpolicy_create_bloom_full (#6132) · e43d2c44
      Peter Dillinger 提交于
      Summary:
      Add overrides needed in FilterPolicy wrapper to fix
      rocksdb_filterpolicy_create_bloom_full (see issue https://github.com/facebook/rocksdb/issues/6129). Re-enabled
      assertion in BloomFilterPolicy::CreateFilter that was being violated.
      Expanded c_test to identify Bloom filter implementations by FP counts.
      (Without the fix, updated test will trigger assertion and fail otherwise
      without the assertion.)
      
      Fixes https://github.com/facebook/rocksdb/issues/6129
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6132
      
      Test Plan: updated c_test, also run under valgrind.
      
      Differential Revision: D18864911
      
      Pulled By: pdillinger
      
      fbshipit-source-id: 08e81d7b5368b08e501cd402ef5583f2650c19fa
      e43d2c44
    • Z
      Fix wrong ExtractUserKey usage in BlockBasedTableBuilder::EnterUnbuff… (#6100) · 7e2f8319
      Ziyue Yang 提交于
      Summary:
      BlockBasedTableBuilder uses ExtractUserKey in EnterUnbuffered. This would
      cause index filter building error, since user-provided timestamp is supported
      by ExtractUserKeyAndStripTimestamp, and it's used in Add. This commit changes
      ExtractUserKey to ExtractUserKeyAndStripTimestamp.
      
      A test case is also added by modifying DBBasicTestWithTimestampWithParam_
      PutAndGet test in db_basic_test to cover ExtractUserKeyAndStripTimestamp usage
      in both kBuffered and kUnbuffered state of BlockBasedTableBuilder.
      
      Before the ExtractUserKeyAndStripTimstamp fix:
      
      ```
      $ ./db_basic_test --gtest_filter="*PutAndGet*"
      Note: Google Test filter = *PutAndGet*
      [==========] Running 2 tests from 1 test case.
      [----------] Global test environment set-up.
      [----------] 2 tests from Timestamp/DBBasicTestWithTimestampWithParam
      [ RUN      ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/0
      db/db_basic_test.cc:2109: Failure
      db_->Get(ropts, cfh, "key" + std::to_string(j), &value)
      NotFound:
      db/db_basic_test.cc:2109: Failure
      db_->Get(ropts, cfh, "key" + std::to_string(j), &value)
      NotFound:
      db/db_basic_test.cc:2109: Failure
      db_->Get(ropts, cfh, "key" + std::to_string(j), &value)
      NotFound:
      db/db_basic_test.cc:2109: Failure
      db_->Get(ropts, cfh, "key" + std::to_string(j), &value)
      NotFound:
      db/db_basic_test.cc:2109: Failure
      db_->Get(ropts, cfh, "key" + std::to_string(j), &value)
      NotFound:
      [  FAILED  ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/0, where GetParam() = false (1177 ms)
      [ RUN      ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/1
      [       OK ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/1 (1056 ms)
      [----------] 2 tests from Timestamp/DBBasicTestWithTimestampWithParam (2233 ms total)
      
      [----------] Global test environment tear-down
      [==========] 2 tests from 1 test case ran. (2233 ms total)
      [  PASSED  ] 1 test.
      [  FAILED  ] 1 test, listed below:
      [  FAILED  ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/0, where GetParam() = false
      
       1 FAILED TEST
      ```
      
      After the ExtractUserKeyAndStripTimstamp fix:
      
      ```
      $ ./db_basic_test --gtest_filter="*PutAndGet*"
      Note: Google Test filter = *PutAndGet*
      [==========] Running 2 tests from 1 test case.
      [----------] Global test environment set-up.
      [----------] 2 tests from Timestamp/DBBasicTestWithTimestampWithParam
      [ RUN      ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/0
      [       OK ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/0 (1417 ms)
      [ RUN      ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/1
      [       OK ] Timestamp/DBBasicTestWithTimestampWithParam.PutAndGet/1 (1041 ms)
      [----------] 2 tests from Timestamp/DBBasicTestWithTimestampWithParam (2458 ms total)
      
      [----------] Global test environment tear-down
      [==========] 2 tests from 1 test case ran. (2458 ms total)
      [  PASSED  ] 2 tests.
      ```
      Pull Request resolved: https://github.com/facebook/rocksdb/pull/6100
      
      Differential Revision: D18769654
      
      Pulled By: riversand963
      
      fbshipit-source-id: 76c2cf2c9a5e0d85db95d98e812e6af0c2a15c6b
      7e2f8319