Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect index eviction metric with two-level indexes #4760

Open
benclay opened this issue Dec 7, 2018 · 0 comments
Open

Incorrect index eviction metric with two-level indexes #4760

benclay opened this issue Dec 7, 2018 · 0 comments
Labels
up-for-grabs Up for grabs

Comments

@benclay
Copy link
Contributor

benclay commented Dec 7, 2018

We are seeing that using:

  • kTwoLevelIndexSearch and
  • cache_index_and_filter_blocks = false

is leaving the BLOCK_CACHE_INDEX_BYTES_EVICT metric at zero, while BLOCK_CACHE_INDEX_BYTES_INSERT keeps climbing. I believe this is because PutDataBlockToCache() is called for the lower-level index blocks, but the deletion callback is always bound to the naive DeleteCachedEntry() data deletion callback.

See discussion and proposed solution in #4663 To summarize:

  1. Augment cache.h with ::Insert function that receives a void (deleter)(const Slice& key, void value, Statistics* statistics).
  2. Then we can add a check to each ::Insert override to ensure that they are called only when the new signature is enabled.
  3. Also extend the constructor to receive a boolean that specifies which ::Insert method is expected to be called later and assert that in each ::Insert implantation to avoid combining the two ::Insert API.
  4. When calling the delete callback upon eviction, pass the cache's own statistics object in
  5. Define a new delete callback for index blocks that correctly increments BLOCK_CACHE_INDEX_BYTES_EVICT
  6. Define a new delete callback for data blocks that is a clone of the existing DeleteCachedEntry(), just with the new statistics pointer
  7. In table/block_based_table_reader.cc BlockBasedTable::PutDataBlockToCache, use the is_index flag to determine if the block is a data or index block when wiring up the delete callback.
  8. Use the upgrade / downgrade boolean added in step 3 above to choose whether to use the current behavior or the new callbacks in step 7

Expected behavior

BLOCK_CACHE_INDEX_BYTES_EVICT metric is accurate even for two-level index

Actual behavior

BLOCK_CACHE_INDEX_BYTES_EVICT is zero always with two-level index

Steps to reproduce the behavior

Use these options together:

  • kTwoLevelIndexSearch and
  • cache_index_and_filter_blocks = false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
up-for-grabs Up for grabs
Projects
None yet
Development

No branches or pull requests

2 participants