Re: Introduce pg_receivewal gzip compression tests

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

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

> ‐‐‐‐‐‐‐ 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.


Please find v2 attached with the above.

Cheers,
//Georgios

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