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

os/bluestore: change algorithm of compression header from string to int #10137

Merged
merged 1 commit into from Jul 8, 2016

Conversation

xiexingguo
Copy link
Member

@xiexingguo xiexingguo commented Jul 5, 2016

The literal description of compression algorithm can vary from
various compression types and thus increases the complexity of
en/decoding, which as a result can cause chaos. Also it can be
more efficient by translating it into a fixed-length type.

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@@ -5770,7 +5770,9 @@ void BlueStore::_do_write_small(
bdev->aio_write(offset, t,
&txc->ioc, wctx->buffered);
});
b->blob.calc_csum(b_off, padded);
if (b->blob.csum_type) {
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO no much sense in this fix as calc_csum performs verification by itself - see corresponding switch inside...

@xiexingguo xiexingguo force-pushed the xxg-wip-bluestore-2016-07-05-02 branch from c5e61a2 to cb39a0b Compare July 6, 2016 02:29
@xiexingguo xiexingguo changed the title os/bluestore: only calculate csum when necessary os/bluestore: change algorithm of compression header from string to int Jul 6, 2016

return alg;
}

Copy link
Member

Choose a reason for hiding this comment

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

I think these all belong in bluestoer_types.h and cc, probably along side CSUM_*

@xiexingguo xiexingguo force-pushed the xxg-wip-bluestore-2016-07-05-02 branch from cb39a0b to a1af00f Compare July 7, 2016 01:01
@xiexingguo
Copy link
Member Author

All fixed.

@liewegas
Copy link
Member

liewegas commented Jul 7, 2016

@ifed01 look ok to you?

@@ -621,11 +645,11 @@ struct bluestore_wal_transaction_t {
WRITE_CLASS_ENCODER(bluestore_wal_transaction_t)

struct bluestore_compression_header_t {
std::string type;
uint8_t type = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

IMHO better use 'type = COM_ALG_NONE' here

@xiexingguo xiexingguo force-pushed the xxg-wip-bluestore-2016-07-05-02 branch from a1af00f to 24a15cb Compare July 7, 2016 22:21
@xiexingguo
Copy link
Member Author

@ifed01 See again?

@xiexingguo xiexingguo force-pushed the xxg-wip-bluestore-2016-07-05-02 branch from 24a15cb to ebff755 Compare July 8, 2016 08:12
return COMP_ALG_SNAPPY;
if (s == "zlib")
return COMP_ALG_ZLIB;

Copy link
Contributor

Choose a reason for hiding this comment

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

What's about reverse mapping for None ?

Copy link
Member Author

Choose a reason for hiding this comment

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

@ifed01 Done, thanks!

The literal description of compression algorithm can vary from
various compression types and thus increases the complexity of
en/decoding, which as a result can cause chaos. Also it can be
more efficient by translating it into a fixed-length type.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
@xiexingguo xiexingguo force-pushed the xxg-wip-bluestore-2016-07-05-02 branch from ebff755 to 1209cb3 Compare July 8, 2016 11:39
@ifed01 ifed01 merged commit 5c2a1bb into ceph:master Jul 8, 2016
@xiexingguo xiexingguo deleted the xxg-wip-bluestore-2016-07-05-02 branch July 8, 2016 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants