facebook / facebook/rocksdb

Incorrect index eviction metric with two-level indexes

Open
#4,760 0 comments 0 reactions 0 assignees View on GitHub
up-for-grabs
Dominant language
C++
Stars
32.1k
Forks
6.9k
Avg merge
32m
Merged PRs (30d)
1

Description

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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.