-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
On 6/2/14, 10:27 AM, Matthew Ahrens wrote:
> We've seen that overall, LZ4 is a better compression algorithm
> than LZJB. It compresses and decompresses faster, and gets a
> better compression ratio. It's been in use long enough for us to
> shake out the few initial problems with the implementation.
>
> I think we should change the default compression algorithm in ZFS
> to LZ4. Specifically, if the LZ4 feature is enabled (e.g. via
> "zpool set feature@lz4_compress=enabled"), and you do "zfs set
> compression=on", then it will compress with LZ4 rather than LZJB..
> We should probably also compress metadata with LZ4 if the feature
> is enabled (after verifying that indirect blocks compress at least
> as well as LZ4).
>
> I am pretty busy with other ZFS work at the moment, but I'd be
> happy to guide / mentor someone from the community who would like
> to implement this. It would also make a good hackathon project at
> the OpenZFS Developer Summit
> <http://www.open-zfs.org/wiki/OpenZFS_Developer_Summit_2014>
> (November 2014 in San Francisco).
This reminds me -- I have talked with Martin Matuska ([email protected])
a while ago on this idea and eventually I have the attached patch (for
FreeBSD, I have personally running it on my own storage system for
quite a while now).
I was relented to push it for discussion because I don't have any
scientific testing data and don't have free cycles to test it against
some corner cases (e.g. running with v28 pools, etc.).
It also uses LZ4 for metadata when the feature is active, by the way.
I took a different approach than Daniil's version but other than not
activating it, the result should be the same.
(Unscientific data from my storage system shows a clear win for using
LZ4 to compress metadata, though).
Cheers,
-----BEGIN PGP SIGNATURE-----
iQIcBAEBCgAGBQJTl+L6AAoJEJW2GBstM+nsJ0wP/3S58WnJ5muhMcClLNNaoZGK
Z/CsXRVm7qTylMFB9fHhlMgb1IsAKXdyiz82RGa6LWuAQecrQksaRV40Tb8ntGU2
EttTmXoXWDF2zGbyoWyHnxne/ODc3rhfWoNGKzrElGnoARlGfA9Eri9Kr5u0xGI8
e9R/flitCz8duzXQluLjl+DYRAWUw2Tx+MyoFhOtqkGBOf6QmrrYTg2eXqN28n4s
EW55kqb3825adY7WuOZeIYa4AlK5owCqooOgnELaLSFPFcGFwLvx8rQmJ6UdKdlv
h0lfATfK7A9NOglRpJ3PafVz+cpZCoI4plz6I5kdeShK0a9P0INgVbis2nEv7aZS
Hn/rI1l4zRF7ssv+Ke9aAunSpRrgFoZvdP+BrBmvrBdc8M2WMzKbxmpJpH0B0u7q
acXU6L469nD0U/oCHAnAW1Usw2ZCBrzHWVzbpwlbk3hGg9TqNiNcRfcmDAYefZzB
PMIijLX4YSI9tyW32zEuyz73LkN/ZCuWcAuf5m9Cpk6zHBPeJ5z7HHX+arVHlmkf
pZRqa0ON9rdXsuvOt6BGNUmk1uKP5/7ftbIbFnCuuWufcBbo5QOQas83lGZ4cr8p
FXatM6fcQFNgAya++x3FimYWb7qw73jvpPIHhi6GYzfGRErFr9UZKZg74n4IlPgs
VnajGHXxo6FIHJL+1AU2
=d+4k
-----END PGP SIGNATURE-----
diff --git a/sys/cddl/boot/zfs/zfsimpl.h b/sys/cddl/boot/zfs/zfsimpl.h
index 7bc4c43..61e61d3 100644
--- a/sys/cddl/boot/zfs/zfsimpl.h
+++ b/sys/cddl/boot/zfs/zfsimpl.h
@@ -444,7 +444,7 @@ enum zio_compress {
ZIO_COMPRESS_FUNCTIONS
};
-#define ZIO_COMPRESS_ON_VALUE ZIO_COMPRESS_LZJB
+#define ZIO_COMPRESS_ON_VALUE ZIO_COMPRESS_LZ4
#define ZIO_COMPRESS_DEFAULT ZIO_COMPRESS_OFF
/* nvlist pack encoding */
diff --git a/sys/cddl/contrib/opensolaris/common/zfs/zfeature_common.c
b/sys/cddl/contrib/opensolaris/common/zfs/zfeature_common.c
index 28b0c9e..ea78d99 100644
--- a/sys/cddl/contrib/opensolaris/common/zfs/zfeature_common.c
+++ b/sys/cddl/contrib/opensolaris/common/zfs/zfeature_common.c
@@ -168,7 +168,7 @@
zfeature_register(SPA_FEATURE_LZ4_COMPRESS,
"org.illumos:lz4_compress", "lz4_compress",
"LZ4 compression algorithm support.", B_FALSE, B_FALSE,
- B_FALSE, NULL);
+ B_TRUE, NULL);
zfeature_register(SPA_FEATURE_MULTI_VDEV_CRASH_DUMP,
"com.joyent:multi_vdev_crash_dump", "multi_vdev_crash_dump",
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
index 7da25c7..c3daeda 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
@@ -1657,7 +1657,8 @@
* that specializes in arrays of bps.
*/
compress = zfs_mdcomp_disable ? ZIO_COMPRESS_EMPTY :
- ZIO_COMPRESS_LZJB;
+ zio_compress_select(dmu_objset_spa(os),
+ ZIO_COMPRESS_ON, ZIO_COMPRESS_ON_VALUE);
/*
* Metadata always gets checksummed. If the data
@@ -1682,7 +1683,8 @@
compress = ZIO_COMPRESS_OFF;
checksum = ZIO_CHECKSUM_NOPARITY;
} else {
- compress = zio_compress_select(dn->dn_compress, compress);
+ compress = zio_compress_select(dmu_objset_spa(os),
+ dn->dn_compress, compress);
checksum = (dedup_checksum == ZIO_CHECKSUM_OFF) ?
zio_checksum_select(dn->dn_checksum, checksum) :
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
index b5c0f5f..003242a 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu_objset.c
@@ -149,7 +149,8 @@
*/
ASSERT(newval != ZIO_COMPRESS_INHERIT);
- os->os_compress = zio_compress_select(newval, ZIO_COMPRESS_ON_VALUE);
+ os->os_compress = zio_compress_select(dmu_objset_spa(os),
+ newval, ZIO_COMPRESS_ON_VALUE);
}
static void
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
index 41960b5..3bb88b4 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
@@ -115,14 +115,14 @@ enum zio_compress {
};
/* N.B. when altering this value, also change BOOTFS_COMPRESS_VALID below */
-#define ZIO_COMPRESS_ON_VALUE ZIO_COMPRESS_LZJB
+#define ZIO_COMPRESS_ON_VALUE ZIO_COMPRESS_LZ4
#define ZIO_COMPRESS_DEFAULT ZIO_COMPRESS_OFF
#define BOOTFS_COMPRESS_VALID(compress) \
((compress) == ZIO_COMPRESS_LZJB || \
(compress) == ZIO_COMPRESS_LZ4 || \
((compress) == ZIO_COMPRESS_ON && \
- ZIO_COMPRESS_ON_VALUE == ZIO_COMPRESS_LZJB) || \
+ ZIO_COMPRESS_ON_VALUE == ZIO_COMPRESS_LZ4) || \
(compress) == ZIO_COMPRESS_OFF)
#define ZIO_FAILURE_MODE_WAIT 0
@@ -568,8 +568,8 @@ extern enum zio_checksum zio_checksum_select(enum
zio_checksum child,
enum zio_checksum parent);
extern enum zio_checksum zio_checksum_dedup_select(spa_t *spa,
enum zio_checksum child, enum zio_checksum parent);
-extern enum zio_compress zio_compress_select(enum zio_compress child,
- enum zio_compress parent);
+extern enum zio_compress zio_compress_select(spa_t *spa,
+ enum zio_compress child, enum zio_compress parent);
extern void zio_suspend(spa_t *spa, zio_t *zio);
extern int zio_resume(spa_t *spa);
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
index 5fd9c67..0c8db9a 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_ioctl.c
@@ -2489,16 +2489,21 @@ static int zfs_fill_zplprops_root(uint64_t, nvlist_t *,
nvlist_t *,
}
case ZFS_PROP_COMPRESSION:
{
- if (intval == ZIO_COMPRESS_LZ4) {
- spa_t *spa;
+ spa_t *spa;
- if ((err = spa_open(dsname, &spa, FTAG)) != 0)
- return (err);
+ if ((err = spa_open(dsname, &spa, FTAG)) != 0)
+ return (err);
- /*
- * Setting the LZ4 compression algorithm activates
- * the feature.
- */
+ /*
+ * Setting the LZ4 compression algorithm activates
+ * the feature.
+ *
+ * When LZ4 compression algorithm is enabled, enabling
+ * compression is equavilent to using LZ4.
+ */
+ if ((intval == ZIO_COMPRESS_LZ4) ||
+ (intval == ZIO_COMPRESS_ON && spa_feature_is_enabled(spa,
+ SPA_FEATURE_LZ4_COMPRESS)) ) {
if (!spa_feature_is_active(spa,
SPA_FEATURE_LZ4_COMPRESS)) {
if ((err = zfs_prop_activate_feature(spa,
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio_compress.c
b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio_compress.c
index 30994a4..89e2cbb 100644
--- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio_compress.c
+++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio_compress.c
@@ -35,6 +35,7 @@
#include <sys/compress.h>
#include <sys/kstat.h>
#include <sys/spa.h>
+#include <sys/zfeature.h>
#include <sys/zio.h>
#include <sys/zio_compress.h>
@@ -83,19 +84,29 @@
};
enum zio_compress
-zio_compress_select(enum zio_compress child, enum zio_compress parent)
+zio_compress_select(spa_t *spa, enum zio_compress child, enum zio_compress
parent)
{
+ enum zio_compress method = ZIO_COMPRESS_FUNCTIONS;
+
ASSERT(child < ZIO_COMPRESS_FUNCTIONS);
ASSERT(parent < ZIO_COMPRESS_FUNCTIONS);
ASSERT(parent != ZIO_COMPRESS_INHERIT && parent != ZIO_COMPRESS_ON);
if (child == ZIO_COMPRESS_INHERIT)
- return (parent);
-
- if (child == ZIO_COMPRESS_ON)
- return (ZIO_COMPRESS_ON_VALUE);
+ method = parent;
+ else if (child == ZIO_COMPRESS_ON)
+ method = ZIO_COMPRESS_ON_VALUE;
+ else
+ method = child;
+
+ if (method == ZIO_COMPRESS_ON_VALUE) {
+ if (spa == NULL || !spa_feature_is_active(spa,
+ SPA_FEATURE_LZ4_COMPRESS))
+ method = ZIO_COMPRESS_LZJB;
+ }
- return (child);
+ ASSERT(method < ZIO_COMPRESS_FUNCTIONS);
+ return (method);
}
size_t
_______________________________________________
developer mailing list
[email protected]
http://lists.open-zfs.org/mailman/listinfo/developer
