Re: pg_basebackup: add test about zstd compress option

From: Ian Lawrence Barwick <barwick(at)gmail(dot)com>
To: Dong Wook Lee <sh95119(at)gmail(dot)com>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pg_basebackup: add test about zstd compress option
Date: 2022-12-03 04:29:27
Message-ID: CAB8KJ=jZnJ9EStv0YSYuGDUZy=EfQ7tQ7M5OfW+j4sndGvb9CQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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(at)gmail(dot)com>:
>
> On Tue, Aug 23, 2022 at 11:37 AM Michael Paquier <michael(at)paquier(dot)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 Content-Type Size
v3-0001-pg_basebackup-refactor-compression-tests.patch text/x-patch 8.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Stark 2022-12-03 04:33:29 Re: Temporary tables versus wraparound... again
Previous Message Bharath Rupireddy 2022-12-03 03:37:38 Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures