Skip to content

os/bluestore: new write path (checksums and compression)#9228

Merged
liewegas merged 136 commits intoceph:masterfrom
liewegas:wip-bluestore-write
Jun 1, 2016
Merged

os/bluestore: new write path (checksums and compression)#9228
liewegas merged 136 commits intoceph:masterfrom
liewegas:wip-bluestore-write

Conversation

@liewegas
Copy link
Member

@liewegas liewegas commented May 20, 2016

This branch completely replaces the read and write path with a new design.
The disk format has changed, and the data structures are very different.

Compression is only partially supported, but checksums work.

ceph_test_objectstore passes all tests.

@liewegas liewegas force-pushed the wip-bluestore-write branch from 0ad2c82 to dc44dda Compare May 20, 2016 15:27
@liewegas liewegas force-pushed the wip-bluestore-write branch from 292d9fc to 6b799e8 Compare May 20, 2016 17:30
@liewegas liewegas mentioned this pull request May 20, 2016
Closed
@liewegas liewegas force-pushed the wip-bluestore-write branch from e383028 to 812c5b6 Compare May 23, 2016 14:08
ifed01
ifed01 reviewed May 24, 2016
View reviewed changes
src/common/config_opts.h
OPTION(bluestore_min_alloc_size_ssd, OPT_U32, 4*1024)
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)
Copy link
Contributor

@ifed01 ifed01 May 24, 2016

Choose a reason for hiding this comment

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

Perhaps we should consider blob size specification in allocation units rather than absolute units. This way we'll always have blobs aligned with alloc units
E.g.
bluestore_compression_min_blob_size = 8 // i.e = 8 * bluestore_mon_alloc_size = 8 * 64 * 1024 bytes

Copy link
Member Author

@liewegas liewegas May 24, 2016 via email

Choose a reason for hiding this comment

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

@markhpc
Copy link
Member

markhpc commented May 25, 2016

Am marking this performance as I want to track this both for the compression support performance characteristics and the other potential impacts on the write path.

ifed01
ifed01 reviewed May 26, 2016
View reviewed changes
src/os/bluestore/BlueStore.cc
alg = "snappy";
} else if (g_conf->bluestore_compression_algorithm == "zlib") {
alg = "zlib";
} else if (g_conf->bluestore_compression_algorithm.length()) {
Copy link
Contributor

@ifed01 ifed01 May 26, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

We can burden Compressor with the check for known compression algorithm instead of doing that here. This way adding new algorithm becomes transparent for the BlueStore. In fact Compressor already has that check..

@liewegas liewegas force-pushed the wip-bluestore-write branch 2 times, most recently from 9b8e7d0 to ba69bc0 Compare May 31, 2016 18:34
@liewegas liewegas force-pushed the wip-bluestore-write branch from f314c23 to e854964 Compare June 1, 2016 15:27
Igor Fedotov and others added 10 commits June 1, 2016 11:38
@liewegas
os/bluestore: data structure to support compression/checksum verifica…
522c751 
…tion. Read handler prototype implementation.

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: add map and map_bl methods to blob
9ed60d8 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: keep an extent_ref_map for each blob
ac2c808 
This lets us track which portions of a blob are still in use.  In some
cases, we may be able to split the blob to deallocate a portion of it.
In other cases, we will want this information to know whether to
recompress the blob (or whatever).

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: fix blob operator<<
cc2cd2b 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: print extents in hex
78f6def 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: use pextent for wal_op
e6e91c3 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: use pextent_t for releases
f230672 
Trying to remove the old extent_t

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: add blob_map to onode_t
f132f15 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: pass c to _do_read
730f24f 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: add blob_map to enode
c22ecc1 
We'll remove the old ref_map once the users go away.

Signed-off-by: Sage Weil <sage@redhat.com>
liewegas and others added 27 commits June 1, 2016 11:40
@liewegas
os/bluestore: do not partially deallocate compressed blobs
502473a 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: compress on write
fe6aaca 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
compressor: Extends decompressor interface to be able to provide comp…
183db05 
…ressed data length.

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: ~0x -> ~
ea1a787 
e.g., 0x432da000~1000 instead of 0x432da000~0x1000

I think it's sufficiently clear that the value after ~ should have the same
base as the first bit, and it's easier to read.  And less text.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: fix offset skew check
a9a5e63 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: drop unused _pad_zeros args
24578bc 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: drop unused _pad_* methods
b0cabb7 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: avoid unnecessary write_onode calls
3d49c2e 
_wctx_finish callers always write the onode; we only need to worry about
our changes to the bnode.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: consolidate WriteContext items into a write_item
eb977e6 
Also include b_off in there.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: use WriteContext and do_alloc_write for _do_write_small
326bb0f 
Kill some mostly-duplicated code

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: Removes legacy block_size retrieval
5361cb8 
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
ceph_test_objectstore: extends SimpleObjectTest with the case where w…
b0981b3 
…rite happens for neighboring csum blocks to verify for potential alignment issue

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: Fixes Bnode serialization/deserialization and removes l…
c7ed3aa 
…egacy Bnode::ref_map

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: Enables cow for cloning at bluestore for store test
e2f6a66 
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: Cosmetic fixes in bluestore logging
8f8fd3a 
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: Fixes configuration observation.
c307c0f 
Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
ceph_test_objecstore: Adds a test case for compression stuff verifica…
6cec4f3 
…tion (incomplete)

Signed-off-by: Igor Fedotov <ifedotov@mirantis.com>
@liewegas
os/bluestore: use bufferptr for csum_data
6327989 
encode/decode of vector<char> is not optimized.  Bufferptr is a more
natural type here anyway.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: simple per-collection lru for buffers
6e5249a 
Size these using a global config.  This is only a starting point--we'll
obviously have to rework this to share memory across collections.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: keep intrusive_list of WRITING buffers
74dd955 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: populate buffer cache on read
7679407 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: no buffered write in wal path
247bcdb 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: do not rely on bdev buffered reads in read path
b6d7c97 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: do not use buffered bdev in write path
029ced9 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: add FLAG_NOCACHE flag; do not cache unbuffered writes
cb7f0f7 
Add a Buffer flag to mark that a buffer should not be cached once it is
stable.

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: fsck: use common helper to verify blobs and refs
380392f 
The checks are the same (or should be--we had missed a few).

Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas
os/bluestore: fsck: check for dup overlay keys
1c2c6cc 
Signed-off-by: Sage Weil <sage@redhat.com>
@liewegas liewegas force-pushed the wip-bluestore-write branch from e854964 to 1c2c6cc Compare June 1, 2016 15:41
@liewegas liewegas merged commit 9c8b085 into ceph:master Jun 1, 2016
@liewegas liewegas deleted the wip-bluestore-write branch August 12, 2016 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bluestore core feature performance

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@liewegas @markhpc @cooljiansir @ifed01 @dheart-joe @chhabaramesh