Re: Fixes for compression options of pg_receivewal and refactoring of backup_compression.{c,h}

[Michael Paquier]

On Tue, Apr 12, 2022 at 06:22:48PM +0900, Michael Paquier wrote:
> On Mon, Apr 11, 2022 at 12:46:02PM +0000, gkokola...@pm.me wrote:
>> It looks good. If you choose to discard the comment regarding the use of
>> 'method' over 'algorithm' from above, can you please use the full word in the
>> variable, e.g. 'wal_compress_algorithm' instead of 'wal_compress_algo'. I can
>> not really explain it, the later reads a bit rude. Then again that may be 
>> just
>> me.
> 
> Thanks.  I have been able to do an extra pass on 0001 and 0002, fixing
> those naming inconsistencies with "algo" vs "algorithm" that you and
> Robert have reported, and applied them.  For 0003, I'll look at it
> later.  Attached is a rebase with improvements about the variable
> names.

This has been done with the proper renames.  With that in place, I see
no reason now to not be able to set the compression level as it is
possible to pass it down with the options available.  This requires
only a couple of lines, as of the attached.  LZ4 has a dummy structure
called LZ4F_INIT_PREFERENCES to initialize LZ4F_preferences_t, that
holds the compression level before passing it down to
LZ4F_compressBegin(), but that's available only in v1.8.3.  Using it
risks lowering down the minimal version of LZ4 we are able to use now,
but replacing that with a set of memset()s is also a way to set up
things as per its documentation.

Thoughts?
--
Michael

diff --git a/src/bin/pg_basebackup/walmethods.c b/src/bin/pg_basebackup/walmethods.c
index d5bcc208a9..1b3cc5c4f7 100644
--- a/src/bin/pg_basebackup/walmethods.c
+++ b/src/bin/pg_basebackup/walmethods.c
@@ -165,6 +165,7 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 	{
 		size_t		ctx_out;
 		size_t		header_size;
+		LZ4F_preferences_t prefs;
 
 		ctx_out = LZ4F_createCompressionContext(&ctx, LZ4F_VERSION);
 		if (LZ4F_isError(ctx_out))
@@ -177,8 +178,13 @@ dir_open_for_write(const char *pathname, const char *temp_suffix, size_t pad_to_
 		lz4bufsize = LZ4F_compressBound(LZ4_IN_SIZE, NULL);
 		lz4buf = pg_malloc0(lz4bufsize);
 
+		/* assign the compression level, default is 0 */
+		memset(&prefs, 0, sizeof(prefs));
+		memset(&prefs.frameInfo, 0, sizeof(prefs.frameInfo));
+		prefs.compressionLevel = dir_data->compression_level;
+
 		/* add the header */
-		header_size = LZ4F_compressBegin(ctx, lz4buf, lz4bufsize, NULL);
+		header_size = LZ4F_compressBegin(ctx, lz4buf, lz4bufsize, &prefs);
 		if (LZ4F_isError(header_size))
 		{
 			dir_data->lasterrstring = LZ4F_getErrorName(header_size);

signature.asc
Description: PGP signature