Date: Mon, 21 Jun 2021 22:13:58 -0500
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>
Subject: Re: Different compression methods for FPI
Message-ID: <20210622031358.GF29179@telsasoft.com>
References: <20210313012820.GJ29463@telsasoft.com>
 <BEF3B1E0-0B31-4F05-8E0A-F681CB918626@yandex-team.ru>
 <20210315003135.GL29463@telsasoft.com>
 <20210315180918.GW29463@telsasoft.com>
 <20210321193004.GA4203@telsasoft.com>
 <YKIelANwse+k2JxT@paquier.xyz>
 <20210519030618.GS373@telsasoft.com>
 <YKTa4+kTFkb3EIBs@paquier.xyz>
 <20210525044444.GV3676@telsasoft.com>
 <CAFiTN-thohtHGzp4dJ1QWc359qb43mVfrELeVcS5BwFuquRLtg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAFiTN-thohtHGzp4dJ1QWc359qb43mVfrELeVcS5BwFuquRLtg@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210622031358.GF29179%40telsasoft.com>
Precedence: bulk

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