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

Enable ClockCache in DB block cache test #10482

Closed
wants to merge 3 commits into from

Conversation

guidotag
Copy link
Contributor

@guidotag guidotag commented Aug 4, 2022

Summary: A test in db_block_cache_test.cc was skipping ClockCache due to the 16-byte key length requirement. We fixed this. Along the way, we fixed a bug in ApplyToSomeEntries, which assumed the function being applied could modify handle metadata, and thus took an exclusive reference. This is incompatible with calls that need to inspect every element (including externally referenced ones) to gather stats.

Test plan: make -j24 check

@guidotag guidotag requested a review from anand1976 August 4, 2022 22:08
@guidotag guidotag changed the title Enable ClockCache in DB block cache test. Enable ClockCache in DB block cache test Aug 4, 2022
Copy link
Contributor

@anand1976 anand1976 left a comment

Choose a reason for hiding this comment

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

LGTM

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

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

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

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