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

Cache warming blocks during flush #8561

Closed

Conversation

akankshamahajan15
Copy link
Contributor

@akankshamahajan15 akankshamahajan15 commented Jul 19, 2021

Summary: Insert warm blocks (data, uncompressed dict, index and filter blocks) during flush in Block cache which is enabled under option BlockBasedTableOptions.prepopulate_block_cache.

Test Plan: Added unit test

Reviewers:

Subscribers:

Tasks:

Tags:

@facebook-github-bot
Copy link
Contributor

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

table/block_based/block_based_table_builder.cc Outdated Show resolved Hide resolved
table/block_based/block_based_table_builder.cc Outdated Show resolved Hide resolved
if (type == kNoCompression) {
s = InsertBlockInCache(block_contents, handle);
s = InsertBlockInCache(block_contents, handle, block_entry,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we insert all block types or just index/filter/data?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anand1976 It currently inserts all block types. I wasn't sure about other blocks and their impact in terms of caching them so I inserted them also. Do we need them or they can be ignored? BlockBasedTableReader doesn't cache them and those blocks are used only once during opening the table so they can be ignored.

Copy link
Contributor

Choose a reason for hiding this comment

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

@akankshamahajan15 They can be ignored, except for uncompression dict. I don't think BlockBasedTableReader even looks in the cache for other block types.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

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

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

@facebook-github-bot
Copy link
Contributor

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

@akankshamahajan15 akankshamahajan15 changed the title Cache warming all blocks during flush Cache warming blocks during flush Aug 3, 2021
@facebook-github-bot
Copy link
Contributor

@akankshamahajan15 merged this pull request in 8b2f60b.

jaepil pushed a commit to deepsearch-hq/rocksdb-backup that referenced this pull request Aug 5, 2021
Summary:
Insert warm blocks  (data, uncompressed dict, index and filter blocks) during flush in Block cache which is enabled under option BlockBasedTableOptions.prepopulate_block_cache.

Pull Request resolved: facebook#8561

Test Plan: Added unit test

Reviewed By: anand1976

Differential Revision: D29773411

Pulled By: akankshamahajan15

fbshipit-source-id: 6631123c10134340ef0bd7e90baafaa6deba0e66

# Conflicts:
#	HISTORY.md
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.

None yet

3 participants