On Mon, Jun 21, 2021 at 10:13:58PM -0500, Justin Pryzby wrote:
> @Michael: I assume that if you merge this patch, you'd set your animals to use
> wal_compression=lz4, and then they would fail the recovery tests.
Yes, I'd like to do that on my animal dangomushi.
> So the patches that you say are unrelated still seem to me to be a
> prerequisite .
I may be missing something, of course, but I still don't see why
that's necessary? We don't get any test failures on HEAD by switching
wal_compression to on, no? That's easy enough to test with a two-line
change that changes the default.
> +/* compression methods supported */
> +#define BKPIMAGE_COMPRESS_PGLZ 0x04
> +#define BKPIMAGE_COMPRESS_ZLIB 0x08
> +#define BKPIMAGE_COMPRESS_LZ4 0x10
> +#define BKPIMAGE_COMPRESS_ZSTD 0x20
> +#define BKPIMAGE_IS_COMPRESSED(info) \
> + ((info & (BKPIMAGE_COMPRESS_PGLZ | BKPIMAGE_COMPRESS_ZLIB | \
> + BKPIMAGE_COMPRESS_LZ4 | BKPIMAGE_COMPRESS_ZSTD)) != 0)
>
> You encouraged saving bits here, so I'm surprised to see that your patches
> use one bit per compression method: 2 bits to support no/pglz/lz4, 3 to add
> zstd, and the previous patch used 4 bits to also support zlib.
Yeah, I know. I have just finished with that to get something
readable for the sake of the tests. As you say, the point is moot
just we add one new method, anyway, as we need just one new bit.
And that's what I would like to do for v15 with LZ4 as the resulting
patch is simple. It would be an idea to discuss more compression
methods here once we hear more from users when this is released in the
field, re-considering at this point if more is necessary or not.
--
Michael