Is it desirable to support specifying a level ?
Maybe there's a concern about using high compression levels, but
I'll start by asking if the feature is wanted at all.
Previous discussion at: 20210614012412.ga31...@telsasoft.com
>From cb30e17cf19fffa370a887d28d6d7e683d588b71 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 14 Mar 2021 17:12:07 -0500
Subject: [PATCH] Use GUC hooks to support compression:level..
..which is useful for zstd, but less so for lz4.
TODO:
windows, pglz, zlib case
---
doc/src/sgml/config.sgml | 6 +-
src/backend/access/transam/xlog.c | 20 ++++
src/backend/access/transam/xloginsert.c | 6 +-
src/backend/access/transam/xlogreader.c | 124 ++++++++++++++++++++
src/backend/utils/misc/guc_tables.c | 39 ++----
src/common/compression.c | 3 -
src/include/access/xlog.h | 6 +
src/include/access/xlogreader.h | 2 +
src/include/utils/guc_hooks.h | 3 +
src/test/regress/expected/compression.out | 19 +++
src/test/regress/expected/compression_1.out | 19 +++
src/test/regress/sql/compression.sql | 10 ++
12 files changed, 220 insertions(+), 37 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 77574e2d4ec..7b34ed630d5 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -3161,7 +3161,7 @@ include_dir 'conf.d'
</varlistentry>
<varlistentry id="guc-wal-compression" xreflabel="wal_compression">
- <term><varname>wal_compression</varname> (<type>enum</type>)
+ <term><varname>wal_compression</varname> (<type>string</type>)
<indexterm>
<primary><varname>wal_compression</varname> configuration parameter</primary>
</indexterm>
@@ -3169,7 +3169,7 @@ include_dir 'conf.d'
<listitem>
<para>
This parameter enables compression of WAL using the specified
- compression method.
+ compression method and optional compression level.
When enabled, the <productname>PostgreSQL</productname>
server compresses full page images written to WAL when
<xref linkend="guc-full-page-writes"/> is on or during a base backup.
@@ -3179,6 +3179,8 @@ include_dir 'conf.d'
was compiled with <option>--with-lz4</option>) and
<literal>zstd</literal> (if <productname>PostgreSQL</productname>
was compiled with <option>--with-zstd</option>).
+ A compression level may optionally be specified, by appending the level
+ number after a colon (<literal>:</literal>)
The default value is <literal>off</literal>.
Only superusers and users with the appropriate <literal>SET</literal>
privilege can change this setting.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 74bd1f5fbe2..e2d81129549 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -125,6 +125,8 @@ bool EnableHotStandby = false;
bool fullPageWrites = true;
bool wal_log_hints = false;
int wal_compression = WAL_COMPRESSION_NONE;
+char *wal_compression_string = "no"; /* Overwritten by GUC */
+int wal_compression_level = 1;
char *wal_consistency_checking_string = NULL;
bool *wal_consistency_checking = NULL;
bool wal_init_zero = true;
@@ -8118,6 +8120,24 @@ assign_xlog_sync_method(int new_sync_method, void *extra)
}
}
+bool
+check_wal_compression(char **newval, void **extra, GucSource source)
+{
+ int tmp;
+ if (get_compression_level(*newval, &tmp) != -1)
+ return true;
+
+ return false;
+}
+
+/* Parse the GUC into integers for wal_compression and wal_compression_level */
+void
+assign_wal_compression(const char *newval, void *extra)
+{
+ wal_compression = get_compression_level(newval, &wal_compression_level);
+ Assert(wal_compression >= 0);
+}
+
/*
* Issue appropriate kind of fsync (if any) for an XLOG output file.
diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c
index 47b6d16eaef..93003744ed0 100644
--- a/src/backend/access/transam/xloginsert.c
+++ b/src/backend/access/transam/xloginsert.c
@@ -906,8 +906,8 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
case WAL_COMPRESSION_LZ4:
#ifdef USE_LZ4
- len = LZ4_compress_default(source, dest, orig_len,
- COMPRESS_BUFSIZE);
+ len = LZ4_compress_fast(source, dest, orig_len,
+ COMPRESS_BUFSIZE, wal_compression_level);
if (len <= 0)
len = -1; /* failure */
#else
@@ -918,7 +918,7 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length,
case WAL_COMPRESSION_ZSTD:
#ifdef USE_ZSTD
len = ZSTD_compress(dest, COMPRESS_BUFSIZE, source, orig_len,
- ZSTD_CLEVEL_DEFAULT);
+ wal_compression_level);
if (ZSTD_isError(len))
len = -1; /* failure */
#else
diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c
index a61b4a99219..728ca9f18c4 100644
--- a/src/backend/access/transam/xlogreader.c
+++ b/src/backend/access/transam/xlogreader.c
@@ -17,7 +17,9 @@
*/
#include "postgres.h"
+#include <limits.h>
#include <unistd.h>
+
#ifdef USE_LZ4
#include <lz4.h>
#endif
@@ -30,6 +32,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecord.h"
#include "catalog/pg_control.h"
+#include "common/compression.h"
#include "common/pg_lzcompress.h"
#include "replication/origin.h"
@@ -56,6 +59,30 @@ static void ResetDecoder(XLogReaderState *state);
static void WALOpenSegmentInit(WALOpenSegment *seg, WALSegmentContext *segcxt,
int segsize, const char *waldir);
+static const struct {
+ char *name;
+ enum WalCompression compress_id; /* The internal ID */
+} wal_compression_options[] = {
+ {"pglz", WAL_COMPRESSION_PGLZ},
+
+#ifdef USE_LZ4
+ {"lz4", WAL_COMPRESSION_LZ4},
+#endif
+
+#ifdef USE_ZSTD
+ {"zstd", WAL_COMPRESSION_ZSTD},
+#endif
+
+ {"on", WAL_COMPRESSION_PGLZ},
+ {"off", WAL_COMPRESSION_NONE},
+ {"true", WAL_COMPRESSION_PGLZ},
+ {"false", WAL_COMPRESSION_NONE},
+ {"yes", WAL_COMPRESSION_PGLZ},
+ {"no", WAL_COMPRESSION_NONE},
+ {"1", WAL_COMPRESSION_PGLZ},
+ {"0", WAL_COMPRESSION_NONE},
+};
+
/* size of the buffer allocated for error message. */
#define MAX_ERRORMSG_LEN 1000
@@ -2035,6 +2062,103 @@ XLogRecGetBlockData(XLogReaderState *record, uint8 block_id, Size *len)
}
}
+/*
+ * Return the wal compression ID, or -1 if the input is
+ * invalid/unrecognized/unsupported.
+ * The compression level is stored in *level.
+ */
+int
+get_compression_level(const char *in, int *level)
+{
+ pg_compress_algorithm alg;
+ pg_compress_specification algdetail;
+ char *algorithm, *detail;
+ char *error_detail;
+ int compress_id = -1;
+
+ /* Parse the algorithm and any detail suffix */
+ parse_compress_options(in, &algorithm, &detail);
+
+ /* Try to find the WAL_COMPRESSION_* value for the algorithm */
+ for (int idx = 0; idx < lengthof(wal_compression_options); ++idx)
+ {
+ if (strcmp(wal_compression_options[idx].name, algorithm) == 0)
+ compress_id = wal_compression_options[idx].compress_id;
+ }
+
+ if (!parse_compress_algorithm(algorithm, &alg))
+ {
+ /*
+ * The compression algorithm wasn't a commonly-used algorithm name, but
+ * may be a supported wal_compression: pglz/on/off/etc.
+ */
+
+ if (compress_id >= 0 && detail != NULL)
+ {
+ /*
+ * Algorithms which aren't known by the generic parser don't
+ * support compression level or other options.
+ */
+#ifndef FRONTEND
+ GUC_check_errdetail("Compression algorithm does not support options: %s",
+ algorithm);
+#endif
+ return -1;
+ }
+
+ *level = 0; /* ignored */
+ return compress_id;
+ }
+ else
+ {
+ /* Show a useful error if the wal compression is known, but not supported */
+ if ((alg == PG_COMPRESSION_ZSTD || alg != PG_COMPRESSION_LZ4) &&
+ compress_id == -1)
+
+ {
+#ifndef FRONTEND
+ GUC_check_errdetail("Compression algorithm is not supported by this build: %s",
+ algorithm);
+#endif
+ return -1;
+ }
+
+ /*
+ * If the algorithm *is* known, then parse its compression level. Note
+ * that "none" is a supported compression algorithm, but not a valid
+ * option for wal_compression
+ */
+ parse_compress_specification(alg, detail, &algdetail);
+ error_detail = validate_compress_specification(&algdetail);
+ if (error_detail != NULL)
+ {
+#ifndef FRONTEND
+ GUC_check_errdetail("Could not parse compression detail: %s",
+ error_detail);
+#endif
+ return -1;
+ }
+
+ *level = algdetail.level;
+ if (algdetail.options != 0)
+ {
+#ifndef FRONTEND
+ GUC_check_errdetail("Compression options other than level= are not supported by wal_compression: %s",
+ detail);
+#endif
+ return -1;
+ }
+ }
+
+ return compress_id;
+
+ /*
+ * gzip or anything parsed by the generic parser which isn't a valid
+ * wal_compression value will get here.
+ */
+ return -1;
+}
+
/*
* Restore a full-page image from a backup block attached to an XLOG record.
*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 5025e80f89d..4a956ea11a3 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -433,25 +433,6 @@ static const struct config_enum_entry default_toast_compression_options[] = {
{NULL, 0, false}
};
-static const struct config_enum_entry wal_compression_options[] = {
- {"pglz", WAL_COMPRESSION_PGLZ, false},
-#ifdef USE_LZ4
- {"lz4", WAL_COMPRESSION_LZ4, false},
-#endif
-#ifdef USE_ZSTD
- {"zstd", WAL_COMPRESSION_ZSTD, false},
-#endif
- {"on", WAL_COMPRESSION_PGLZ, false},
- {"off", WAL_COMPRESSION_NONE, false},
- {"true", WAL_COMPRESSION_PGLZ, true},
- {"false", WAL_COMPRESSION_NONE, true},
- {"yes", WAL_COMPRESSION_PGLZ, true},
- {"no", WAL_COMPRESSION_NONE, true},
- {"1", WAL_COMPRESSION_PGLZ, true},
- {"0", WAL_COMPRESSION_NONE, true},
- {NULL, 0, false}
-};
-
/*
* Options for enum values stored in other modules
*/
@@ -4491,6 +4472,16 @@ struct config_string ConfigureNamesString[] =
check_wal_consistency_checking, assign_wal_consistency_checking, NULL
},
+ {
+ {"wal_compression", PGC_SUSET, WAL_SETTINGS,
+ gettext_noop("Compresses full-page writes written in WAL file with specified method."),
+ NULL
+ },
+ &wal_compression_string,
+ "no",
+ check_wal_compression, assign_wal_compression, NULL
+ },
+
{
{"jit_provider", PGC_POSTMASTER, CLIENT_CONN_PRELOAD,
gettext_noop("JIT provider to use."),
@@ -4749,16 +4740,6 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
- {
- {"wal_compression", PGC_SUSET, WAL_SETTINGS,
- gettext_noop("Compresses full-page writes written in WAL file with specified method."),
- NULL
- },
- &wal_compression,
- WAL_COMPRESSION_NONE, wal_compression_options,
- NULL, NULL, NULL
- },
-
{
{"wal_level", PGC_POSTMASTER, WAL_SETTINGS,
gettext_noop("Sets the level of information written to the WAL."),
diff --git a/src/common/compression.c b/src/common/compression.c
index 2d3e56b4d62..a2e7abb6a47 100644
--- a/src/common/compression.c
+++ b/src/common/compression.c
@@ -357,8 +357,6 @@ validate_compress_specification(pg_compress_specification *spec)
return NULL;
}
-#ifdef FRONTEND
-
/*
* Basic parsing of a value specified through a command-line option, commonly
* -Z/--compress.
@@ -418,4 +416,3 @@ parse_compress_options(const char *option, char **algorithm, char **detail)
*detail = pstrdup(sep + 1);
}
}
-#endif /* FRONTEND */
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index cfe5409738c..115257e52bb 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -16,6 +16,8 @@
#include "datatype/timestamp.h"
#include "lib/stringinfo.h"
#include "nodes/pg_list.h"
+#include "storage/fd.h"
+#include "utils/guc.h"
/* Sync methods */
@@ -54,6 +56,10 @@ extern PGDLLIMPORT int wal_decode_buffer_size;
extern PGDLLIMPORT int CheckPointSegments;
+extern PGDLLIMPORT char *wal_compression_string;
+extern PGDLLIMPORT int wal_compression;
+extern PGDLLIMPORT int wal_compression_level;
+
/* Archive modes */
typedef enum ArchiveMode
{
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index da64f99f0b3..f371fbc0e85 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -440,4 +440,6 @@ extern bool XLogRecGetBlockTagExtended(XLogReaderState *record, uint8 block_id,
BlockNumber *blknum,
Buffer *prefetch_buffer);
+extern int get_compression_level(const char *in, int *level);
+
#endif /* XLOGREADER_H */
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index aeb3663071f..fa87d8ae12c 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -155,4 +155,7 @@ extern bool check_wal_consistency_checking(char **newval, void **extra,
extern void assign_wal_consistency_checking(const char *newval, void *extra);
extern void assign_xlog_sync_method(int new_sync_method, void *extra);
+extern bool check_wal_compression(char **newval, void **extra, GucSource source);
+extern void assign_wal_compression(const char *newval, void *extra);
+
#endif /* GUC_HOOKS_H */
diff --git a/src/test/regress/expected/compression.out b/src/test/regress/expected/compression.out
index 4c997e2602f..72735f4f91a 100644
--- a/src/test/regress/expected/compression.out
+++ b/src/test/regress/expected/compression.out
@@ -360,3 +360,22 @@ ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -
ERROR: invalid compression method "i_do_not_exist_compression"
DROP TABLE badcompresstbl;
\set HIDE_TOAST_COMPRESSION true
+-- pglz doesn't accept a compression level:
+SET wal_compression = 'pglz:1';
+ERROR: invalid value for parameter "wal_compression": "pglz:1"
+DETAIL: Compression algorithm does not support options: pglz
+-- none and gzip aren't supported wal_compression algorithms:
+SET wal_compression = 'none';
+ERROR: invalid value for parameter "wal_compression": "none"
+DETAIL: Compression algorithm is not supported by this build: none
+SET wal_compression = 'gzip';
+ERROR: invalid value for parameter "wal_compression": "gzip"
+DETAIL: Compression algorithm is not supported by this build: gzip
+SET wal_compression = 'gzip:1';
+ERROR: invalid value for parameter "wal_compression": "gzip:1"
+DETAIL: Compression algorithm is not supported by this build: gzip
+-- wrong spelling with a dash instead of a colon:
+SET wal_compression = 'zstd-1';
+ERROR: invalid value for parameter "wal_compression": "zstd-1"
+SET wal_compression = 'lz4-1';
+ERROR: invalid value for parameter "wal_compression": "lz4-1"
diff --git a/src/test/regress/expected/compression_1.out b/src/test/regress/expected/compression_1.out
index c0a47646eb2..e9c053cb56b 100644
--- a/src/test/regress/expected/compression_1.out
+++ b/src/test/regress/expected/compression_1.out
@@ -354,3 +354,22 @@ ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -
ERROR: invalid compression method "i_do_not_exist_compression"
DROP TABLE badcompresstbl;
\set HIDE_TOAST_COMPRESSION true
+-- pglz doesn't accept a compression level:
+SET wal_compression = 'pglz:1';
+ERROR: invalid value for parameter "wal_compression": "pglz:1"
+DETAIL: Compression algorithm does not support options: pglz
+-- none and gzip aren't supported wal_compression algorithms:
+SET wal_compression = 'none';
+ERROR: invalid value for parameter "wal_compression": "none"
+DETAIL: Compression algorithm is not supported by this build: none
+SET wal_compression = 'gzip';
+ERROR: invalid value for parameter "wal_compression": "gzip"
+DETAIL: Compression algorithm is not supported by this build: gzip
+SET wal_compression = 'gzip:1';
+ERROR: invalid value for parameter "wal_compression": "gzip:1"
+DETAIL: Compression algorithm is not supported by this build: gzip
+-- wrong spelling with a dash instead of a colon:
+SET wal_compression = 'zstd-1';
+ERROR: invalid value for parameter "wal_compression": "zstd-1"
+SET wal_compression = 'lz4-1';
+ERROR: invalid value for parameter "wal_compression": "lz4-1"
diff --git a/src/test/regress/sql/compression.sql b/src/test/regress/sql/compression.sql
index 86332dcc510..5a3cb324220 100644
--- a/src/test/regress/sql/compression.sql
+++ b/src/test/regress/sql/compression.sql
@@ -151,3 +151,13 @@ ALTER TABLE badcompresstbl ALTER a SET COMPRESSION I_Do_Not_Exist_Compression; -
DROP TABLE badcompresstbl;
\set HIDE_TOAST_COMPRESSION true
+
+-- pglz doesn't accept a compression level:
+SET wal_compression = 'pglz:1';
+-- none and gzip aren't supported wal_compression algorithms:
+SET wal_compression = 'none';
+SET wal_compression = 'gzip';
+SET wal_compression = 'gzip:1';
+-- wrong spelling with a dash instead of a colon:
+SET wal_compression = 'zstd-1';
+SET wal_compression = 'lz4-1';
--
2.25.1
