Horton
Menu

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Set minimum required version of zstd as 1.4.0.

  2. Add support for zstd with compression of full-page writes in WAL

  1. head fails to build on SLES 12

    Devrim Gündüz <devrim@gunduz.org> — 2022-03-31T14:23:22Z

    Hi,

Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
4.8-8. Build log is attached.

Please let me know if you want me to provide more info.

Thanks!

Regards,
-- 
Devrim Gündüz
Open Source Solution Architect, PostgreSQL guy
Twitter: @DevrimGunduz , @DevrimGunduzTR

  2. Re: head fails to build on SLES 12

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-31T14:26:31Z

    Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:
> Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
> 4.8-8. Build log is attached.

Hmm, what version of libzstd is present?

			regards, tom lane

  3. Re: head fails to build on SLES 12

    Devrim Gündüz <devrim@gunduz.org> — 2022-03-31T14:38:39Z

    Hi,

On Thu, 2022-03-31 at 10:26 -0400, Tom Lane wrote:
> Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:
> > Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
> > 4.8-8. Build log is attached.
> 
> Hmm, what version of libzstd is present?

1.3.3

Regards,
-- 
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

  4. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Justin Pryzby <pryzby@telsasoft.com> — 2022-03-31T15:37:40Z

    On Thu, Mar 31, 2022 at 03:38:39PM +0100, Devrim Gündüz wrote:
> On Thu, 2022-03-31 at 10:26 -0400, Tom Lane wrote:
> > Devrim =?ISO-8859-1?Q?G=FCnd=FCz?= <devrim@gunduz.org> writes:
> > > Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
> > > 4.8-8. Build log is attached.
> > 
> > Hmm, what version of libzstd is present?
> 
> 1.3.3

That's due to commit e9537321a74a2b062c8f7a452314b4570913f780.

Possible responses look like:
 - Use 0 which also means "default" (need to verify that works across versions);
 - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
 - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
   v15 in any case, since we're using a few different APIs (at least
   ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).

I dug up this history:

commit b2632bcf6cf7b9b96e0ac99beea079df4d1eaec5
Merge: 170f948e 869e2718
Author: Yann Collet <Cyan4973@users.noreply.github.com>
Date:   Tue Jun 12 12:09:01 2018 -0700

    Merge pull request #1174 from duc0/document_default_level
    
    Expose ZSTD_CLEVEL_DEFAULT and update documentation

commit e34c000e44444b9f8bd62e5af0a355ee186eb21f
Author: Duc Ngo <duc@fb.com>
Date:   Fri Jun 8 11:29:51 2018 -0700

    Expose ZSTD_CLEVEL_DEFAULT and update documentation

commit 6d4fef36de21908e333b2a1fde8ded0a7f086ae1
Author: Yann Collet <cyan@fb.com>
Date:   Wed May 17 18:36:15 2017 -0700

    Added ZSTD_compress_generic()

    Used in fileio.c (zstd cli).
    Need to set macro ZSTD_NEWAPI to trigger it.

commit 236d94fa9a4ff8723922971274a119c6084d5dbc
Author: Yann Collet <yann.collet.73@gmail.com>
Date:   Wed May 18 12:06:33 2016 +0200

    reverted default compression level to 1

  5. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-31T15:44:40Z

    Justin Pryzby <pryzby@telsasoft.com> writes:
> Possible responses look like:
>  - Use 0 which also means "default" (need to verify that works across versions);
>  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
>  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
>    v15 in any case, since we're using a few different APIs (at least
>    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).

In view of 51c0d186d ("Allow parallel zstd compression"), I agree
that some clarity about the minimum supported version of zstd
seems essential.  I don't want to be dealing with threading bugs
in ancient zstd versions.  However, why do you suggest 1.3.7 in
particular?

			regards, tom lane

  6. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Robert Haas <robertmhaas@gmail.com> — 2022-03-31T16:26:46Z

    On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > Possible responses look like:
> >  - Use 0 which also means "default" (need to verify that works across versions);
> >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> >    v15 in any case, since we're using a few different APIs (at least
> >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
>
> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> that some clarity about the minimum supported version of zstd
> seems essential.  I don't want to be dealing with threading bugs
> in ancient zstd versions.  However, why do you suggest 1.3.7 in
> particular?

One thing to note is that apparently threading wasn't enabled in the
default build before 1.5.0, which was released in May 2021, but it did
exist as an option in the code for some period of time prior to that.
I don't know how long exactly. I don't want to jump to the conclusion
that other people's old versions are full of bugs, but if that should
happen to be true here, there's some chance that PostgreSQL users
won't be exposed to them just because threading wasn't enabled by
default until quite recently.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  7. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Justin Pryzby <pryzby@telsasoft.com> — 2022-03-31T16:34:41Z

    On Thu, Mar 31, 2022 at 12:26:46PM -0400, Robert Haas wrote:
> On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Justin Pryzby <pryzby@telsasoft.com> writes:
> > > Possible responses look like:
> > >  - Use 0 which also means "default" (need to verify that works across versions);
> > >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> > >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> > >    v15 in any case, since we're using a few different APIs (at least
> > >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
> >
> > In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> > that some clarity about the minimum supported version of zstd
> > seems essential.  I don't want to be dealing with threading bugs
> > in ancient zstd versions.  However, why do you suggest 1.3.7 in
> > particular?
> 
> One thing to note is that apparently threading wasn't enabled in the
> default build before 1.5.0, which was released in May 2021, but it did
> exist as an option in the code for some period of time prior to that.
> I don't know how long exactly. I don't want to jump to the conclusion
> that other people's old versions are full of bugs, but if that should
> happen to be true here, there's some chance that PostgreSQL users
> won't be exposed to them just because threading wasn't enabled by
> default until quite recently.

Right.  Importantly, it's a run-time failure condition if threading wasn't
enabled at compile time.  Postgres should still compile --with-zstd even if it
wasn't, and pg_basebackup should work, except if workers is specified (or maybe
if workers>0, but it's possible that allowing workers=0 wasn't true before some
version).  I'll write more later.

-- 
Justin

  8. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-31T16:37:18Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Mar 31, 2022 at 11:44 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
>> that some clarity about the minimum supported version of zstd
>> seems essential.  I don't want to be dealing with threading bugs
>> in ancient zstd versions.  However, why do you suggest 1.3.7 in
>> particular?

> One thing to note is that apparently threading wasn't enabled in the
> default build before 1.5.0, which was released in May 2021, but it did
> exist as an option in the code for some period of time prior to that.
> I don't know how long exactly. I don't want to jump to the conclusion
> that other people's old versions are full of bugs, but if that should
> happen to be true here, there's some chance that PostgreSQL users
> won't be exposed to them just because threading wasn't enabled by
> default until quite recently.

Hm.  After rereading 51c0d186d I see that we're not asking for
parallel compression unless the user tells us to, so I guess
our fallback answer for any complaints in that area can be
"if it hurts, don't do it".  Still, I like the idea of having
a well-defined minimum zstd version that we consider supported.
The evident fact that their APIs are still changing (or at
least have done so within the memory of LTS platforms) makes
that fairly pressing.  Question is what to set the minimum to.

			regards, tom lane

  9. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Robert Haas <robertmhaas@gmail.com> — 2022-03-31T16:55:18Z

    On Thu, Mar 31, 2022 at 12:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hm.  After rereading 51c0d186d I see that we're not asking for
> parallel compression unless the user tells us to, so I guess
> our fallback answer for any complaints in that area can be
> "if it hurts, don't do it".

Right. We can also tell people that if they are running buggy versions
of libzstd or liblz4 or libz, they should upgrade to non-buggy
versions. Our ability to paper over bugs in compression libraries is
going to be extremely limited.

> Still, I like the idea of having
> a well-defined minimum zstd version that we consider supported.
> The evident fact that their APIs are still changing (or at
> least have done so within the memory of LTS platforms) makes
> that fairly pressing.  Question is what to set the minimum to.

I think we should aim, if we can, to be compatible with libzstd
versions that are still being shipped with still-supported releases of
mainstream Linux distributions. If that turns out to be too hard, we
can be less ambitious.

On the particular question of ZSTD_CLEVEL_DEFAULT, it does not seem
likely that the library would have only recently exposed a symbol that
is required for correct use of the API, so I bet there's a relatively
simple way to avoid needing that altogether (perhaps by writing "0"
instead).

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  10. Re: head fails to build on SLES 12

    Andrew Dunstan <andrew@dunslane.net> — 2022-03-31T19:58:34Z

    On 3/31/22 10:23, Devrim Gündüz wrote:
> Hi,
>
> Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
> 4.8-8. Build log is attached.
>
> Please let me know if you want me to provide more info.
>


AFAICT we don't have any buildfarm animals currently reporting on SLES
12 (and the one we did have was on s390x, if that matters).

So if this is something that needs support we should address that. After
all, that's exactly what the buildfarm was created for.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  11. Re: head fails to build on SLES 12

    Larry Rosenman <ler@lerctr.org> — 2022-03-31T20:03:50Z

    On 03/31/2022 2:58 pm, Andrew Dunstan wrote:
> On 3/31/22 10:23, Devrim Gündüz wrote:
>> Hi,
>> 
>> Latest snapshot tarball fails to build on SLES 12.5, which uses GCC
>> 4.8-8. Build log is attached.
>> 
>> Please let me know if you want me to provide more info.
>> 
> 
> 
> AFAICT we don't have any buildfarm animals currently reporting on SLES
> 12 (and the one we did have was on s390x, if that matters).
> 
> So if this is something that needs support we should address that. 
> After
> all, that's exactly what the buildfarm was created for.
> 
> 
> cheers
> 
> 
> andrew
> 
> --
> Andrew Dunstan
> EDB: https://www.enterprisedb.com

I can spin up a VM on SLES 12 assuming someone can point me to the right 
place to get
an ISO, etc, and I don't need a payfor license.

-- 
Larry Rosenman                     http://www.lerctr.org/~ler
Phone: +1 214-642-9640                 E-Mail: ler@lerctr.org
US Mail: 5708 Sabbia Dr, Round Rock, TX 78665-2106

  12. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Justin Pryzby <pryzby@telsasoft.com> — 2022-03-31T22:42:30Z

    On Thu, Mar 31, 2022 at 11:44:40AM -0400, Tom Lane wrote:
> Justin Pryzby <pryzby@telsasoft.com> writes:
> > Possible responses look like:
> >  - Use 0 which also means "default" (need to verify that works across versions);
> >  - Or #ifndef ZSTD_CLEVEL_DEFAULT #define ZSTD_CLEVEL_DEFAULT 3;
> >  - Add a test for a minimum zstd version v1.3.7.  This may be a good idea for
> >    v15 in any case, since we're using a few different APIs (at least
> >    ZSTD_compress and ZSTD_compressStream2 and execve(zstd)).
> 
> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
> that some clarity about the minimum supported version of zstd
> seems essential.  I don't want to be dealing with threading bugs
> in ancient zstd versions.  However, why do you suggest 1.3.7 in
> particular?

That's where I found that ZSTD_CLEVEL_DEFAULT was added, in their git.

I've just installed a .deb for 1.3.8, and discovered that the APIs used by
basebackup were considered experimental/nonpublic/static-lib-only until 1.4.0
https://github.com/facebook/zstd/releases/tag/v1.4.0
ZSTD_CCtx_setParameter ZSTD_c_compressionLevel ZSTD_c_nbWorkers ZSTD_CCtx_reset ZSTD_reset_session_only ZSTD_compressStream2 ZSTD_e_continue ZSTD_e_end

FYI: it looks like parallel, thread workers were also a nonpublic,
"experimental" API until v1.3.7.  Actually, ZSTD_p_nbThreads was renamed to
ZSTD_p_nbWorkers and then (in 1.3.8) renamed to ZSTD_c_nbWorkers.

Versions less than 1.3.8 would have required compile-time conditionals for both
ZSTD_CLEVEL_DEFAULT and ZSTD_p_nbThreads/ZSTD_p_nbWorkers/ZSTD_c_nbWorkers (but
that is moot).

Negative compression levels were added in 1.3.4 (but I think the range of their
levels was originally -1..-7 and now expanded).  And long-distance matching
added in 1.3.2.

cirrus has installed:
linux (debian bullseye) 1.4.8
macos has 1.5.0
freebsd has 1.5.0

debian buster (oldstable) has v1.3.8, which is too old.
ubuntu focal LTS has 1.4.4 (bionic LTS has 1.3.3 which seems too old)
rhel7 has 1.5.2 in EPEL;

SLES 12.5 has zstd 1.3.3, so it won't be supported.  But postgres should fail
gracefully during ./configure rather than during build.

Note that check-world fails if wal_compression is enabled due to two
outstanding issues, so it's not currently possible to set that in CI or
buildfarm...
https://www.postgresql.org/message-id/c86ce84f-dd38-9951-102f-13a931210f52%40dunslane.net

  13. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-03-31T23:38:29Z

    Justin Pryzby <pryzby@telsasoft.com> writes:
> On Thu, Mar 31, 2022 at 11:44:40AM -0400, Tom Lane wrote:
>> In view of 51c0d186d ("Allow parallel zstd compression"), I agree
>> that some clarity about the minimum supported version of zstd
>> seems essential.  I don't want to be dealing with threading bugs
>> in ancient zstd versions.  However, why do you suggest 1.3.7 in
>> particular?

> That's where I found that ZSTD_CLEVEL_DEFAULT was added, in their git.

> I've just installed a .deb for 1.3.8, and discovered that the APIs used by
> basebackup were considered experimental/nonpublic/static-lib-only until 1.4.0

Indeed.  I tried building against 1.3.6 (mainly because it was laying
around) and the error reported by Devrim is just the tip of the iceberg.
With "make -k", I see unknown-symbol failures on

ZSTD_CCtx_setParameter
ZSTD_c_compressionLevel
ZSTD_c_nbWorkers
ZSTD_CCtx_reset
ZSTD_reset_session_only
ZSTD_compressStream2
ZSTD_e_continue
ZSTD_e_end

I wonder whether Robert's ambition to be compatible with old versions
extends that far.

			regards, tom lane

  14. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Robert Haas <robertmhaas@gmail.com> — 2022-04-01T00:39:28Z

    On Thu, Mar 31, 2022 at 7:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Indeed.  I tried building against 1.3.6 (mainly because it was laying
> around) and the error reported by Devrim is just the tip of the iceberg.
> With "make -k", I see unknown-symbol failures on
>
> ZSTD_CCtx_setParameter
> ZSTD_c_compressionLevel
> ZSTD_c_nbWorkers
> ZSTD_CCtx_reset
> ZSTD_reset_session_only
> ZSTD_compressStream2
> ZSTD_e_continue
> ZSTD_e_end
>
> I wonder whether Robert's ambition to be compatible with old versions
> extends that far.

It definitely doesn't, and the fact that there's that much difference
in the APIs between 2018 and the present frankly makes my heart sink.

It looks like this stuff may be what libzstd calls the "advanced API"
which was considered unstable until 1.4.0 according to
https://github.com/facebook/zstd/releases -- so maybe we ought to
declare anything pre-1.4.0. to be unsupported. I really hope they're
serious about keeping the advanced API stable, though. I'm excited
about the potential of using libzstd, but I don't want to have to keep
adjusting our code to work with new library versions.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

  15. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-01T01:10:00Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Mar 31, 2022 at 7:38 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Indeed.  I tried building against 1.3.6 (mainly because it was laying
>> around) and the error reported by Devrim is just the tip of the iceberg.
>> ...
>> I wonder whether Robert's ambition to be compatible with old versions
>> extends that far.

> It definitely doesn't, and the fact that there's that much difference
> in the APIs between 2018 and the present frankly makes my heart sink.

AFAICS it's just additions; this stuff is not evidence that they
broke anything.

> It looks like this stuff may be what libzstd calls the "advanced API"
> which was considered unstable until 1.4.0 according to
> https://github.com/facebook/zstd/releases -- so maybe we ought to
> declare anything pre-1.4.0. to be unsupported.

That seems like the appropriate answer to me.  I verified that we
build cleanly and pass check-world against 1.4.0, and later I'm
going to set up BF member longfin to use that.  So that will give
us an anchor that we support zstd that far back.  Had we written
this code earlier, maybe we'd have confined ourselves to 1.3.x
features ... but we didn't, and I don't see much value in doing
so now.

In short, I think we should push Justin's version-check patch,
and also fix the SGML docs to say that we require zstd >= 1.4.0.

			regards, tom lane

  16. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Michael Paquier <michael@paquier.xyz> — 2022-04-01T04:35:47Z

    On Thu, Mar 31, 2022 at 09:10:00PM -0400, Tom Lane wrote:
> In short, I think we should push Justin's version-check patch,
> and also fix the SGML docs to say that we require zstd >= 1.4.0.

1.4.0 was released in April 2019, just 3 years ago.  It does not sound
that bad to me to make this version number a requirement for 15~ if
one wants to use zstd.
--
Michael

  17. Re: head fails to build on SLES 12 (wal_compression=zstd)

    Tom Lane <tgl@sss.pgh.pa.us> — 2022-04-01T15:06:53Z

    I wrote:
> That seems like the appropriate answer to me.  I verified that we
> build cleanly and pass check-world against 1.4.0, and later I'm
> going to set up BF member longfin to use that.

Done ...

> In short, I think we should push Justin's version-check patch,
> and also fix the SGML docs to say that we require zstd >= 1.4.0.

... and done.

			regards, tom lane