Discussion:
Review request #3964: L2ARC should always compress metadata buffers
Saso Kiselkov
2013-08-02 11:14:55 UTC
Permalink
Here's a quick review. ATM when strictly bind data compression in the
L2ARC to the 'compression' setting on the dataset, even though metadata
is always compressed on the pool regardless of this setting. As a
result, unless compression is enabled on the dataset, we won't compress
it in L2ARC, leading to some potentially suboptimal corner cases (e.g.
compress=off and secondarycache=metadata means we won't compress
anything in the L2ARC, even though it is all metadata which is
compressible). This patch changes the behavior so that metadata blocks
are always compressed in the L2ARC (in accordance with how we deal with
them on the main pool) and user-data blocks are compressed according to
the 'compress' property setting on the dataset.

Issue: https://www.illumos.org/issues/3964
Webrev: http://cr.illumos.org/~webrev/skiselkov/3964/

Cheers,
--
Saso
Matthew Ahrens
2013-08-02 17:20:33 UTC
Permalink
Good idea Saso. Changes look good to me.

--matt
Post by Saso Kiselkov
Here's a quick review. ATM when strictly bind data compression in the
L2ARC to the 'compression' setting on the dataset, even though metadata
is always compressed on the pool regardless of this setting. As a
result, unless compression is enabled on the dataset, we won't compress
it in L2ARC, leading to some potentially suboptimal corner cases (e.g.
compress=off and secondarycache=metadata means we won't compress
anything in the L2ARC, even though it is all metadata which is
compressible). This patch changes the behavior so that metadata blocks
are always compressed in the L2ARC (in accordance with how we deal with
them on the main pool) and user-data blocks are compressed according to
the 'compress' property setting on the dataset.
Issue: https://www.illumos.org/issues/3964
Webrev: http://cr.illumos.org/~webrev/skiselkov/3964/
Cheers,
--
Saso
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
https://www.listbox.com/member/archive/rss/182191/21635000-ebd1d460
https://www.listbox.com/member/?&
Powered by Listbox: http://www.listbox.com
-------------------------------------------
illumos-zfs
Archives: https://www.listbox.com/member/archive/182191/=now
RSS Feed: https://www.listbox.com/member/archive/rss/182191/23047029-187a0c8d
Modify Your Subscription: https://www.listbox.com/member/?member_id=23047029&id_secret=23047029-2e85923f
Powered by Listbox: http://www.listbox.com
Saso Kiselkov
2013-08-02 18:00:28 UTC
Permalink
Post by Matthew Ahrens
Good idea Saso. Changes look good to me.
Thanks for the review, but to give credit where due, the idea came from
Brian Behlendorf with the ZFS on Linux project.

Cheers,
--
Saso
Loading...