Re: Different compression methods for FPI

Justin Pryzby <pryzby@telsasoft.com>

From: Justin Pryzby <pryzby@telsasoft.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Andrey Borodin <x4mmm@yandex-team.ru>, pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>
Date: 2021-06-22T03:13:58Z
Lists: pgsql-hackers
On Tue, May 25, 2021 at 12:05:19PM +0530, Dilip Kumar wrote:
> +++ b/src/test/recovery/t/011_crash_recovery.pl
> @@ -14,7 +14,7 @@ use Config;
>  plan tests => 3;
> 
>  my $node = get_new_node('primary');
> -$node->init(allows_streaming => 1);
> +$node->init();
>  $node->start;
> 
> How this change is relevant?

It's necessary for the tests to pass - see the prior discussions.
Revert them and the tests fail.

time make -C src/test/recovery check
#   Failed test 'new xid after restart is greater'

@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.  So the
patches that you say are unrelated still seem to me to be a prerequisite.

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>                                                                                                                                                                                  
Subject: [PATCH v8 2/9] Run 011_crash_recovery.pl with wal_level=minimal                                                                                                                                                           

From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>                                                                                                                                                                                  
Subject: [PATCH v8 3/9] Make sure published XIDs are persistent                                                                                                                                                                    

+/* 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.

There are spare bits available for that, but now there can be an inconsistency
if two bits are set.  Also, 2 bits could support 4 methods (including "no").

-- 
Justin



Commits

  1. Add more error context to RestoreBlockImage() and consume it

  2. Add support for LZ4 with compression of full-page writes in WAL

  3. Extended statistics on expressions

  4. Be clear about whether a recovery pause has taken effect.

  5. Add GUC to enable compression of full page images stored in WAL.