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

Fix off-by-one bug in VersionStorageInfo::ComputeFilesMarkedForForcedBlobGC #9542

Closed
wants to merge 1 commit into from

Conversation

ltamasi
Copy link
Contributor

@ltamasi ltamasi commented Feb 10, 2022

Summary:
Fixes a bug introduced in #9526 where we index one position past the
end of a vector.

Test Plan:
make asan_check

Will add a unit test in a separate PR.

Copy link
Contributor

@akankshamahajan15 akankshamahajan15 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

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

facebook-github-bot pushed a commit that referenced this pull request Feb 11, 2022
…9548)

Summary:
We had a bug in `VersionStorageInfo::ComputeFilesMarkedForForcedBlobGC`
related to the edge case where all blob files are part of the "oldest batch",
i.e. where only the very oldest file has any linked SSTs. (See #9542)
This PR tries to make the logic in this method clearer and also adds a unit test
for the problematic case.

Pull Request resolved: #9548

Test Plan: `make check`

Reviewed By: akankshamahajan15

Differential Revision: D34158959

Pulled By: ltamasi

fbshipit-source-id: fbab6d749c569728382aa04f7b7c60c92cca7650
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.

3 participants