Integrated blob garbage collection: relocate blobs #7694
Conversation
There was a problem hiding this comment.
@ltamasi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@ltamasi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
LGTM with a few minor comments. Thanks @ltamasi for the PR.
db/compaction/compaction_iterator.h
Outdated
| void PrepareOutput(); | ||
|
|
||
| bool ExtractLargeValueImpl(); |
There was a problem hiding this comment.
Maybe it's better to put some comment on these methods?
| std::advance( | ||
| it, compaction->blob_garbage_collection_age_cutoff() * blob_files.size()); | ||
|
|
||
| return it != blob_files.end() ? it->first |
There was a problem hiding this comment.
Just curious: if !blob_files.empty() and it == blob_files.end(), will returning blob_files.back()->first + 1 also be correct?
There was a problem hiding this comment.
Yes, that would also work.
db/compaction/compaction_iterator.h
Outdated
| void PrepareOutput(); | ||
|
|
||
| bool ExtractLargeValueImpl(); | ||
| void ExtractLargeValue(); |
There was a problem hiding this comment.
Neither ExtractLargeValue() nor ExtractLargeValueImpl() guarantee to extract large value. Maybe it's clearer to name MaybeExtractLargeValueIfNeeded() and MaybeExtractLargeValueIfNeededImpl()?
|
Thanks so much for the review @riversand963 ! |
…elper methods in CompactionIterator
…blob files before compaction
…e to be met; add comments
ltamasi
force-pushed
the
blob_integrated_gc
branch
from
e2571b0
to
3722d53
Compare
|
@ltamasi has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@ltamasi has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Summary: The patch adds basic garbage collection support to the integrated BlobDB implementation. Valid blobs residing in the oldest blob files are relocated as they are encountered during compaction. The threshold that determines which blob files qualify is computed based on the configuration option `blob_garbage_collection_age_cutoff`, which was introduced in facebook#7661 . Once a blob is retrieved for the purposes of relocation, it passes through the same logic that extracts large values to blob files in general. This means that if, for instance, the size threshold for key-value separation (`min_blob_size`) got changed or writing blob files got disabled altogether, it is possible for the value to be moved back into the LSM tree. In particular, one way to re-inline all blob values if needed would be to perform a full manual compaction with `enable_blob_files` set to `false`, `enable_blob_garbage_collection` set to `true`, and `blob_file_garbage_collection_age_cutoff` set to `1.0`. Some TODOs that I plan to address in separate PRs: 1) We'll have to measure the amount of new garbage in each blob file and log `BlobFileGarbage` entries as part of the compaction job's `VersionEdit`. (For the time being, blob files are cleaned up solely based on the `oldest_blob_file_number` relationships.) 2) When compression is used for blobs, the compression type hasn't changed, and the blob still qualifies for being written to a blob file, we can simply copy the compressed blob to the new file instead of going through decompression and compression. 3) We need to update the formula for computing write amplification to account for the amount of data read from blob files as part of GC. Pull Request resolved: facebook#7694 Test Plan: `make check` Reviewed By: riversand963 Differential Revision: D25069663 Pulled By: ltamasi fbshipit-source-id: bdfa8feb09afcf5bca3b4eba2ba72ce2f15cd06a
Summary:
The patch adds basic garbage collection support to the integrated BlobDB
implementation. Valid blobs residing in the oldest blob files are relocated
as they are encountered during compaction. The threshold that determines
which blob files qualify is computed based on the configuration option
blob_garbage_collection_age_cutoff, which was introduced in #7661 .Once a blob is retrieved for the purposes of relocation, it passes through the
same logic that extracts large values to blob files in general. This means that
if, for instance, the size threshold for key-value separation (
min_blob_size)got changed or writing blob files got disabled altogether, it is possible for the
value to be moved back into the LSM tree. In particular, one way to re-inline
all blob values if needed would be to perform a full manual compaction with
enable_blob_filesset tofalse,enable_blob_garbage_collectionset totrue, andblob_file_garbage_collection_age_cutoffset to1.0.Some TODOs that I plan to address in separate PRs:
BlobFileGarbageentries as part of the compaction job'sVersionEdit.(For the time being, blob files are cleaned up solely based on the
oldest_blob_file_numberrelationships.)and the blob still qualifies for being written to a blob file, we can simply copy
the compressed blob to the new file instead of going through decompression
and compression.
for the amount of data read from blob files as part of GC.
Test Plan:
make check