Re: Compression of bigger WAL records

Hello

+static void
+AllocCompressionBuffers(void)
+{
+ uint32 new_size = wal_compression_buffer;

This is called in the assign hook - and isn't that called before the
global variable is updated?

+ compressed_header->method = XLR_COMPRESS_LZ4;
+ compr_len = LZ4_compress_default((char *) &src_header[1], (char *)
&compressed_header[1],
+ orig_len, compressed_data_size);

compressed_header[1] has an offset of 32, but compressed_data_size
refers to the entire size, isn't there a possible buffer overrun here?
Same with ZSTD.

+/* Header prepended to a whole-record compressed WAL record */
+typedef struct XLogCompressionHeader
+{
+ XLogRecord record_header;
+ uint8 method; /* XLR_COMPRESS_* */
+ uint32 decompressed_length;
+} XLogCompressionHeader;

This has 3 bytes of uninitialized padding, is that okay? I remember
seeing a separate thread about possibly cleaning these up not long
ago.

+# Enable WAL compression for recovery tests.
+# lz4 is used here; 052_wal_compression.pl separately tests all methods.
+wal_compression = 'lz4'

Doesn't this test needs a check to require a build with compression flags?

+#wal_compression = lz4 # enables compression of
full-page writes;

But the default value is still off, isn't this example misleading?

+ total_len_decomp = -1; /* XLogCompressionHeader spans pages */

at multiple places, but this is an unsigned variable.

+ variable => 'wal_compression_buffer',
+ boot_val => '295972',
+ min => '295972',

Doesn't guc_params.dat support using macros instead of this hardcoded
magic number?