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

Log the amount of blob garbage generated by compactions in the MANIFEST #8450

Closed
wants to merge 13 commits into from

Conversation

ltamasi
Copy link
Contributor

@ltamasi ltamasi commented Jun 23, 2021

Summary:
The patch builds on BlobGarbageMeter and BlobCountingIterator
(introduced in #8426 and
#8443 respectively)
and ties it all together. It measures the amount of garbage
generated by a compaction and logs the corresponding BlobFileGarbage
records as part of the compaction job's VersionEdit. Note: in order
to have accurate results, kRemoveAndSkipUntil for compaction filters
is implemented using iteration.

Test Plan:
Ran make check and the crash test script.

@facebook-github-bot
Copy link
Contributor

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

Copy link
Contributor

@jay-zhuang jay-zhuang left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -1136,6 +1147,15 @@ void CompactionJob::ProcessKeyValueCompaction(SubcompactionState* sub_compact) {
input = clip.get();
}

std::unique_ptr<InternalIterator> blob_counter;

if (sub_compact->compaction->DoesInputReferenceBlobFiles()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

cool that we check it first to avoid unnecessary counting.👍

@facebook-github-bot
Copy link
Contributor

@ltamasi merged this pull request in 68d8b28.

ltamasi added a commit to ltamasi/rocksdb that referenced this pull request Jul 6, 2021
…nge"

Summary:
We ended up using a different approach for tracking the amount of
garbage in blob files (see e.g. facebook#8450),
so the ability to apply only a range of table property collectors is
now unnecessary. The patch reverts this part of
facebook#8298 while keeping the cleanup.

Test Plan:
`make check`
facebook-github-bot pushed a commit that referenced this pull request Jul 6, 2021
…ange (#8465)

Summary:
We ended up using a different approach for tracking the amount of
garbage in blob files (see e.g. #8450),
so the ability to apply only a range of table property collectors is
now unnecessary. The patch reverts this part of
#8298 while keeping the cleanup done
in that PR.

Pull Request resolved: #8465

Test Plan: `make check`

Reviewed By: jay-zhuang

Differential Revision: D29399921

Pulled By: ltamasi

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