Commit Graph

4807 Commits

Author SHA1 Message Date
Marton Trencseni
522de4f59e Adding pin_l0_filter_and_index_blocks_in_cache feature.
Summary:
When a block based table file is opened, if prefetch_index_and_filter is true, it will prefetch the index and filter blocks, putting them into the block cache.
What this feature adds: when a L0 block based table file is opened, if pin_l0_filter_and_index_blocks_in_cache is true in the options (and prefetch_index_and_filter is true), then the filter and index blocks aren't released back to the block cache at the end of BlockBasedTableReader::Open(). Instead the table reader takes ownership of them, hence pinning them, ie. the LRU cache will never push them out. Meanwhile in the table reader, further accesses will not hit the block cache, thus avoiding lock contention.
When the table reader is destroyed, it releases the pinned blocks (if there were any). This has to happen before the cache is destroyed, so I had to introduce a TableReader::Close(), to guarantee the order of destruction.

Test Plan:
Added two unit tests for this. Existing unit tests run fine (default is pin_l0_filter_and_index_blocks_in_cache=false).

DISABLE_JEMALLOC=1 OPT=-g make all valgrind_check -j32
  Mac: OK.
  Linux: with D55287 patched in it's OK.

Reviewers: sdong

Reviewed By: sdong

Subscribers: andrewkr, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D54801
2016-03-17 22:40:01 +00:00
Islam AbdelRahman
be22271265 Merge pull request #1041 from yuslepukhin/adjust_for_jemalloc
Latest versions of Jemalloc library do not require je_init()/je_unint()
2016-03-17 14:50:19 -07:00
Dmitri Smirnov
2ca0994cf7 Latest versions of Jemalloc library do not require je_init()/je_unint()
calls. #ifdef in the source code and make this a default build option.
2016-03-17 11:25:20 -07:00
Gunnar Kudrjavets
90aff0c444 Update --max_write_buffer_number for compaction benchmarks
Summary: For compactions benchmarks (both level and universal) we'll use `--max_write_buffer_number=4`. For all the other benchmarks which don't customize the value of `--max_background_flushes` we'll continue using `--max_write_buffer_number=8`.

Test Plan:
To validate basic correctness and command-line options:

```
cd ~/rocksdb
NKEYS=10000000 ./tools/run_flash_bench.sh
```

Reviewers: MarkCallaghan

Reviewed By: MarkCallaghan

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55497
2016-03-17 10:14:23 -07:00
Andrew Kryczka
72224104d3 Forge current file for checkpoint
Summary:
This fixes a similar issue as D54711: "CURRENT" file can mutate between
GetLiveFiles() and copy to the tmp directory, in which case it would reference
the wrong manifest filename. To fix this, I forge the "CURRENT" file such that
it simply contains the filename for the manifest returned by GetLiveFiles().

- Changed CreateCheckpoint() to forge current file
- Added CreateFile() utility function
- Added test case that rolls manifest during checkpoint creation

Test Plan:
  $ ./checkpoint_test

Reviewers: sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55065
2016-03-17 10:07:21 -07:00
dhruba borthakur
33d568611d Merge pull request #1040 from bureau14/master
Fixes warnings and ensure correct int behavior on 32-bit platforms.
2016-03-17 03:23:30 -07:00
Edouard A
02e62ebbc8 Fixes warnings and ensure correct int behavior on 32-bit platforms. 2016-03-16 22:57:57 +01:00
Siying Dong
9cad56861a Merge pull request #1039 from bureau14/master
Improve documentation of the allow_os_buffer parameter.
2016-03-16 10:30:01 -07:00
Edouard Alligand
3d29f91466 Improve documentation of the allow_os_buffer parameter. 2016-03-16 15:37:55 +01:00
Islam AbdelRahman
3ff98bd209 Fix no compression test
Summary:
DBBlockCacheTest.TestWithCompressedBlockCache is depending on compression using snappy, so this test fail when snappy is not available
block this test when we don't have snappy

https://ci-builds.fb.com/view/rocksdb/job/rocksdb_no_compression/833/console

Test Plan: run the test when compression libraries are not avaliable

Reviewers: sdong, yiwu

Reviewed By: yiwu

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55413
2016-03-15 12:17:40 -07:00
Igor Canadi
b9cc42a72c Merge pull request #1038 from SherlockNoMad/HistFix
Fix FB internal CI build failure
2016-03-15 11:43:06 -07:00
SherlockNoMad
f76b260eff Fix FB internal CI build failure 2016-03-15 11:38:15 -07:00
Siying Dong
774922c680 Merge pull request #1026 from SherlockNoMad/Hist
Histogram Concurrency Improvement and Time-Windowing Support
2016-03-15 11:27:54 -07:00
Igor Canadi
17b879b91e Merge pull request #1037 from SherlockNoMad/BuildFix
Fix AppVeyor build error
2016-03-15 11:15:56 -07:00
SherlockNoMad
f11b0df121 Fix AppVeyor build error 2016-03-15 10:57:33 -07:00
Gunnar Kudrjavets
6b03f93d4f Fix the build break on Ubuntu 15.10 when gcc 5.2.1 is used
Summary:
Using gcc 5.2.1 to compile RocksDB on Ubuntu 15.10 results in a warning about unused variable. Warning is treated as an error and all of it results in a build break. Fix the issue and make sure that everything compiles with that particular configuration.

P.S. Lint complains about a non-ASCII character used in the source code ("Wagner-Fischer"). Fix this as well.

Test Plan:
  # Everything compiler cleanly on the system which exhibited the problem.
  # `make clean && make -j 16` on CentOS.
  # `make clean && USE_CLANG=1 make -j 16` on CentOS.

Reviewers: sdong, igor

Reviewed By: igor

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55503
2016-03-15 10:30:10 -07:00
Gunnar Kudrjavets
697fab820a Updates to RocksDB subcompaction benchmarking script
Summary: Set of updates to the subcompaction benchmark script which are based on our internal discussions. The intent behind the changes is to make sure that the scripts will correctly reflect how we're doing the actual benchmarking.

Test Plan: Tested by exercising the full set of compaction benchmarks and validating the execution and consistency of results.

Reviewers: MarkCallaghan, sdong, yhchiang

Reviewed By: yhchiang

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55461
2016-03-14 23:09:04 -07:00
SherlockNoMad
58379bfb55 remove division from histogramwidowing impl 2016-03-14 21:41:13 -07:00
Dhruba Borthakur
1a2cc27e01 ColumnFamilyOptions SanitizeOptions is buggy on 32-bit platforms.
Summary:
The pre-existing code is trying to clamp between 65,536 and 0,
resulting in clamping to 65,536, resulting in very small buffers,
resulting in ShouldFlushNow() being true quite easily,
resulting in assertion failing and database performance
being "not what it should be".

https://github.com/facebook/rocksdb/issues/1018

Test Plan: make check

Reviewers: sdong, andrewkr, IslamAbdelRahman, yhchiang, igor

Reviewed By: igor

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55455
2016-03-14 16:21:54 -07:00
Igor Canadi
e778c34e78 Merge pull request #1035 from bureau14/master
Added quasardb to the USERS.md file
2016-03-14 15:50:55 -07:00
Edouard A
5bd3da1c59 Added quasardb to the USERS.md file 2016-03-14 23:48:28 +01:00
sdong
b2ae5950ba Index Reader should not be reused after DB restart
Summary:
In block based table reader, wow we put index reader to block cache, which can be retrieved after DB restart. However, index reader may reference internal comparator, which can be destroyed after DB restarts, causing problems.
Fix it by making cache key identical per table reader.

Test Plan: Add a new test which failed with out the commit but now pass.

Reviewers: IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: maro, yhchiang, kradhakrishnan, leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55287
2016-03-14 10:04:09 -07:00
Andrew Kryczka
0267655dad Update change log for 4.6 release
Summary: as titled

Test Plan: N/A

Reviewers: sdong, kradhakrishnan, anthony, yhchiang, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55323
2016-03-12 13:51:57 -08:00
Andrew Kryczka
08304c0867 Expose RepairDB as ldb command
Summary: This will make it easier for admins and devs to use RepairDB.

Test Plan:
Tried deleting the manifest and verified it recovers:

  $ ldb --create_if_missing --db=/tmp/test_db put ok ok
  $ rm -f /tmp/test_db/MANIFEST-000001
  $ ./ldb --db=/tmp/test_db repair
  $ ldb --db=/tmp/test_db get ok
  ok

Reviewers: yhchiang, sdong, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55359
2016-03-12 13:50:20 -08:00
SherlockNoMad
fd664a27b5 Fix Build Error 2016-03-11 22:56:25 -08:00
Islam AbdelRahman
580fede347 Aggregate hot Iterator counters in LocalStatistics (DBIter::Next perf regression)
Summary:
This patch bump the counters in the frequent code path DBIter::Next() / DBIter::Prev() in a local data members and send them to Statistics when the iterator is destroyed
A better solution will be to have thread_local implementation for Statistics

New performance
```
readseq      :       0.035 micros/op 28597881 ops/sec; 3163.7 MB/s
     1,851,568,819      stalled-cycles-frontend   #   31.29% frontend cycles idle    [49.86%]
       884,929,823      stalled-cycles-backend    #   14.95% backend  cycles idle    [50.21%]
readreverse  :       0.071 micros/op 14077393 ops/sec; 1557.3 MB/s
     3,239,575,993      stalled-cycles-frontend   #   27.36% frontend cycles idle    [49.96%]
     1,558,253,983      stalled-cycles-backend    #   13.16% backend  cycles idle    [50.14%]

```

Existing performance

```
readreverse  :       0.174 micros/op 5732342 ops/sec;  634.1 MB/s
    20,570,209,389      stalled-cycles-frontend   #   70.71% frontend cycles idle    [50.01%]
    18,422,816,837      stalled-cycles-backend    #   63.33% backend  cycles idle    [50.04%]

readseq      :       0.119 micros/op 8400537 ops/sec;  929.3 MB/s
    15,634,225,844      stalled-cycles-frontend   #   79.07% frontend cycles idle    [49.96%]
    14,227,427,453      stalled-cycles-backend    #   71.95% backend  cycles idle    [50.09%]
```

Test Plan: unit tests

Reviewers: yhchiang, sdong, igor

Reviewed By: sdong

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D55107
2016-03-11 19:01:12 -08:00
SherlockNoMad
54f6b9e162 Histogram Concurrency Improvement and Time-Windowing Support 2016-03-11 16:54:25 -08:00
agiardullo
790252805d Add multithreaded transaction test
Summary: Refactored db_bench transaction stress tests so that they can be called from unit tests as well.

Test Plan: run new unit test as well as db_bench

Reviewers: yhchiang, IslamAbdelRahman, sdong

Reviewed By: IslamAbdelRahman

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55203
2016-03-11 15:16:52 -08:00
Baris Yazici
e8e6cf0173 fix: handle_fatal_signal (sig=6) in std::vector<std::string, std::allocator<std::string> >::_M_range_check | c++/4.8.2/bits/stl_vector.h:794 #174
Summary:
Fix for https://github.com/facebook/mysql-5.6/issues/174

When there is no old files to purge, vector.at(i) function was crashing

if (old_info_log_file_count != 0 &&
      old_info_log_file_count >= db_options_.keep_log_file_num) {
    std::sort(old_info_log_files.begin(), old_info_log_files.end());
    size_t end = old_info_log_file_count - db_options_.keep_log_file_num;
    for (unsigned int i = 0; i <= end; i++) {
      std::string& to_delete = old_info_log_files.at(i);

Added check to old_info_log_file_count be non zero.

Test Plan: run existing tests

Reviewers: gunnarku, vasilep, sdong, yhchiang

Reviewed By: yhchiang

Subscribers: andrewkr, webscalesql-eng, dhruba

Differential Revision: https://reviews.facebook.net/D55245
2016-03-11 11:11:45 -08:00
Andrew Kryczka
d9620239d2 Cleanup stale manifests outside of full purge
Summary:
- Keep track of obsolete manifests in VersionSet
- Updated FindObsoleteFiles() to put obsolete manifests in the JobContext for later use by PurgeObsoleteFiles()
- Added test case that verifies a stale manifest is deleted by a non-full purge

Test Plan:
  $ ./backupable_db_test --gtest_filter=BackupableDBTest.ChangeManifestDuringBackupCreation

Reviewers: IslamAbdelRahman, yoshinorim, sdong

Reviewed By: sdong

Subscribers: andrewkr, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D55269
2016-03-10 18:16:21 -08:00
Yi Wu
f71fc77b7c Cache to have an option to fail Cache::Insert() when full
Summary:
Cache to have an option to fail Cache::Insert() when full. Update call sites to check status and handle error.

I totally have no idea what's correct behavior of all the call sites when they encounter error. Please let me know if you see something wrong or more unit test is needed.

Test Plan: make check -j32, see tests pass.

Reviewers: anthony, yhchiang, andrewkr, IslamAbdelRahman, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D54705
2016-03-10 17:35:19 -08:00
Igor Canadi
ee8cc35201 Merge pull request #938 from alexander-fenster/master
added --no_value option to ldb scan to dump key only
2016-03-10 16:44:52 -08:00
Yueh-Hsuan Chiang
765597fa78 Update compaction score right after CompactFiles forms a compaction
Summary:
This is a follow-up patch of https://reviews.facebook.net/D54891.
As the information about files being compacted will also be used
when making compaction decision, it is necessary to update the compaction
score when a compaction plan has been made but not yet execute.

This patch adds a missing call to update the compaction score in
CompactFiles().

Test Plan: compact_files_test

Reviewers: sdong, IslamAbdelRahman, kradhakrishnan, yiwu, andrewkr

Reviewed By: andrewkr

Subscribers: andrewkr, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55227
2016-03-10 14:34:28 -08:00
Alexander Fenster
f0161c37b0 formatting fix 2016-03-10 13:34:42 -08:00
Yueh-Hsuan Chiang
aa3f02d50c Improve comment in compaction.h and compaction_picker.h
Summary:
ReleaseCompactionFiles must be called when DB mutex is held,
but the documentation is mission.

Test Plan: no code change

Reviewers: anthony, IslamAbdelRahman, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D54987
2016-03-08 16:46:41 -08:00
agiardullo
2200295ee1 optimistic transactions support for reinitialization
Summary: Extend optimization in D53835 to optimistic transactions for completeness.

Test Plan: added test

Reviewers: sdong, IslamAbdelRahman, horuff, jkedgar

Reviewed By: horuff

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55059
2016-03-07 19:03:09 -08:00
Islam AbdelRahman
badd6b7846 Ignore db_test2
Summary: Add db_test2 to .gitignore

Test Plan: make sure db_test2 dont show in "git status"

Reviewers: sdong, yhchiang, andrewkr, anthony

Reviewed By: anthony

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D55191
2016-03-07 15:56:16 -08:00
agiardullo
200080ed72 Improve snapshot handling for Transaction reinitialization
Summary: Previously, reusing a transaction (by passing it as an argument to BeginTransaction) would not clear the transaction's snapshot.  This is not a clear, well-definited behavior.

Test Plan: improved test

Reviewers: sdong, IslamAbdelRahman, horuff, jkedgar

Reviewed By: jkedgar

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55053
2016-03-07 13:28:11 -08:00
Islam AbdelRahman
171c8e80b1 Update dependencies / Fix Clang
Summary:
Update dependencies and fix broken clang symlink

version changes
```
zstd 0.4.7 => 0.5.1
jemalloc 4.0.3 => master
```

Test Plan:
make check -j64
USE_CLANG=1 make check -j64
ROCKSDB_FBCODE_BUILD_WITH_481=1 make check -j64

Reviewers: sdong, yhchiang, anthony, andrewkr

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D55179
2016-03-07 12:57:19 -08:00
sdong
294bdf9ee2 Change Property name from "rocksdb.current_version_number" to "rocksdb.current-super-version-number"
Summary: I realized I again is wrong about the naming convention. Let me change it to the correct one.

Test Plan: Run unit tests.

Reviewers: IslamAbdelRahman, kradhakrishnan, yhchiang, andrewkr

Reviewed By: andrewkr

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D55041
2016-03-04 18:15:29 -08:00
Yueh-Hsuan Chiang
bf1c4089db Use pure if-then check instead of assert in EraseColumnFamilyInfo
Summary:
Use pure if-then check instead of assert in EraseColumnFamilyInfo
when the specified column family does not found in the cf_info_map_.
So the second deletion will be no op instead of crash.

Test Plan: existing test.

Reviewers: sdong, anthony, kradhakrishnan, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55023
2016-03-04 16:03:31 -08:00
Yueh-Hsuan Chiang
a7d4eb2f34 Fix a bug where flush does not happen when a manual compaction is running
Summary:
Currently, when rocksdb tries to run manual compaction to refit data into a level,
there's a ReFitLevel() process that requires no bg work is currently running.
When RocksDB plans to ReFitLevel(), it will do the following:

 1. pause scheduling new bg work.
 2. wait until all bg work finished
 3. do the ReFitLevel()
 4. unpause scheduling new bg work.

However, as it pause scheduling new bg work at step one and waiting for all bg work
finished in step 2, RocksDB will stop flushing until all bg work is done (which
could take a long time.)

This patch fix this issue by changing the way ReFitLevel() pause the background work:

1. pause scheduling compaction.
2. wait until all bg work finished.
3. pause scheduling flush
4. do ReFitLevel()
5. unpause both flush and compaction.

The major difference is that.  We only pause scheduling compaction in step 1 and wait
for all bg work finished in step 2.  This prevent flush being blocked for a long time.
Although there's a very rare case that ReFitLevel() might be in starvation in step 2,
but it's less likely the case as flush typically finish very fast.

Test Plan: existing test.

Reviewers: anthony, IslamAbdelRahman, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55029
2016-03-04 14:24:52 -08:00
Gunnar Kudrjavets
68189f7e1b Update benchmarks used to measure subcompaction performance
Summary: After closely working with Mark, Siying, and Yueh-Hsuan this set of changes reflects the updates needed to measure RocksDB subcompaction performance in a correct manner. The essence of the benchmark is executing `fillrandom` followed by `compact` with the correct set of options for various number of subcompactions specified.

Test Plan: Tested internally to verify correctness and reliability.

Reviewers: sdong, yhchiang, MarkCallaghan

Reviewed By: MarkCallaghan

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D55089
2016-03-04 12:32:11 -08:00
Islam AbdelRahman
dfe96c72c3 Fix WriteLevel0TableForRecovery file delete protection
Summary:
The call to

```
CaptureCurrentFileNumberInPendingOutputs()
```

should be before

```
versions_->NewFileNumber()
```
Right now we are not actually protecting the file from being deleted

Test Plan: make check

Reviewers: sdong, anthony, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D54645
2016-03-03 18:25:07 -08:00
Igor Canadi
451678c8c9 Merge pull request #1025 from SherlockNoMad/BuildFix
Fix Windows build
2016-03-03 15:24:25 -08:00
SherlockNoMad
58ecd91326 Fix Windows build 2016-03-03 15:08:24 -08:00
Andrew Kryczka
501927ffc4 [backupable db] Remove file size embedded in name workaround
Summary:
Now that we get sizes efficiently, we no longer need the workaround to
embed file size in filename.

Test Plan:
  $ ./backupable_db_test

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D55035
2016-03-03 13:32:20 -08:00
sdong
ef204df7ef Compaction always needs to be removed from level0_compactions_in_progress_ for universal compaction
Summary: We always put compaction to level0_compactions_in_progress_ for universal compaction, so we should also remove it. The bug causes assert failure when running manual compaction.

Test Plan:
TEST_TMPDIR=/dev/shm/ ./db_bench --benchmarks=fillrandom,compact --subcompactions=16 --compaction_style=1
always fails on my host. After the fix, it doesn't fail any more.

Reviewers: IslamAbdelRahman, andrewkr, kradhakrishnan, yhchiang

Reviewed By: yhchiang

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D55017
2016-03-02 21:23:28 -08:00
sdong
e79ad9e184 Add Iterator Property rocksdb.iterator.version_number
Summary: We want to provide a way to detect whether an iterator is stale and needs to be recreated. Add a iterator property to return version number.

Test Plan: Add two unit tests for it.

Reviewers: IslamAbdelRahman, yhchiang, anthony, kradhakrishnan, andrewkr

Reviewed By: andrewkr

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D54921
2016-03-02 16:23:59 -08:00
sdong
19ea40f8b6 Subcompaction boundary keys should not terminate after an empty level
Summary: Now we skip to add boundary keys to subcompaction candidates since we see an empty level. This makes subcompaction almost disabled for universal compaction. We should consider all files instead.

Test Plan: Run existing tests.

Reviewers: IslamAbdelRahman, andrewkr, yhchiang

Reviewed By: yhchiang

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D55005
2016-03-02 15:45:07 -08:00