Skip to content

Conversation

@sagar0
Copy link
Contributor

@sagar0 sagar0 commented Feb 20, 2018

User-Visible changes:

  1. Do not fail a Seek-family operation (Seek, SeekForPrev, SeekToFirst, SeekToLast) as Incomplete when RocksDB encounters many internal keys to get to a valid user-visible key, on setting ReadOptions.max_skippable_internal_keys. Fail only Next and Prev operations. This change will enable users to re-seek on a failed Next/Prev op.
  2. Added a property rocksdb.iterator.internal-key to return the internal key at which the iterator stopped.
  3. Allow the same iterator to be re-used even after Next/Prev return Incomplete status. Note that an iterator returns Incomplete status only on encountering too many internal keys (say, tombstones). The user can then seek to the next valid key. This wasn't possible previously, as the user had to create a new iterator on encountering a non-ok status. Creating a new iterator is costly as compared to re-using the same one ... so this change was done to reduce iterator-creation cost.

Non-user visible changes:

  • Added a new enum Operation in DBIter to keep track of what is the current operation that is being performed. This helps in making decisions based on the operation in the functions down the call stack. For example: TooManyInternalKeys method is called in both Seek and Next call-stack. We can make better decision in these functions if we know what top-level API calls it.

Test Plan:
New tests added.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sagar0
Copy link
Contributor Author

sagar0 commented Feb 20, 2018

Discarding this PR in favor of #3525. After a lot of internal discussion, we decided not to special-case the functionality between Seek* and Next/Prev.
I'll create another PR just to expose the internal key with a property.

@sagar0 sagar0 closed this Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants