Commit Graph

4325 Commits

Author SHA1 Message Date
Islam AbdelRahman
5b9ce1a323 Merge pull request #820 from yuslepukhin/enable_compiler_warnings
Enable Windows warnings C4307 C4309 C4512 C4701
2015-11-06 12:08:25 -08:00
Dmitri Smirnov
20f57b1715 Enable Windows warnings C4307 C4309 C4512 C4701
Enable C4307 'operator' : integral constant overflow
  Longs and ints on Windows are 32-bit hence the overflow
  Enable C4309 'conversion' : truncation of constant value
  Enable C4512 'class' : assignment operator could not be generated
  Enable C4701 Potentially uninitialized local variable 'name' used
2015-11-06 11:34:06 -08:00
Islam AbdelRahman
8be568a9c2 Adding new table properties
Summary:
This diff introduce new table properties that will be written for block based tables
These properties are
  - comparator name
  - merge operator name
  - property collectors names

Test Plan:
  - Added a new unit test to verify that these tests are written/read correctly
  - Running all other tests right now (wont land until all tests finish)

Reviewers: rven, kradhakrishnan, igor, sdong, anthony, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D34269
2015-11-06 11:19:01 -08:00
Nathan Bronson
2b42000f43 incorrect batch group size computation for write throttling
Summary:
When a write batch can't join a batch group due to the total
size of the contained batches, the write controller's GetDelay is passed
a size value that includes the rejected batch.

Test Plan: make check

Reviewers: sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D50343
2015-11-06 09:23:55 -08:00
Nathan Bronson
c745f1d2c4 Fix TSAN build for fbcode
Summary:
TSAN builds for gcc 4.9 need a PIC version of the libraries
taken from the fbcode platform.  This is accomplished by assuming every
.a has a _pic.a sibling, and by fixing the third-party2 zlib build.

Test Plan: make COMPILE_WITH_TSAN=1 check

Reviewers: sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D50331
2015-11-06 09:05:12 -08:00
agiardullo
fe789c5f2b Document SingleDelete
Summary: Docuemented what is currently supported by SingleDelete based on its current implementation.

Test Plan: n/a

Reviewers: sdong, kradhakrishnan, yhchiang, rven

Reviewed By: rven

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D50205
2015-11-05 17:24:07 -08:00
Islam AbdelRahman
e89e5b253c Merge pull request #818 from yuslepukhin/improve_test_concurrency
Improve concurrency when running tests
2015-11-05 17:21:34 -08:00
Venkatesh Radhakrishnan
ae7940b628 Fix regression failure in PrefixTest.PrefixValid
Summary: Use IterKey to store prefix_start_ so that it doesn't get freed

Test Plan: PrefixTest.PrefixValid

Reviewers: anthony, IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D50289
2015-11-05 16:43:54 -08:00
Dmitri Smirnov
3277d172ba Improve concurrency when running tests
PowerShell seems to have a hard time when a flood of async tasks is
  scheduled at the same time.
  I speculated that WaitForMultipleObjects() in Windows can only take up
  to 64 process handles and if you want to handle more than you should write
  some additional code which can be sub-optimal. I.e to implement Wait-Job -Any.
  I decided to test that suggestion and introduced a $Concurrency parameter with a default value of 62.
  So in the new version the script fires up up to $Concurrency value
  and wait for anything to complete before starting any more processes.
  This improved matters greatly. Individual tests against ramdrive now
  run in 8 minutes and all of the 200+ db_tests run in 9 minutes with concurrency
  values of 8-16. About 48 is required to load a CPU on my box running against HD
  but that does not improve running times much.

  Other changes include respect -EnableJE for the individual test exes.
  Enforce exclusions for the individual tests.
2015-11-05 14:03:41 -08:00
Satnam Singh
c8e01ef982 Delete test iterators
Summary:
Valgrind reports an issue with the test for GeoIterator.
This diff explicitly deletes the two iterators used in this test.

Test Plan: This diff is for a test. The test still passes.

Reviewers: IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D50193
2015-11-05 13:30:51 -08:00
Venkatesh Radhakrishnan
9d50afc3b9 Prefix-based iterating only shows keys in prefix
Summary:
MyRocks testing found an issue that while iterating over keys
that are outside the prefix, sometimes wrong results were seen for keys
outside the prefix. We now tighten the range of keys seen with a new
read option called prefix_seen_at_start. This remembers the starting
prefix and then compares it on a Next for equality of prefix. If they
are from a different prefix, it sets valid to false.

Test Plan: PrefixTest.PrefixValid

Reviewers: IslamAbdelRahman, sdong, yhchiang, anthony

Reviewed By: anthony

Subscribers: spetrunia, hermanlee4, yoshinorim, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D50211
2015-11-05 13:24:05 -08:00
Igor Canadi
14c6e1a04c Add write_stress to RocksDB Legocastle runs
Summary: As title. Let's run it for 1 hour.

Test Plan: How can I test legocastle changes?

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

Reviewed By: kradhakrishnan

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D49653
2015-11-05 12:07:39 -08:00
Yueh-Hsuan Chiang
db3f5e4948 Update HISTORY.md
Summary:
Move some of the items in public API changes section to to new
features section.

Test Plan: no code change.

Reviewers: igor, sdong, anthony, IslamAbdelRahman

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D50229
2015-11-04 23:52:09 -08:00
Islam AbdelRahman
042fb053fd Fix clang
Summary: Fix build for clang

Test Plan:
USE_CLANG=1 make all -j64
make clean
make check -j64

Reviewers: yhchiang

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D50217
2015-11-04 21:02:20 -08:00
Islam AbdelRahman
2419f435a1 Merge pull request #816 from SherlockNoMad/GeoDBTestFix
Fix appveyor build failure
2015-11-04 20:40:57 -08:00
SherlockNoMad
2e45409910 Fix appveyor build failure 2015-11-04 20:10:16 -08:00
Yueh-Hsuan Chiang
183cadfc87 Add OptionsSanityCheckLevel
Summary:
This patch introduces OptionsSanityCheckLevel internally to enable
sanity check rocksdb options.

Utilities API will be added in the follow-up diffs.

Test Plan: Added more tests in options_test

Reviewers: igor, IslamAbdelRahman, sdong, anthony

Reviewed By: anthony

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D49515
2015-11-04 18:53:30 -08:00
Yueh-Hsuan Chiang
dba5e00741 Fixed the compile error in RocksDBLite in memory_test.cc
Summary:
Fixed the following compile error in RocksDBLite:

18:00:33   CC       utilities/memory/memory_test.o
18:00:33 utilities/memory/memory_test.cc: In function ‘int main(int, char**)’:
18:00:33 utilities/memory/memory_test.cc:268:66: error: ‘printf’ was not declared in this scope
18:00:33    printf("Skipped in RocksDBLite as utilities are not supported.");
18:00:33                                                                   ^

Test Plan: make OPT=-DROCKSDB_LITE memory_test

Reviewers: igor, sdong, anthony, IslamAbdelRahman

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D50145
2015-11-03 18:06:23 -08:00
Yueh-Hsuan Chiang
7d7ee2b654 Add Memory Insight support to utilities
Summary:
This patch introduces utilities/memory, which currently includes
GetApproximateMemoryUsageByType that reports different types of
rocksdb memory usage given a list of input DBs.

The API also take care of the case where Cache could be shared
across multiple column families / multiple db instances.

Currently, it reports memory usage of memtable, table-readers
and cache.

Test Plan: utilities/memory/memory_test.cc

Reviewers: igor, anthony, IslamAbdelRahman, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D49257
2015-11-03 17:52:17 -08:00
Yueh-Hsuan Chiang
3ecbab0040 Add GetAggregatedIntProperty(): returns the aggregated value from all CFs
Summary:
This patch adds GetAggregatedIntProperty() that returns the aggregated
value from all CFs

Test Plan: Added a test in db_test

Reviewers: igor, sdong, anthony, IslamAbdelRahman, rven

Reviewed By: rven

Subscribers: rven, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D49497
2015-11-03 15:54:18 -08:00
Satnam Singh
93a9667223 Merge branch 'master' of github.com:facebook/rocksdb 2015-11-03 15:36:14 -08:00
Satnam Singh
c9aef3c41c Add RocksDb/GeoDb Iterator interface
Summary:
This diff is a first step towards an iterator based interface for the
SearchRadial method which replaces a vector of GeoObjects with an
iterator for GeoObjects. This diff works by just wrapping the iterator
for the encapsulated vector of GeoObjects. A future diff could extend
this approach by defining an interator in terms of the underlying
iteration in SearchRadial which would then remove the need to have
an in-memory representation for all the matching GeoObjects.
Fixes T8421387

Test Plan:
The existing tests have been modified to work with the new
interface.

Reviewers: IslamAbdelRahman, kradhakrishnan, dhruba, igor

Reviewed By: igor

Subscribers: igor, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D50031
2015-11-03 15:20:58 -08:00
Islam AbdelRahman
f31442fb5c Merge pull request #803 from SherlockNoMad/SkipFlush
Add Option to Skip Flushing in TableBuilder
2015-11-02 14:56:11 -08:00
Igor Canadi
dcc898b021 Merge pull request #812 from yuslepukhin/fix_windows_warnings
Enable compiler warnings
2015-11-02 14:22:33 -08:00
SherlockNoMad
df7ed91ef9 Fix white space at end of line 2015-11-02 14:12:29 -08:00
Dmitri Smirnov
a0163c0682 Do not disable compiler warnings:
C4101 'identifier' : unreferenced local variable
  C4189 'identifier' : local variable is initialized but not referenced
  C4100 'identifier' : unreferenced formal parameter
  C4296 'operator' : expression is always false
2015-11-02 14:11:28 -08:00
Igor Canadi
279c8e0cd8 Merge pull request #811 from OverlordQ/unused-variable-warning
Fix introduced in 2ab7065 was reverted by 18285c1.
2015-11-02 12:44:27 -08:00
Brent Garber
affd833690 Fix introduced in 2ab7065 was reverted by 18285c1.
Corrects:

db/memtablerep_bench.cc:135:22: error: ‘FLAGS_env’ defined but not used [-Werror=unused-variable]
 static rocksdb::Env* FLAGS_env = rocksdb::Env::Default();
                      ^
cc1plus: all warnings being treated as errors
Makefile:1147: recipe for target 'db/memtablerep_bench.o' failed
2015-11-02 15:35:45 -05:00
Igor Canadi
db68a2c09e Merge pull request #806 from yuslepukhin/signed_unsigned_warning
Do not suppress C4018 'expression' : signed/unsigned mismatch
2015-11-02 10:09:47 -08:00
SherlockNoMad
ccc8c10c0c Move skip_table_builder_flush to BlockBasedTableOption 2015-10-30 18:33:01 -07:00
Dmitri Smirnov
eaaf081d16 Do not suppress C4018 'expression' : signed/unsigned mismatch
The code compiles cleanly for the most part. Fix db_test.
  Move debug file to testutil library.
2015-10-30 17:03:16 -07:00
Islam AbdelRahman
ff4499e297 Update DB::AddFile() to have less restrictions
Summary:
Update DB::AddFile() restrictions to be
  - Key range in loaded table file don't overlap with existing keys or tombstones in DB.
  - No other writes happen during AddFile call.

The updated AddFile() will verify that the file key range don't overlap with any keys or tombstones in the DB, and then add the file to L0

Test Plan: unit tests

Reviewers: igor, rven, anthony, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: adsharma, ameyag, dhruba

Differential Revision: https://reviews.facebook.net/D49233
2015-10-30 16:38:10 -07:00
sdong
11c71a365a db_bench: --compaction_pri default should be rocksdb::Options().compaction_pri
Summary: Currently db_bnech's --compaction_pri default is set to be rocksdb::Options().compaction_style. Change it to rocksdb::Options().compaction_pri. Although, for now both is 0.

Test Plan: Build db_bench

Reviewers: anthony, rven, IslamAbdelRahman, kradhakrishnan, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D49773
2015-10-30 15:02:33 -07:00
sdong
335e4ce8ca options_test: fix a bug of assertion
Summary: new_cf_opt.table_factory->Name() is char*, ASSERT_EQ doesn't work with char* directly. Construct a string using it.

Test Plan: Run the test that failed.

Reviewers: igor, kradhakrishnan, rven, IslamAbdelRahman, anthony

Reviewed By: anthony

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D49767
2015-10-30 11:53:47 -07:00
Siying Dong
66a3a87ab3 Merge pull request #797 from SherlockNoMad/optionHelper
Support PlainTableOption in option_helper.cc
2015-10-30 09:44:11 -07:00
SherlockNoMad
550af4ee68 Fix Travis Build Error 2015-10-29 22:41:57 -07:00
SherlockNoMad
a6dd0831d5 Add Option to Skip Flushing in TableBuilder 2015-10-29 22:10:25 -07:00
Islam AbdelRahman
2872e0c8c2 Clean and expose CreateLoggerFromOptions
Summary:
CreateLoggerFromOptions have some parameters like  db_log_dir and env, these parameters are redundant since they already exist in DBOptions

this patch remove the redundant parameters and expose CreateLoggerFromOptions to users

Test Plan: make check

Reviewers: igor, anthony, yhchiang, rven, kradhakrishnan, sdong

Reviewed By: sdong

Subscribers: dhruba, hermanlee4

Differential Revision: https://reviews.facebook.net/D49713
2015-10-29 18:07:37 -07:00
sdong
296c3a1f94 "make format" in some recent commits
Summary: Run "make format" for some recent commits.

Test Plan: Build and run tests

Reviewers: IslamAbdelRahman

Reviewed By: IslamAbdelRahman

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D49707
2015-10-29 17:11:14 -07:00
Siying Dong
6388e7f4e2 Merge pull request #798 from yuslepukhin/readahead_buffermanagement
Implement smart buffer management in Windows Env.
2015-10-29 15:02:59 -07:00
Islam AbdelRahman
f4cbb90c41 Merge pull request #799 from yuslepukhin/fix_random_generator_compile
Fix compilation problem on Windows.
2015-10-29 13:41:36 -07:00
Dmitri Smirnov
1277a48f1b Fix 80 character limit issue. 2015-10-29 11:34:34 -07:00
Dmitri Smirnov
ee2c3236dd Fix compilation problem on Windows.
char is not a valid template parameter for std::uniform_int_distribution
  according to the standard. Replacing with int should be just fine.
2015-10-29 11:29:18 -07:00
SherlockNoMad
b69b9b624e Support PlainTableOption in option_helper 2015-10-28 23:01:33 -07:00
Igor Canadi
c97667d9f1 Fix RocksDB lite build for write_stress
Summary: We don't have access to GetLiveFilesMetadata() in RocksDB lite. If compiling write_stress for lite, I skip the check for leaked files, which depends on this function.

Test Plan: OPT=-DROCKSDB_LITE m write_stress

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D49647
2015-10-28 16:37:39 -07:00
Herman Lee
0d720dfc17 Use the correct variable when fetching table properties.
Summary:
An uninitialized parameter was being passed into the call to fetch the table
properties during the compaction notification callbacks.

Test Plan:
Build it with myrocks and verify unit test passed.
Run unit tests.

Reviewers: rven, yhchiang, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D49635
2015-10-28 16:28:11 -07:00
Igor Canadi
4b66d95344 Write stress test
Summary:
The goal of this diff is to create a simple stress test with focus on catching:
* bugs in compaction/flush processes, especially the ones that cause assertion errors
* bugs in the code that deletes obsolete files

There are two parts of the test:
* write_stress, a binary that writes to the database
* write_stress_runner.py, a script that invokes and kills write_stress

Here are some interesting parts of write_stress:
* Runs with very high concurrency of compactions and flushes (32 threads total) and tries to create a huge amount of small files
* The keys written to the database are not uniformly distributed -- there is a 3-character prefix that mutates occasionally (in prefix mutator thread), in such a way that the first character mutates slower than second, which mutates slower than third character. That way, the compaction stress tests some interesting compaction features like trivial moves and bottommost level calculation
* There is a thread that creates an iterator, holds it for couple of seconds and then iterates over all keys. This is supposed to test RocksDB's abilities to keep the files alive when there are references to them.
* Some writes trigger WAL sync. This is stress testing our WAL sync code.
* At the end of the run, we make sure that we didn't leak any of the sst files

write_stress_runner.py changes the mode in which we run write_stress and also kills and restarts it. There are some interesting characteristics:
* At the beginning we divide the full test runtime into smaller parts -- shorter runtimes (couple of seconds) and longer runtimes (100, 1000) seconds
* The first time we run write_stress, we destroy the old DB. Every next time during the test, we use the same DB.
* We can run in kill mode or clean-restart mode. Kill mode kills the write_stress violently.
* We can run in mode where delete_obsolete_files_with_fullscan is true or false
* We can run with low_open_files mode turned on or off. When it's turned on, we configure table cache to only hold a couple of files -- that way we need to reopen files every time we access them.

Another goal was to create a stress test without a lot of parameters. So tools/write_stress_runner.py should only take one parameter -- runtime_sec and it should figure out everything else on its own.

In a separate diff, I'll add this new test to our nightly legocastle runs.

Test Plan:
The goal of this test was to retroactively catch the following bugs: D33045, D48201, D46899, D42399. I failed to reproduce D48201, but all others have been caught!

When i reverted https://reviews.facebook.net/D33045:

     ./write_stress --runtime_sec=200 --low_open_files_mode=true
     Iterator statuts not OK: IO error: /fast-rocksdb-tmp/rocksdb_test/write_stress/089166.sst: No such file or directory

When i reverted https://reviews.facebook.net/D42399:

    python tools/write_stress_runner.py --runtime_sec=5000
    Running write_stress, will kill after 5 seconds: ./write_stress --runtime_sec=-1
    Running write_stress, will kill after 2 seconds: ./write_stress --runtime_sec=-1 --destroy_db=false --delete_obsolete_files_with_fullscan=true
    Running write_stress, will kill after 7 seconds: ./write_stress --runtime_sec=-1 --destroy_db=false
    Running write_stress, will kill after 5 seconds: ./write_stress --runtime_sec=-1 --destroy_db=false
    Running write_stress, will kill after 8 seconds: ./write_stress --runtime_sec=-1 --destroy_db=false --low_open_files_mode=true
    Write to DB failed: IO error: /fast-rocksdb-tmp/rocksdb_test/write_stress/019250.sst: No such file or directory
    ERROR: write_stress died with exitcode=-6

When i reverted https://reviews.facebook.net/D46899:

    python tools/write_stress_runner.py --runtime_sec=1000
    runtime: 1000
    Going to execute write stress for [3, 3, 100, 3, 2, 100, 1, 788]
    Running write_stress for 3 seconds: ./write_stress --runtime_sec=3 --low_open_files_mode=true
    Running write_stress for 3 seconds: ./write_stress --runtime_sec=3 --destroy_db=false --delete_obsolete_files_with_fullscan=true
    Running write_stress, will kill after 100 seconds: ./write_stress --runtime_sec=-1 --destroy_db=false --delete_obsolete_files_with_fullscan=true
    write_stress: db/db_impl.cc:2070: void rocksdb::DBImpl::MarkLogsSynced(uint64_t, bool, const rocksdb::Status&): Assertion `log.getting_synced' failed.
    ERROR: write_stress died with exitcode=-6

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

Reviewed By: anthony

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D49533
2015-10-28 16:15:07 -07:00
sdong
47414c6cd6 Move include/posix/io_posix.h to util/io_posix.h
Summary: include/posix/io_posix.h is not a public API. Although include/posix/ is not a public header directory, it is confusing to put non-public headers to under include/. Move it to util/ to be clearer.

Test Plan: Run all tests

Reviewers: rven, IslamAbdelRahman, anthony, kradhakrishnan, yhchiang, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D49611
2015-10-28 12:15:51 -07:00
sdong
2889df84cb Revert "Avoid to reply on ROCKSDB_FALLOCATE_PRESENT in include/posix/io_posix.h"
This reverts commit c37223c083.
2015-10-28 11:55:20 -07:00
Siying Dong
28c8758a34 Merge pull request #795 from yuslepukhin/fix_mocktable_id
Fix MockTable ID storage
2015-10-28 11:37:37 -07:00