os/bluestore: garbage collect partially overlapped blobs #11232
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
liewegas
changed the title
liewegas
requested changes
Sep 27, 2016
| region_t(const region_t& from) | ||
| : logical_offset(from.logical_offset), | ||
| blob_xoffset(from.blob_xoffset), | ||
| length(from.length) {} | ||
| length(from.length){} | ||
| //blob_depth(from.blob_depth) {} |
There was a problem hiding this comment.
Please drop teh commented out bits from region_t
| @@ -2421,6 +2434,7 @@ void BlueStore::_init_logger() | |||
| b.add_u64(l_bluestore_txc, "bluestore_txc", "Transactions committed"); | |||
| b.add_u64(l_bluestore_onode_reshard, "bluestore_onode_reshard", | |||
| "Onode extent map reshard events"); | |||
| b.add_u64(l_bluestore_gc_bytes, "bluestore_gc_bytes", "Total garbage collection"); | |||
There was a problem hiding this comment.
"Total bytes rewritten during garbage collection" ?
There was a problem hiding this comment.
Only the delta is added. Updated the comment.
| @@ -972,6 +972,8 @@ OPTION(bluestore_compression, OPT_STR, "none") // force|aggressive|passive|none | |||
| OPTION(bluestore_compression_algorithm, OPT_STR, "snappy") | |||
| OPTION(bluestore_compression_min_blob_size, OPT_U32, 256*1024) | |||
| OPTION(bluestore_compression_max_blob_size, OPT_U32, 4*1024*1024) | |||
| OPTION(bluestore_blob_depth_for_gc, OPT_U32, 3) | |||
| OPTION(bluestore_merge_gc_data, OPT_BOOL, true) | |||
There was a problem hiding this comment.
bluestore_gc_max_blob_depth
bluestore_gc_merge_data
| @@ -7436,6 +7456,119 @@ void BlueStore::_wctx_finish( | |||
| } | |||
| } | |||
|
|
|||
| bool BlueStore::blobs_need_garbage_collection( | |||
There was a problem hiding this comment.
_blobs_need_garbage_collection
|
A few cosmetic nits, but otherwise this looks good to go. Thanks! |
liewegas
approved these changes
Sep 28, 2016
|
Sorry, one that thing: can you squash this into a single commit, make sure the commit message is prefixed with "os/bluestore:" (somethign like "os/bluestore: add garbage collection"), and make sure it has a Signed-off-by: line? Thanks! |
Signed-off-by: Roushan Ali <roushan.ali@sandisk.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Because of overwrite operation, a new blob is created instead of merging data with the old blob. This leads to overlapping of blobs(example given below).
...................
......................
......................
Because of overlapping blobs, space is wasted and read-performance can degrade. As part of this code change, we try to garbage collect the overlapping blobs. If the new write request results in overlapping count of more than 3, we extent the write request so that it completely overwrite the blobs adjacent to the new write request.
To keep track of the overlapping count, a new persistent field "blob_depth" is added to lextent. Whenever we add a new lextent, the "blob_depth" is set to the max of all the overlapping lextents + 1 (In fact we need not check all the overlapping lextents, we just need to check only two lextents covering the two ends of the new lextents) . This will not always give the correct depth. It may over count in certain scenarios but will never undercount. Note: if lextents are splitted at the blobs boundary, it may be possible to correctly keep track of the depth, but that will increase the lextent counts and make "depth" update operation complicated.
This pr is based on #10406 , rebased to latest code and fixed the review comments