- 14 11月, 2015 1 次提交
-
-
由 Venkatesh Radhakrishnan 提交于
Summary: Under a tailing workload, there were increased block cache misses when a memtable was flushed because we were rebuilding iterators in that case since the version set changed. This was exacerbated in the case of iterate_upper_bound, since file iterators which were over the iterate_upper_bound would have been deleted and are now brought back as part of the Rebuild, only to be deleted again. We now renew the iterators and only build iterators for files which are added and delete file iterators for files which are deleted. Refer to https://reviews.facebook.net/D50463 for previous version Test Plan: DBTestTailingIterator.TailingIteratorTrimSeekToNext Reviewers: anthony, IslamAbdelRahman, igor, tnovak, yhchiang, sdong Reviewed By: sdong Subscribers: yhchiang, march, dhruba, leveldb, lovro Differential Revision: https://reviews.facebook.net/D50679
-
- 14 10月, 2015 1 次提交
-
-
由 sdong 提交于
Summary: Separate a new class InternalIterator from class Iterator, when the look-up is done internally, which also means they operate on key with sequence ID and type. This change will enable potential future optimizations but for now InternalIterator's functions are still the same as Iterator's. At the same time, separate the cleanup function to a separate class and let both of InternalIterator and Iterator inherit from it. Test Plan: Run all existing tests. Reviewers: igor, yhchiang, anthony, kradhakrishnan, IslamAbdelRahman, rven Reviewed By: rven Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D48549
-
- 09 9月, 2015 2 次提交
-
-
由 Andres Notzli 提交于
Summary: It looks like in some cases an assert in SeekInternal failed when computing the hints for the next level because user_key was the same as the largest key and not strictly smaller. Relaxing the assert to expect smaller or equal keys. Test Plan: make clean all check Reviewers: sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D46443
-
由 Andres Noetzli 提交于
Summary: When computing the hint for GetNextLevelIndex(), ForwardIterator was doing a redundant comparison. This patch fixes the comparison (using https://github.com/facebook/rocksdb/blob/master/db/version_set.cc#L158 as a reference) and moves it inside an assert because we expect `level_files[f_idx]` to contain the next key after Seek(), so user_key should always be smaller than the largest key. Test Plan: make clean all check Reviewers: rven, anthony, yhchiang, igor, sdong Reviewed By: sdong Subscribers: tnovak, sdong, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D46227
-
- 05 9月, 2015 1 次提交
-
-
由 Venkatesh Radhakrishnan 提交于
Summary: This diff fixes a case when the forward iterator misses a new insert when the mutable iterator is not current. The test is also improved and the check for deleted iterators is made more informative. Test Plan: DBTailingIteratorTest.*Trim Reviewers: tnovak, sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D46167
-
- 02 9月, 2015 1 次提交
-
-
由 Tomislav Novak 提交于
Summary: I noticed that memtable iterator usually crosses the `iterate_upper_bound` threshold when tailing. Changes introduced in D43833 made `NeedToSeekImmutable` always return true in such case, even when `Seek()` only needs to rewind the memtable iterator. In a test I ran, this caused the "tailing efficiency" (ratio of calls to `Seek()` that only affect the memtable versus all seeks) to drop almost to zero. This diff attempts to fix the regression by using a different flag to indicate that `current_` is over the limit instead of resetting `valid_` in `UpdateCurrent()`. Test Plan: `DBTestTailingIterator.TailingIteratorUpperBound` Reviewers: sdong, rven Reviewed By: rven Subscribers: dhruba, march Differential Revision: https://reviews.facebook.net/D45909
-
- 29 8月, 2015 1 次提交
-
-
由 Venkatesh Radhakrishnan 提交于
Summary: The immutable memtable iterators are allocated from an arena and there is no benefit from deleting these. Also the immutable memtables themselves will continue to be in memory until the version set containing it is alive. We will not remove immutable memtable iterators over the upper bound. We now add immutable iterators to the test. Test Plan: db_tailing_iter_test.TailingIteratorTrimSeekToNext Reviewers: tnovak, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D45597
-
- 26 8月, 2015 2 次提交
-
-
由 Venkatesh Radhakrishnan 提交于
Summary: We have earlier added a feature to delete file iterators when the current key is over the iterate upper bound. We now add a whitebox test to check if the file iterators were actually deleted. Test Plan: Add check for a range which has deleted iterators. Reviewers: sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D45321
-
由 Venkatesh Radhakrishnan 提交于
Summary: After deleting file iterators which are over the iterate upper bound, we also need to check for null pointers in ResetIncompletIterators. Test Plan: db_tailing_iter_test.TailingIteratorTrimSeekToNext Reviewers: tnovak, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D45525
-
- 20 8月, 2015 2 次提交
-
-
由 Andres Noetzli 提交于
Summary: Removing two unused variables that prevented compilation. Test Plan: make all Reviewers: rven, sdong, yhchiang, anthony, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D44991
-
由 Venkatesh Radhakrishnan 提交于
Summary: This diff improves the memory utilization for tailing iterators RocksDB, by freeing file iterators which are over the upper bound. It is an updating on Siying's original diff for improving the memory usage for tailing iterators. The changes for the seek and next path are now complete and a test has been added to exercise these paths while deleting file iterators which are above the upper bound. Test Plan: db_tailing_iter_test.TailingIteratorTrimSeekToNext Reviewers: march, tnovak, sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D43833
-
- 19 8月, 2015 1 次提交
-
-
由 Venkatesh Radhakrishnan 提交于
Summary: Reseek mutable_iter if it is invalid in Next and immutable_iter is invalid. Test Plan: DBTestTailingIterator.TailingIteratorSeekToNext Reviewers: tnovak, march, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D44865
-
- 15 8月, 2015 1 次提交
-
-
由 sdong 提交于
Summary: In internal stats, remember read latency histogram, if statistics is enabled. It can be retrieved from DB::GetProperty() with "rocksdb.dbstats" property, if it is enabled. Test Plan: Manually run db_bench and prints out "rocksdb.dbstats" by hand and make sure it prints out as expected Reviewers: igor, IslamAbdelRahman, rven, kradhakrishnan, anthony, yhchiang Reviewed By: yhchiang Subscribers: MarkCallaghan, leveldb, dhruba Differential Revision: https://reviews.facebook.net/D44193
-
- 08 7月, 2015 1 次提交
-
-
由 Yueh-Hsuan Chiang 提交于
Summary: Fixes valgrind errors in column_family_test. Test Plan: `make check`, `make valgrind_check` Reviewers: igor, yhchiang Reviewed By: yhchiang Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D41181
-
- 27 2月, 2015 1 次提交
-
-
由 Igor Sugak 提交于
Summary: When using latest clang (3.6 or 3.7/trunck) rocksdb is failing with many errors. Almost all of them are missing override errors. This diff adds missing override keyword. No manual changes. Prerequisites: bear and clang 3.5 build with extra tools ```lang=bash % USE_CLANG=1 bear make all # generate a compilation database http://clang.llvm.org/docs/JSONCompilationDatabase.html % clang-modernize -p . -include . -add-override % make format ``` Test Plan: Make sure all tests are passing. ```lang=bash % #Use default fb code clang. % make check ``` Verify less error and no missing override errors. ```lang=bash % # Have trunk clang present in path. % ROCKSDB_NO_FBCODE=1 CC=clang CXX=clang++ make ``` Reviewers: igor, kradhakrishnan, rven, meyering, sdong Reviewed By: sdong Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D34077
-
- 13 2月, 2015 1 次提交
-
-
由 Igor Canadi 提交于
Summary: It would be good to assing background job their IDs. Two benefits: 1) makes LOGs more readable 2) I might use it in my EventLogger, which will try to make our LOG easier to read/query/visualize Test Plan: ran rocksdb, read the LOG Reviewers: sdong, rven, yhchiang Reviewed By: yhchiang Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D31617
-
- 19 11月, 2014 1 次提交
-
-
由 Lei Jin 提交于
Summary: The very last reference happens in DBImpl::GetOptions() I built with both DBImpl::GetOptions() and ColumnFamilyData::options() commented out Test Plan: make all check Reviewers: sdong, yhchiang, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D29073
-
- 12 11月, 2014 1 次提交
-
-
由 Igor Canadi 提交于
Summary: We need to turn on -Wshorten-64-to-32 for mobile. See D1671432 (internal phabricator) for details. This diff turns on the warning flag and fixes all the errors. There were also some interesting errors that I might call bugs, especially in plain table. Going forward, I think it makes sense to have this flag turned on and be very very careful when converting 64-bit to 32-bit variables. Test Plan: compiles Reviewers: ljin, rven, yhchiang, sdong Reviewed By: yhchiang Subscribers: bobbaldwin, dhruba, leveldb Differential Revision: https://reviews.facebook.net/D28689
-
- 11 11月, 2014 1 次提交
-
-
由 Tomislav Novak 提交于
Summary: In D19581 I made `ForwardIterator::status()` check all child iterators, including immutable ones. It's, however, not necessary to do it every time -- it'll suffice to check only when they're used and their status could change. This diff: * introduces `immutable_status_` which is updated by `Seek()` and `Next()` * removes special handling of `kIncomplete` status in those methods Test Plan: * `db_test` * hacked ReadSequential in db_bench.cc to check `status()` in addition to validity: ``` $ ./db_bench -use_existing_db -benchmarks readseq -disable_auto_compactions \ -use_tailing_iterator # without this patch Keys: 16 bytes each Values: 100 bytes each (50 bytes after compression) Entries: 1000000 [...] DB path: [/dev/shm/rocksdbtest/dbbench] readseq : 0.562 micros/op 1778103 ops/sec; 98.4 MB/s $ ./db_bench -use_existing_db -benchmarks readseq -disable_auto_compactions \ -use_tailing_iterator # with the patch readseq : 0.433 micros/op 2311363 ops/sec; 127.8 MB/s ``` Reviewers: igor, sdong, ljin Reviewed By: ljin Subscribers: dhruba, leveldb, march, lovro Differential Revision: https://reviews.facebook.net/D24063
-
- 05 11月, 2014 1 次提交
-
-
由 sdong 提交于
Summary: Enforce the accessier naming convention in functions in version_set.h Test Plan: make all check Reviewers: ljin, yhchiang, rven, igor Reviewed By: igor Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D28143
-
- 01 11月, 2014 1 次提交
-
-
由 sdong 提交于
Summary: Rename Version::Builder to VersionBuilder and expose its definition to a header. Make VerisonBuilder not reference Version or ColumnFamilyData, only working with VersionStorageInfo. Add version_builder_test which has a simple test. Test Plan: make all check Reviewers: rven, yhchiang, igor, ljin Reviewed By: igor Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D27969
-
- 30 10月, 2014 1 次提交
-
-
由 sdong 提交于
Summary: Make compaction picker easier to test. The basic idea is to separate a minimum subcomponent of Version to VersionStorageInfo, which just responsible to LSM tree. A stub VersionStorageInfo can then be easily created and passed into compaction picker so that we can check the outputs. It now passes most tests. Still two things need to be done: (1) deal with the FIFO compaction's file size. (2) write an example test to make sure the interface can do the job. Add a compaction_picker_test to make sure compaction picker codes can be easily unit tested. Test Plan: Pass all unit tests and compaction_picker_test Reviewers: yhchiang, rven, igor, ljin Reviewed By: ljin Subscribers: leveldb, dhruba Differential Revision: https://reviews.facebook.net/D27639
-
- 29 10月, 2014 2 次提交
-
-
由 Igor Canadi 提交于
Summary: Abstract out FlushProcess and take it out of DBImpl. This also includes taking DeletionState outside of DBImpl. Currently this diff is only doing the refactoring. Future work includes: 1. Decoupling flush_process.cc, make it depend on less state 2. Write flush_process_test, which will mock out everything that FlushProcess depends on and test it in isolation Test Plan: make check Reviewers: rven, yhchiang, sdong, ljin Reviewed By: ljin Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D27561
-
由 Lei Jin 提交于
Summary: as title Test Plan: make release will run full test on all stacked diffs Reviewers: sdong, yhchiang, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D27597
-
- 24 10月, 2014 1 次提交
-
-
由 Lei Jin 提交于
Summary: This is not a critical options. Making it dynamic so that we can remove more reference to cfd->options() Test Plan: unit test Reviewers: yhchiang, sdong, igor Reviewed By: igor Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D24957
-
- 02 10月, 2014 1 次提交
-
-
由 Tomislav Novak 提交于
Summary: Since ForwardIterator is on a level below DBIter, the latter may call Next() on it (e.g. in order to skip deletion markers). Since this also updates `prev_key_`, it may prevent the Seek() optimization. For example, assume that there's only one SST file and it contains the following entries: 0101, 0201 (`ValueType::kTypeDeletion`, i.e. a tombstone record), 0201 (`kTypeValue`), 0202. Memtable is empty. `Seek(0102)` will result in `prev_key_` being set to `0201` instead of `0102`, since `DBIter::Seek()` will call `ForwardIterator::Next()` to skip record 0201. Therefore, when `Seek(0102)` is called again, `NeedToSeekImmutable()` will return true. This fix relies on `prefix_extractor_` to detect prefix changes. `prev_key_` is only set to `current_->key()` as long as they have the same prefix. I also made a small change to `NeedToSeekImmutable()` so it no longer returns true when the db is empty (i.e. there's nothing but a memtable). Test Plan: $ TEST_TMPDIR=/dev/shm/rocksdbtest ROCKSDB_TESTS=TailingIterator ./db_test Reviewers: sdong, igor, ljin Reviewed By: ljin Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D23823
-
- 05 9月, 2014 1 次提交
-
-
由 Stanislau Hlebik 提交于
Summary: Simply code by removing code path which does not use Arena from NewInternalIterator Test Plan: make all check make valgrind_check Reviewers: sdong Reviewed By: sdong Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D22395
-
- 30 8月, 2014 1 次提交
-
-
由 Tomislav Novak 提交于
Summary: When reading from kBlockCacheTier, ForwardIterator's internal child iterators may end up in the incomplete state (read was unable to complete without doing disk I/O). `ForwardIterator::status()` will correctly report that; however, the iterator may be stuck in that state until all sub-iterators are rebuilt: * `NeedToSeekImmutable()` may return false even if some sub-iterators are incomplete * one of the child iterators may be an empty iterator without any state other that the kIncomplete status (created using `NewErrorIterator()`); seeking on any such iterator has no effect -- we need to construct it again Akin to rebuilding iterators after a superversion bump, this diff makes forward iterator reset all incomplete child iterators when `Seek()` or `Next()` are called. Test Plan: TEST_TMPDIR=/dev/shm/rocksdbtest ROCKSDB_TESTS=TailingIterator ./db_test Reviewers: igor, sdong, ljin Reviewed By: ljin Subscribers: lovro, march, leveldb Differential Revision: https://reviews.facebook.net/D22575
-
- 11 7月, 2014 2 次提交
-
-
由 Tomislav Novak 提交于
Summary: If `NeedToSeekImmutable()` returns false, `SeekInternal()` won't reset the contents of `immutable_min_heap_`. However, since it calls `UpdateCurrent()` unconditionally, if `current_` is one of immutable iterators (previously popped from `immutable_min_heap_`), `UpdateCurrent()` will overwrite it. As a result, if old `current_` in fact pointed to the smallest entry, forward iterator will skip some records. Fix implemented in this diff pushes `current_` back to `immutable_min_heap_` before calling `UpdateCurrent()`. Test Plan: New unit test (courtesy of @lovro): $ ROCKSDB_TESTS=TailingIteratorSeekToSame ./db_test Reviewers: igor, dhruba, haobo, ljin Reviewed By: ljin Subscribers: lovro, leveldb Differential Revision: https://reviews.facebook.net/D19653
-
由 Tomislav Novak 提交于
Summary: Forward iterator only checked `status_` and `mutable_iter_->status()`, which is not sufficient. For example, when reading exclusively from cache (kBlockCacheTier), `mutable_iter_->status()` may return kOk (e.g. there's nothing in the memtable), but one of immutable iterators could be in kIncomplete. In this case, `ForwardIterator::status()` ought to return that status instead of kOk. This diff changes `status()` to also check `imm_iters_`, `l0_iters_`, and `level_iters_`. Test Plan: ROCKSDB_TESTS=TailingIteratorIncomplete ./db_test Reviewers: ljin, igor Reviewed By: igor Subscribers: dhruba, leveldb Differential Revision: https://reviews.facebook.net/D19581
-
- 09 7月, 2014 1 次提交
-
-
由 Lei Jin 提交于
Summary: Use search hint to reduce FindFile range thus avoid comparison For a small DB with 50M keys, perf_context counter shows it reduces comparison from 2B to 1.3B for a 15-minute run. No perf change was observed for 1 seek thread, but quite good improvement was seen for 32 seek threads, when CPU was busy. will post detail results when ready Test Plan: db_bench and db_test Reviewers: haobo, sdong, dhruba, igor Reviewed By: igor Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D18879
-
- 17 6月, 2014 1 次提交
-
-
由 sdong 提交于
Summary: We added multiple fields to FileMetaData recently and are planning to add more. This refactoring separate the minimum information for accessing the file. This object is copyable (FileMetaData is not copyable since the ref counter). I hope this refactoring can enable further improvements: (1) use it to design a more efficient data structure to speed up read queries. (2) in the future, when we add information of storage level, we can easily do the encoding, instead of enlarge this structure, which might expand memory work set for file meta data. The definition is same as current EncodedFileMetaData used in two level iterator, so now the logic in two level iterator is easier to understand. Test Plan: make all check Reviewers: haobo, igor, ljin Reviewed By: ljin Subscribers: leveldb, dhruba, yhchiang Differential Revision: https://reviews.facebook.net/D18933
-
- 11 6月, 2014 1 次提交
-
-
由 Lei Jin 提交于
Summary: obvious Test Plan: db_test Reviewers: sdong, haobo, igor Reviewed By: igor Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D18987
-
- 04 6月, 2014 1 次提交
-
-
由 Igor Canadi 提交于
Summary: https://phabricator.fb.com/P11372644 Test Plan: compiles Reviewers: sdong, ljin Reviewed By: ljin Subscribers: leveldb Differential Revision: https://reviews.facebook.net/D18873
-
- 31 5月, 2014 1 次提交
-
-
由 Lei Jin 提交于
Summary: Forward iterator puts everything together in a flat structure instead of a hierarchy of nested iterators. this should simplify the code and provide better performance. It also enables more optimization since all information are accessiable in one place. Init evaluation shows about 6% improvement Test Plan: db_test and db_bench Reviewers: dhruba, igor, tnovak, sdong, haobo Reviewed By: haobo Subscribers: sdong, leveldb Differential Revision: https://reviews.facebook.net/D18795
-