]> git-server-git.apps.pok.os.sepia.ceph.com Git - rocksdb.git/log
rocksdb.git
6 years agoFix SeekForPrev bug with Partitioned Filters and Prefix (#5907)
Maysam Yabandeh [Sat, 12 Oct 2019 03:28:36 +0000 (20:28 -0700)]
Fix SeekForPrev bug with Partitioned Filters and Prefix (#5907)

Summary:
Partition Filters make use of a top-level index to find the partition that might have the bloom hash of the key. The index is with internal key format (before format version 3). Each partition contains the i) blooms of the keys in that range ii) bloom of prefixes of keys in that range, iii) the bloom of the prefix of the last key in the previous partition.
When ::SeekForPrev(key), we first perform a prefix bloom test on the SST file. The partition however is identified using the full internal key, rather than the prefix key. The reason is to be compatible with the internal key format of the top-level index. This creates a corner case. Example:
- SST k, Partition N: P1K1, P1K2
- SST k, top-level index: P1K2
- SST k+1, Partition 1: P2K1, P3K1
- SST k+1 top-level index: P3K1
When SeekForPrev(P1K3), it should point us to P1K2. However SST k top-level index would reject P1K3 since it is out of range.
One possible fix would be to search with the prefix P1 (instead of full internal key P1K3) however the details of properly comparing prefix with full internal key might get complicated. The fix we apply in this PR is to look into the last partition anyway even if the key is out of range.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5907

Differential Revision: D17889918

Pulled By: maysamyabandeh

fbshipit-source-id: 169fd7b3c71dbc08808eae5a8340611ebe5bdc1e

6 years agoUpdate HISTORY.md with a bug fix
anand76 [Mon, 7 Oct 2019 23:55:47 +0000 (16:55 -0700)]
Update HISTORY.md with a bug fix

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

6 years agoFix data block upper bound checking for iterator reseek case (#5883)
anand76 [Fri, 4 Oct 2019 03:52:17 +0000 (20:52 -0700)]
Fix data block upper bound checking for iterator reseek case (#5883)

Summary:
When an iterator reseek happens with the user specifying a new iterate_upper_bound in ReadOptions, and the new seek position is at the end of the same data block, the Seek() ends up using a stale value of data_block_within_upper_bound_ and may return incorrect results.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5883

Test Plan: Added a new test case DBIteratorTest.IterReseekNewUpperBound. Verified that it failed due to the assertion failure without the fix, and passes with the fix.

Differential Revision: D17752740

Pulled By: anand1976

fbshipit-source-id: f9b635ff5d6aeb0e1bef102cf8b2f900efd378e3

6 years agoFix a bug of the previous fix.
sdong [Tue, 1 Oct 2019 23:50:15 +0000 (16:50 -0700)]
Fix a bug of the previous fix.

6 years agoBump up the version to 6.4.5
sdong [Tue, 1 Oct 2019 23:32:46 +0000 (16:32 -0700)]
Bump up the version to 6.4.5

6 years agoRevert "Merging iterator to avoid child iterator reseek for some cases (#5286)" ...
sdong [Tue, 1 Oct 2019 18:20:50 +0000 (11:20 -0700)]
Revert "Merging iterator to avoid child iterator reseek for some cases (#5286)" (#5871)

Summary:
This reverts commit 9fad3e21eb90d215b6719097baba417bc1eeca3c.

Iterator verification in stress tests sometimes fail for assertion
table/block_based/block_based_table_reader.cc:2973: void rocksdb::BlockBasedTableIterator<TBlockIter, TValue>::FindBlockForward() [with TBlockIter = rocksdb::DataBlockIter; TValue = rocksdb::Slice]: Assertion `!next_block_is_out_of_bound || user_comparator_.Compare(*read_options_.iterate_upper_bound, index_iter_->user_key()) <= 0' failed.

It is likely to be linked to https://github.com/facebook/rocksdb/pull/5286 together with https://github.com/facebook/rocksdb/pull/5468 as the former PR makes some child iterator's seek being avoided, so that upper bound condition fails to be updated there. Strictly speaking, the former PR was merged before the latter one, but the latter one feels a more important improvement so I choose to revert the former one for now.
Pull Request resolved: https://github.com/facebook/rocksdb/pull/5871

Differential Revision: D17689196

fbshipit-source-id: 4ded5be68f67bee2782d31a29cb72ea68f59dd8c