Commit Graph

3634 Commits

Author SHA1 Message Date
Mark Callaghan
283a042969 Set --seed per test
Summary:
This is done to avoid having each thread use the same seed between runs
of db_bench. Without this we can inflate the OS filesystem cache hit rate on
reads for read heavy tests and generally see the same key sequences get generated
between teste runs.

Task ID: #

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37563
2015-04-23 09:18:25 -07:00
Venkatesh Radhakrishnan
618d07b068 Making PreShutdown tests more reliable.
Summary:
A couple of times on Travis, we have had the thread status say that there were no compactions done and since we assert for it, the test failed.
We now fix this by waiting till compaction started.

Test Plan:
run DBTEST::*PreShutdown*

d=/tmp/j; rm -rf $d; seq 200 | parallel --gnu --eta 'd=/tmp/j/d-{}; mkdir -p $d; TEST_TMPDIR=$d ./db_test --gtest_filter=DBTest.PreShutdown* >& '$d'/log-{}'

Reviewers: sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37545
2015-04-23 08:35:02 -07:00
Jim Meyering
0a91bca5db test: avoid vuln-inducing use of temporary directory
Summary:
Without this change, someone on the machine on which
I run "make check" could cause me to overwrite arbitrary
files owned by me, via a symlink attack.

Instead of using a predictable temporary directory and
accepting to use a preexisting one, always create a new
one using mkdtemp.  If $TEST_IOCTL_FRIENDLY_TMPDIR is
set and usable, attempt first to find a usable
temporary directory therein.  If not, or if unusable,
then try /var/tmp and /tmp.  If none of those is usable
abort with a diagnostic.

To do that, I added a new class.
Its constructor finds a suitable directory or aborts,
the sole member prints that directory's name, and the
destructor unlinks what should be an empty directory.

Note that while the code before this did not remove
its temporary directory, there was only one per $UID.
Now, there would be at least one per run or one per
test, depending on implementation, so it is important
to remove them.

Test Plan:
  Run this on a fedora rawhide system, where /tmp
  is a tmpfs file system, and /var/tmp is ext4.

  # This gives a diagnostic that /dev/shm is not suitable
  # and ends up using /var/tmp.
  TEST_IOCTL_FRIENDLY_TMPDIR=/dev/shm ./env_test

  # Uses /var/tmp; same as when envvar not set.
  TEST_IOCTL_FRIENDLY_TMPDIR=/var/tmp ./env_test

  # Uses /tmp unless it's tmpfs, in which case it gives
  # a diagnostic and uses /var/tmp.
  TEST_IOCTL_FRIENDLY_TMPDIR=/tmp ./env_test

Reviewers: ljin, rven, igor.sugak, yhchiang, sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37287
2015-04-23 08:00:56 -07:00
Mark Callaghan
6e359419fe Add rpath for production builds
Summary:
This lets the production toolchain libraries get used on devservers and
in production.

Task ID: #6849362

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37533
2015-04-22 17:17:50 -07:00
Mark Callaghan
78dbd087d1 Improve benchmark scripts
Summary:
This adds:
1) use of --level_compaction_dynamic_level_bytes=true
2) use of --bytes_per_sync=2M
The second is a big win for disks. The first helps in general.

This also adds a new test, fillseq with 32kb values to increase the peak
ingest and make it more likely that storage limits throughput.

Sample outpout from the first 3 tests - https://gist.github.com/mdcallag/e793bd3038e367b05d6f

Task ID: #

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37509
2015-04-22 13:23:08 -07:00
Igor Canadi
6a5ffee0cc Fix gflags Makefile
Summary: `echo` correctly interpretes \n on mac, but not on linux. On linux you have to give it `-e` to interpret \n. Unfortunately, `-e` options is not available on Mac. Go back to old way of checking gflags

Test Plan: build_tools/build_detect_platform on mac and linux

Reviewers: sdong

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37515
2015-04-22 12:50:28 -07:00
Siying Dong
108a927f0e Merge pull request #589 from coderplay/patch-1
Add "Tango Me" section in USERS.md
2015-04-21 14:57:36 -07:00
Min Zhou
a58fd74276 Update USERS.md 2015-04-21 14:53:09 -07:00
Igor Canadi
d85d08c7b3 One last fix to Makefile
Summary: Based on comment from D37455

Test Plan: make install after make static_lib

Reviewers: meyering

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37461
2015-04-20 20:46:08 -07:00
Igor Canadi
2db96dca18 Fix make install when there is no shared lib
Summary: make install fails when there is no shared lib. We need to revert the conditions, which will have the same effect, but without the failure

Test Plan: make install after only compiling static library

Reviewers: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37455
2015-04-20 20:39:54 -07:00
Igor Canadi
7d136994c9 Get rid of error output
Summary: We should send error output to /dev/null

Test Plan: none

Reviewers: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37449
2015-04-20 19:44:32 -07:00
Igor Canadi
79c1b021a7 Fix Makefile
Summary: The current version tries to install librocksdb.so even though it doesn't exist. This version will install librocksdb.so.3.10.0 and then create soft links in place

Test Plan:
`make static_lib; sudo make install` does not try to install librocksdb.so
`make shared_lib; sudo make install` installs one library and 3 symlinks. Before, four libraries were installed

Reviewers: sdong, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37425
2015-04-20 19:39:51 -07:00
Igor Canadi
742fa9e316 Fix compile with two gflags
Summary:
If the system has gflags with both `google` and `gflags` namespaces installed, we try to define GFLAGS as two things. This breaks the compile.

Fix: Use `else if` -- try compiling with `google` namespace only if compile with `gflags` failed

Test Plan: build_tools/build_detect_platform correctly identifies gflags

Reviewers: lgalanis

Reviewed By: lgalanis

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37389
2015-04-20 10:55:17 -07:00
Jim Meyering
79c21ec0c4 skip ioctl-using tests when not supported
Summary:
[NB: this is a prerequisite for the /tmp-abuse-fixing patch]
This avoids spurious test failure on Linux systems
like Fedora for which /tmp is a tmpfs file system.

On a devtmpfs file
system, ioctl(fd, FS_IOC_GETVERSION, &version) returns -1 with
errno == ENOTTTY, indicating that that ioctl is not supported
on such a file system.  Do not let this cause test failures, e.g.,
where env_test would assert that file->GetUniqueId(...) > 0.

Before this change, ./env_test would fail these three tests
on a fedora rawhide system:

  [  FAILED  ] 3 tests, listed below:
  [  FAILED  ] EnvPosixTest.RandomAccessUniqueID
  [  FAILED  ] EnvPosixTest.RandomAccessUniqueIDConcurrent
  [  FAILED  ] EnvPosixTest.RandomAccessUniqueIDDeletes
   3 FAILED TESTS

The fix:
  When support for that ioctl is lacking, skip each affected test.
  Could be improved by noting which sub-tests are being skipped.

Test Plan:
run these on F21 and note that they now pass.

  TEST_TMPDIR=/dev/shm/rdb ./env_test
  ./env_test

Reviewers: ljin, rven, igor.sugak, yhchiang, sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37323
2015-04-17 20:39:02 -07:00
Igor Canadi
6059bdf86a Add experimental API MarkForCompaction()
Summary:
Some Mongo+Rocks datasets in Parse's environment are not doing compactions very frequently. During the quiet period (with no IO), we'd like to schedule compactions so that our reads become faster. Also, aggressively compacting during quiet periods helps when write bursts happen. In addition, we also want to compact files that are containing deleted key ranges (like old oplog keys).

All of this is currently not possible with CompactRange() because it's single-threaded and blocks all other compactions from happening. Running CompactRange() risks an issue of blocking writes because we generate too much Level 0 files before the compaction is over. Stopping writes is very dangerous because they hold transaction locks. We tried running manual compaction once on Mongo+Rocks and everything fell apart.

MarkForCompaction() solves all of those problems. This is very light-weight manual compaction. It is lower priority than automatic compactions, which means it shouldn't interfere with background process keeping the LSM tree clean. However, if no automatic compactions need to be run (or we have extra background threads available), we will start compacting files that are marked for compaction.

Test Plan: added a new unit test

Reviewers: yhchiang, rven, MarkCallaghan, sdong

Reviewed By: sdong

Subscribers: yoshinorim, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37083
2015-04-17 16:44:45 -07:00
Jim Meyering
acf8a4141d maint: use ASSERT_TRUE, not ASSERT_EQ(true; same for false
Summary:
The usage I'm fixing here caused trouble on Fedora 21 when
compiling with the current gcc version 4.9.2 20150212 (Red Hat 4.9.2-6) (GCC):

  db/write_controller_test.cc: In member function ‘virtual void rocksdb::WriteControllerTest_SanityTest_Test::TestBody()’:
  db/write_controller_test.cc:23:165: error: converting ‘false’ to pointer type for argument 1 of ‘char testing::internal::IsNullLiteralHelper(testing::internal::Secret*)’ [-Werror=conversion-null]
     ASSERT_EQ(false, controller.IsStopped());
                                                                                                                                                                          ^

This change was induced mechanically via:

  git grep -l -E 'ASSERT_EQ\(false'|xargs perl -pi -e 's/ASSERT_EQ\(false, /ASSERT_FALSE(/'
  git grep -l -E 'ASSERT_EQ\(true'|xargs perl -pi -e 's/ASSERT_EQ\(true, /ASSERT_TRUE(/'

Except for the three in utilities/backupable/backupable_db_test.cc for which
I ended up reformatting (joining lines) in the result.

As for why this problem is exhibited with that version of gcc, and none
of the others I've used (from 4.8.1 through gcc-5.0.0 and newer), I suspect
it's a bug in F21's gcc that has been fixed in gcc-5.0.0.

Test Plan:
  "make" now succeed on Fedora 21

Reviewers: ljin, rven, igor.sugak, yhchiang, sdong, igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37329
2015-04-17 14:54:17 -07:00
Igor Canadi
b5400f90fe Kill dead code
Summary: this is not used anywhere

Test Plan: compiles

Reviewers: yhchiang, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37053
2015-04-17 12:07:47 -07:00
Igor Canadi
48b0a045da Speed up reduce_levels_test
Summary: For some reason reduce_levels is opening the databse with 65.000 levels. This makes ComputeCompactionScore() function terribly slow and the tests is also very slow (20seconds).

Test Plan: mr reduce_levels_test now takes 20ms

Reviewers: sdong, rven, kradhakrishnan, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37059
2015-04-16 19:31:34 -07:00
Igor Canadi
00c2afcd38 Fix bug in ExpandWhileOverlapping()
Summary: If ExpandWhileOverlapping() we don't clear inputs. That's a bug introduced by my recent patch https://reviews.facebook.net/D36687. However, we have no tests covering ExpandWhileOverlapping(). I created a task t6771252 to add ExpandWhileOverlapping() tests.

Test Plan: make check

Reviewers: sdong, rven, yhchiang

Reviewed By: yhchiang

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D37077
2015-04-16 19:31:10 -07:00
fyrz
019ecd1932 [RocksJava] Remove deprecated methods
Summary:
- Removed deprecated ColumnFamilyDescript constructor methods
- Removed deprecated skipLogErrorOnRecovery methods
- Removed deprecated tableCacheRemoveScanCountLimit methods

Test Plan:
make rocksdbjava
make jtest

Reviewers: yhchiang, adamretter, ankgup87

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D37191
2015-04-16 00:31:43 +02:00
Igor Canadi
98ef21d2ff Merge pull request #584 from pshareghi/rocksdb-3.10-falloch
Added falloc.h in build_detect_platform
2015-04-15 14:56:36 -07:00
fyrz
5b7131c751 [RocksJava] Removed deprecated skipLogErrorOnRecovery methods.
As annonunced these options are not used anymore so these are
removed from code.
2015-04-15 23:37:24 +02:00
fyrz
566f652716 [RocksJava] Removed deprecated ColumnFamilyDescriptor methods
As announced previously removed methods are obsolete and will
be replaced by its byte array pendants.
2015-04-15 23:31:59 +02:00
Igor Canadi
6997aa0b6b Merge pull request #582 from fyrz/RocksJava-Fix-RateLimiter
[RocksJava] Fix RateLimiter Tests in 3.10
2015-04-15 14:23:29 -07:00
fyrz
582c4b0f74 [RocksJava] Fix RateLimiter Tests in 3.10 2015-04-15 23:19:41 +02:00
Igor Canadi
6cfb2150d4 Merge pull request #581 from vladb38/patch-3
Update USERS.md
2015-04-15 13:36:27 -07:00
Vlad Balan
d71e8f7adf Update USERS.md 2015-04-15 13:25:52 -07:00
sdong
debaf85ef5 Bug of trivial move of dynamic level
Summary: D36669 introduces a bug that trivial moved data is not going to specific level but the next level, which will incorrectly be level 1 for level 0 compaciton if base level is not level 1. Fixing it by appreciating the output level

Test Plan: Run all tests

Reviewers: MarkCallaghan, rven, yhchiang, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D37119
2015-04-14 21:42:08 -07:00
sdong
12d7d3d28d Fix and Improve DBTest.DynamicLevelCompressionPerLevel2
Summary:
Recent change of DBTest.DynamicLevelCompressionPerLevel2 has a bug that the second sync point is not enabled. Fix it. Also add an assert for that.
Also, flush compression is not tracked in the test. Add it.

Test Plan: Build everything

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D37101
2015-04-14 21:42:08 -07:00
sdong
a1271c6c6f Fix build break introduced by new SyncPoint interface change
Summary: When commiting the sync point interface change, didn't resolve the new occurance of the old interface in rebase. Fix it.

Test Plan: Build and see it pass

Reviewers: igor, yhchiang, rven, anthony, kradhakrishnan

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D37095
2015-04-14 16:42:37 -07:00
sdong
fcb206b667 SyncPoint to allow a callback with an argument and use it to get DBTest.DynamicLevelCompressionPerLevel2 more straight-forward
Summary:
Allow users to give a callback function with parameter using sync point, so more complicated verification can be done in tests.
Use it in DBTest.DynamicLevelCompressionPerLevel2 so that failures will be more easy to debug.

Test Plan: Run all tests. Run DBTest.DynamicLevelCompressionPerLevel2 with valgrind check.

Reviewers: rven, yhchiang, anthony, kradhakrishnan, igor

Reviewed By: igor

Subscribers: leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D36999
2015-04-14 16:18:50 -07:00
Igor Canadi
281db8bb62 Temporarily disable test CompactFilesOnLevelCompaction
Summary: https://reviews.facebook.net/D36963 made the debug build much faster and that triggered failures of CompactFilesOnLevelCompaction test. 3 out of 4 last tests on Jenkins failed. I'm disabling this test temporarily, since we likely know the reason why it's failing and there's already work in progress to address it -- https://reviews.facebook.net/D36225

Test Plan: none

Reviewers: sdong, rven, yhchiang, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36993
2015-04-13 19:30:40 -07:00
Pooya Shareghi
e8808b9128 Added falloc.h in build_detect_platform
On Centos 6, you need to explicitely include linux/falloc.h which is
whele the  FALLOC_FL_* flags are defined. Otherwise, the fallocate()
support test defined in build_detect_platform will fail.

Signed-off-by: Pooya Shareghi <shareghi@gmail.com>
2015-04-13 17:56:12 -07:00
Igor Canadi
1983fadcbc assert(sorted) in vector rep
Summary: based on discussion on https://reviews.facebook.net/D36969

Test Plan: will let jenkins do its job

Reviewers: sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36975
2015-04-13 17:33:24 -07:00
Mark Callaghan
9da8748016 Get benchmark.sh loads to run faster
Summary:
This changes loads to use vector memtable and disable the WAL. This also
increases the chance we will see IO bottlenecks during loads which is good to stress
test HW. But I also think it is a good way to load data quickly as this is a bulk
operation and the WAL isn't needed.

The two numbers below are the MB/sec rates for fillseq, bulkload using a skiplist
or vector memtable and the WAL enabled or disabled. There is a big benefit from
using the vector memtable and WAL disabled. Alas there is also a perf bug in
the use of std::sort for ordered input when the vector is flushed. Task is open
for that.
  112, 66 - skiplist with wal
  250, 116 - skiplist without wal
  110, 108 - vector with wal
  232, 370 - vector without wal

Task ID: #

Blame Rev:

Test Plan:
Revert Plan:

Database Impact:

Memcache Impact:

Other Notes:

EImportant:

- begin *PUBLIC* platform impact section -
Bugzilla: #
- end platform impact -

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D36957
2015-04-13 17:18:07 -07:00
Igor Canadi
9b983befa8 Fix flakiness of WalManagerTest
Summary: We should use mocked-out env for these tests to make it more realiable. Added benefit is that instead of actually sleeping for 3 seconds, we can instead pretend to sleep and just increase time counters.

Test Plan: for i in `seq 100`; do ./wal_manager_test --gtest_filter=WalManagerTest.WALArchivalTtl ;done

Reviewers: rven, meyering

Reviewed By: meyering

Subscribers: meyering, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36951
2015-04-13 16:15:05 -07:00
Igor Canadi
d41a565a4a Don't do O(N^2) operations in debug mode for vector memtable
Summary: As title. For every operation we're asserting Valid(), which sorts the data. That's pretty terrible. We have to be careful to have decent performance even with DEBUG builds.

Test Plan: make check

Reviewers: sdong, rven, yhchiang, MarkCallaghan

Reviewed By: MarkCallaghan

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36969
2015-04-13 16:11:47 -07:00
Igor Canadi
08be1803ee Fix bad performance in debug mode
Summary:
See github issue 574: https://github.com/facebook/rocksdb/issues/574

Basically when we're running in DEBUG mode we're calling `usleep(0)` on
every mutex lock. I bisected the issue to
https://reviews.facebook.net/D36963. Instead of calling sleep(0), this
diff just avoids calling SleepForMicroseconds() when delay is not set.

Test Plan:
    bpl=10485760;overlap=10;mcz=2;del=300000000;levels=2;ctrig=10000000; delay=10000000; stop=10000000; wbn=30; mbc=20; mb=1073741824;wbs=268435456; dds=1; sync=0; r=100000; t=1; vs=800; bs=65536; cs=1048576; of=500000; si=1000000; ./db_bench --benchmarks=fillrandom --disable_seek_compaction=1 --mmap_read=0 --statistics=1 --histogram=1 --num=$r --threads=$t --value_size=$vs --block_size=$bs --cache_size=$cs --bloom_bits=10 --cache_numshardbits=4 --open_files=$of --verify_checksum=1 --db=/tmp/rdb10test --sync=$sync --disable_wal=1 --compression_type=snappy --stats_interval=$si --compression_ratio=0.5 --disable_data_sync=$dds --write_buffer_size=$wbs --target_file_size_base=$mb --max_write_buffer_number=$wbn --max_background_compactions=$mbc --level0_file_num_compaction_trigger=$ctrig --level0_slowdown_writes_trigger=$delay --level0_stop_writes_trigger=$stop --num_levels=$levels --delete_obsolete_files_period_micros=$del --min_level_to_compress=$mcz --max_grandparent_overlap_factor=$overlap --stats_per_interval=1 --max_bytes_for_level_base=$bpl --memtablerep=vector --use_existing_db=0 --disable_auto_compactions=1 --source_compaction_factor=10000000 | grep ops

Before:
fillrandom   :     117.525 micros/op 8508 ops/sec;    6.6 MB/s
After:
fillrandom   :       1.283 micros/op 779502 ops/sec;  606.6 MB/s

Reviewers: rven, yhchiang, sdong

Reviewed By: sdong

Subscribers: meyering, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36963
2015-04-13 15:58:45 -07:00
Venkatesh Radhakrishnan
0a0501c8d5 Add Xfunc to makefile
Summary: Make target for running all xfunc tests

Test Plan: make xfunc

Reviewers: igor, sdong, meyering

Reviewed By: meyering

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36873
2015-04-13 14:21:32 -07:00
Igor Canadi
e7ad14926a Fix flakiness in FIFOCompaction test (github issue #573)
Summary:
The problem is that sometimes two memtables will be compacted together into a single file. In that case, our assertion

        ASSERT_EQ(NumTableFilesAtLevel(0), 5);

fails because same amount of data is in 4 files instead of 5. We should wait for flush so that we prevent two memtables merging into a single file.

Test Plan: `for i in `seq 20`; do mrtest FIFOCompactionTest; done` -- fails at least once before. fails zero times after.

Reviewers: rven

Reviewed By: rven

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36939
2015-04-13 11:39:45 -07:00
Igor Canadi
abb4052278 Kill benchharness
Summary:
1. it doesn't work
2. we're not using it

In the future, if we need general benchmark framework, we should probably use https://github.com/google/benchmark

Test Plan: make all

Reviewers: yhchiang, rven, anthony, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36777
2015-04-13 10:17:42 -07:00
Dhruba Borthakur
894e9f7454 Update Patent Grant.
Summary:
https://code.facebook.com/posts/1639473982937255/updating-our-open-source-patent-grant/

This has been done by other FB's open source projects already:
b8ba8c83f3
https://github.com/facebook/osquery/blob/master/PATENTS

Test Plan: make check

Reviewers: igor, sdong

Reviewed By: sdong

Subscribers: jamesgpearce, leveldb, dhruba

Differential Revision: https://reviews.facebook.net/D36879
2015-04-13 10:33:43 +01:00
Igor Canadi
590fadc407 Fix compile warning on CLANG
Summary: oops

Test Plan: compiles now

Reviewers: sdong, yhchiang

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36867
2015-04-10 15:14:57 -07:00
Igor Canadi
47b8743984 Make Compaction class easier to use
Summary:
The goal of this diff is to make Compaction class easier to use. This should also make new compaction algorithms easier to write (like CompactFiles from @yhchiang and dynamic leveled and multi-leveled universal from @sdong).

Here are couple of things demonstrating that Compaction class is hard to use:
1. we have two constructors of Compaction class
2. there's this thing called grandparents_, but it appears to only be setup for leveled compaction and not compactfiles
3. it's easy to introduce a subtle and dangerous bug like this: D36225
4. SetupBottomMostLevel() is hard to understand and it shouldn't be. See this comment: afbafeaeae/db/compaction.cc (L236-L241). It also made it harder for @yhchiang to write CompactFiles, as evidenced by this: afbafeaeae/db/compaction_picker.cc (L204-L210)

The problem is that we create Compaction object, which holds a lot of state, and then pass it around to some functions. After those functions are done mutating, then we call couple of functions on Compaction object, like SetupBottommostLevel() and MarkFilesBeingCompacted(). It is very hard to see what's happening with all that Compaction's state while it's travelling across different functions. If you're writing a new PickCompaction() function you need to try really hard to understand what are all the functions you need to run on Compaction object and what state you need to setup.

My proposed solution is to make important parts of Compaction immutable after construction. PickCompaction() should calculate compaction inputs and then pass them onto Compaction object once they are finalized. That makes it easy to create a new compaction -- just provide all the parameters to the constructor and you're done. No need to call confusing functions after you created your object.

This diff doesn't fully achieve that goal, but it comes pretty close. Here are some of the changes:
* have one Compaction constructor instead of two.
* inputs_ is constant after construction
* MarkFilesBeingCompacted() is now private to Compaction class and automatically called on construction/destruction.
* SetupBottommostLevel() is gone. Compaction figures it out on its own based on the input.
* CompactionPicker's functions are not passing around Compaction object anymore. They are only passing around the state that they need.

Test Plan:
make check
make asan_check
make valgrind_check

Reviewers: rven, anthony, sdong, yhchiang

Reviewed By: yhchiang

Subscribers: sdong, yhchiang, dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36687
2015-04-10 15:01:54 -07:00
agiardullo
753dd1fdd0 Fix valgrind issues in memtable_list_test
Summary: Need to remember to unref MemTableList->current() before deleting.

Test Plan: ran test with valgrind

Reviewers: igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36855
2015-04-10 14:16:03 -07:00
krad
697380f3d7 Repairer documentation improvement.
Summary: Adding verbosity to existing comments.

Test Plan: None

Reviewers: sdong

CC: leveldb

Task ID: #6718960

Blame Rev:
2015-04-10 12:35:28 -07:00
Igor Canadi
2f66d7f925 Add LinkedIn back to USERS.md
Summary: Thanks Ankit!

Test Plan: none

Reviewers: ankgup87

Subscribers: dhruba

Differential Revision: https://reviews.facebook.net/D36837
2015-04-10 09:50:19 -07:00
agiardullo
0feeee6433 Fix memtable_list_test
Summary:
Test failing due to a missing directory caused by a simple bug (did not run into this on my dev box since the path already existed).

We should look into deleting test::TmpDir() before each test run.

Test Plan: ran test

Reviewers: igor, yhchiang, meyering, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36831
2015-04-09 22:11:35 -07:00
Yueh-Hsuan Chiang
7b9581bc3b Fixed xfunc related compile errors in ROCKSDB_LITE
Summary:
Fixed xfunc related compile errors in ROCKSDB_LITE

Now make OPT=-DROCKSDB_LITE shared_lib -j32 would work

Test Plan:
make clean
make OPT=-DROCKSDB_LITE shared_lib -j32
make clean
make OPT=-DROCKSDB_LITE static_lib -j32

Reviewers: sdong, igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36825
2015-04-09 21:05:18 -07:00
agiardullo
fabc115690 MemTableList tests
Summary: Add tests for MemTableList

Test Plan: run test

Reviewers: yhchiang, kradhakrishnan, sdong, igor

Reviewed By: igor

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D36735
2015-04-09 18:01:11 -07:00