Re: Introduce pg_receivewal gzip compression tests

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐

On Monday, July 12th, 2021 at 11:42, <gkokolatos@pm.me> wrote:

> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
>
> On Monday, July 12th, 2021 at 08:42, Michael Paquier michael@paquier.xyz wrote:
>
> > On Fri, Jul 09, 2021 at 11:26:58AM +0000, Georgios wrote:
> >
> > > As suggested on a different thread [1], pg_receivewal can increase it's test
> > >
> > > coverage. There exists a non trivial amount of code that handles gzip
> > >
> > > compression. The current patch introduces tests that cover creation of gzip
> > >
> > > compressed WAL files and the handling of gzip partial segments. Finally the
> > >
> > > integrity of the compressed files is verified.
> >
> > -           # Verify compressed file's integrity
> >
> >
> > -           my $gzip_is_valid = system_log('gzip', '--test', $gzip_wals[0]);
> >
> >
> > -           is($gzip_is_valid, 0, "program gzip verified file's integrity");
> >
> >
> >
> > libz and gzip are usually split across different packages, hence there
> >
> > is no guarantee that this command is always available (same comment as
> >
> > for LZ4 from a couple of days ago).
>
> Of course. Though while going for it, I did find in Makefile.global.in:
>
> TAR = @TAR@
>
> XGETTEXT = @XGETTEXT@
>
> GZIP = gzip
>
> BZIP2 = bzip2
>
> DOWNLOAD = wget -O $@ --no-use-server-timestamps
>
> Which is also used by GNUmakefile.in
>
> distcheck: dist
>
> rm -rf $(dummy)
>
> mkdir $(dummy)
>
> $(GZIP) -d -c $(distdir).tar.gz | $(TAR) xf -
>
> install_prefix=`cd $(dummy) && pwd`; \
>
> This to my understanding means that gzip is expected to exist.
>
> If this is correct, then simply checking for the headers should
>
> suffice, since that is the only dependency for the files to be
>
> created.
>
> If this is wrong, then I will add the discovery code as in the
>
> other patch.
>
> > -                   [
> >
> >
> > -                           'pg_receivewal', '-D',     $stream_dir, '--verbose',
> >
> >
> > -                           '--endpos',      $nextlsn, '-Z', '5'
> >
> >
> > -                   ],
> >
> >
> >
> > I would keep the compression level to a minimum here, to limit CPU
> >
> > usage but still compress something faster.
> >
> > -           # Verify compressed file's integrity
> >
> >
> > -           my $gzip_is_valid = system_log('gzip', '--test', $gzip_wals[0]);
> >
> >
> > -           is($gzip_is_valid, 0, "program gzip verified file's integrity");
> >
> >
> >
> > Shouldn't this be coded as a loop going through @gzip_wals?
>
> I would hope that there is only one gz file created. There is a line
>
> further up that tests exactly that.
>
> -   is (scalar(@gzip_wals), 1, "one gzip compressed WAL was created");


Let me amend that. The line should be instead:

      is (scalar(keys @gzip_wals), 1, "one gzip compressed WAL was created");

To properly test that there is one entry.

Let me provide with v2 to fix this.

Cheers,
//Georgios

>
>     Then there should also be a partial gz file which is tested further ahead.
>
>     Cheers,
>
>     //Georgios
>
> > Michael