WritePrepared Txn: Fix DBIterator and add test #3382
Conversation
There was a problem hiding this comment.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
There was a problem hiding this comment.
@yiwu-arbug has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Why not implementing this logic inside "iter_->Seek(last_key);"? Are there any other usages of Seek in the code? What would happen if the user calls Seek? Do we skip values outside the snapshot for them? |
|
There are differences of Seek and FindValueForCurrentKeyUsingSeek. For example with FindValueForCurrentKeyUsingSeek we know which key to find and the key is guarantee to have a valid verions, while they are not true for Seek. Stats counting are also different. It would be nice to merge the two, but given they are diverged, I would not make refactoring this time. |
|
@yiwu-arbug has updated the pull request. View: changes, changes since last import |
There was a problem hiding this comment.
@yiwu-arbug is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: In DBIter, Prev() calls FindValueForCurrentKey() to search the current value backward. If it finds that there are too many stale value being skipped, it falls back to FindValueForCurrentKeyUsingSeek(), seeking directly to the key with snapshot sequence. After introducing read_callback, however, the key it seeks to might not be visible, according to read_callback. It thus needs to keep searching forward until the first visible value. Closes facebook#3382 Differential Revision: D6756148 Pulled By: yiwu-arbug fbshipit-source-id: 064e39b1eec5e083af1c10142600f26d1d2697be
Summary:
In DBIter, Prev() calls FindValueForCurrentKey() to search the current value backward. If it finds that there are too many stale value being skipped, it falls back to FindValueForCurrentKeyUsingSeek(), seeking directly to the key with snapshot sequence. After introducing read_callback, however, the key it seeks to might not be visible, according to read_callback. It thus needs to keep searching forward until the first visible value.
Test Plan:
Added a new test to test DBIter with read_callback.