Re: pg_basebackup: add test about zstd compress option - Mailing list pgsql-hackers

From Ian Lawrence Barwick
Subject Re: pg_basebackup: add test about zstd compress option
Date
Msg-id CAB8KJ=jZnJ9EStv0YSYuGDUZy=EfQ7tQ7M5OfW+j4sndGvb9CQ@mail.gmail.com
Whole thread Raw
In response to Re: pg_basebackup: add test about zstd compress option  (Dong Wook Lee <sh95119@gmail.com>)
Responses Re: pg_basebackup: add test about zstd compress option  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Hi

I was looking at the commitfest entry for this patch [1] as it's been dormant
for quite a while, with the intent of returning it with feedback.

[1] https://commitfest.postgresql.org/40/3835/

2022年8月25日(木) 16:52 Dong Wook Lee <sh95119@gmail.com>:
>
> On Tue, Aug 23, 2022 at 11:37 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> > It seems to me that checking that the contents generated are valid is
> > equally necessary.  We do that with zlib with gzip --test, and you
> > could use ${ZSTD} in the context of this test.
>
> Thank you for the good points.
> I supplemented the test according to your suggestion.
> However, there was a problem.
> Even though I did export ZSTD on the Makefile , the test runner can't
> find ZSTD when it actually tests.
> ```
> my $zstd = $ENV{ZSTD};
> skip "program zstd is not found in your system", 1
>   if (!defined $zstd
>     || $zstd eq '');
> ```
> log: regress_log_010_pg_basebackup
> ```
> ok 183 # skip program zstd is not found in your system.
> ```
> Could you check if I missed anything?

Taking a quick look at the patch itself, as-is it does actually work; maybe
 the zstd binary itself was missing or not in the normal system path?
 It might not have been installed even if the devel library was (IIRC
 this was the case on Rocky Linux).

However the code largely duplicates the preceding gzip test,
 and as Michael mentioned there's still lz4 without coverage.
Attached patch refactors this part of the test so it can be used
for multiple compression methods, similar to the test in
src/bin/pg_verifybackup/t/009_extract.pl mentioned by Robert.
The difference to that test is that we can exercise all the
command line options and directly check the generated files with
the respective binary.

Though on reflection maybe it's overkill and the existing tests
suffice. Anyway leaving the patch here in the interests of pushing
this forward in some direction.

Regards

Ian Barwick

Attachment

pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures
Next
From: Greg Stark
Date:
Subject: Re: Temporary tables versus wraparound... again