Обсуждение: change default default_toast_compression to lz4?
How about changing the default of default_toast_compression to lz4? I have seen cases where servers had performance problems and were CPU-bound because of pglz TOAST compression, and changing it to lz4 relieved it significantly. I suspect many users are leaving easy performance improvements on the table by not using this option. The feature was introduced in PostgreSQL 14, so it should be well stabilized now. I suppose one issue is that lz4 support is not compiled-in by default, but in practice most users will have it. The default could be lz4 if lz4 support is built, otherwise pglz. This would be similar to other parameters where the default is the best one depending on the build configuration.
Hi Peter, > How about changing the default of default_toast_compression to lz4? To me it sounds like a great idea. > I have seen cases where servers had performance problems and were > CPU-bound because of pglz TOAST compression, and changing it to lz4 > relieved it significantly. I suspect many users are leaving easy > performance improvements on the table by not using this option. > > The feature was introduced in PostgreSQL 14, so it should be well > stabilized now. > > I suppose one issue is that lz4 support is not compiled-in by default, > but in practice most users will have it. The default could be lz4 if > lz4 support is built, otherwise pglz. This would be similar to other > parameters where the default is the best one depending on the build > configuration. Are there good reasons why we can't simply make lz4 a required dependency? In the worst case we could simply copy its implementation, the license permits. -- Best regards, Aleksander Alekseev
> On 21 Nov 2025, at 12:33, Aleksander Alekseev <aleksander@tigerdata.com> wrote: >> The default could be lz4 if >> lz4 support is built, otherwise pglz. This would be similar to other >> parameters where the default is the best one depending on the build >> configuration. +1 > Are there good reasons why we can't simply make lz4 a required > dependency? In the worst case we could simply copy its implementation, > the license permits. I think we should, as much as we can, avoid vendoring code, especially something like lz4 which can be expected to be available nearly everywhere. -- Daniel Gustafsson
Hi, On 2025-11-21 12:11:38 +0100, Peter Eisentraut wrote: > How about changing the default of default_toast_compression to lz4? > > I have seen cases where servers had performance problems and were CPU-bound > because of pglz TOAST compression, and changing it to lz4 relieved it > significantly. I suspect many users are leaving easy performance > improvements on the table by not using this option. +1 - I also have seen *and* hit this numerous times. IIRC it makes the tests runs a tad bit faster too. > I suppose one issue is that lz4 support is not compiled-in by default, but > in practice most users will have it. The default could be lz4 if lz4 > support is built, otherwise pglz. This would be similar to other parameters > where the default is the best one depending on the build configuration. I think we should mark lz4 as a default-required dependency if we change the default. That way one needs to explicitly opt into a build that won't be compatible with existing data directories created (due to pre-existing lz4 . Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2025-11-21 12:11:38 +0100, Peter Eisentraut wrote:
>> I suppose one issue is that lz4 support is not compiled-in by default, but
>> in practice most users will have it. The default could be lz4 if lz4
>> support is built, otherwise pglz. This would be similar to other parameters
>> where the default is the best one depending on the build configuration.
> I think we should mark lz4 as a default-required dependency if we change the
> default. That way one needs to explicitly opt into a build that won't be
> compatible with existing data directories created (due to pre-existing lz4 .
Agreed --- we should make it act like icu or readline, you have to
opt out. (I'm not voting one way or the other on changing the
default, but if we do we should do it that way.)
regards, tom lane
On 2025-Nov-21, Daniel Gustafsson wrote: > > On 21 Nov 2025, at 12:33, Aleksander Alekseev <aleksander@tigerdata.com> wrote: > > > Are there good reasons why we can't simply make lz4 a required > > dependency? In the worst case we could simply copy its implementation, > > the license permits. > > I think we should, as much as we can, avoid vendoring code, especially > something like lz4 which can be expected to be available nearly everywhere. Yeah. There's the security aspect: if lz4 is found to have a security bug, we would be obliged to issue an advisory and matching release. It's best if the library code is kept separate, so their own security advisory is enough. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
On Fri, Nov 21, 2025 at 12:11:38PM +0100, Peter Eisentraut wrote: > I suppose one issue is that lz4 support is not compiled-in by default, but > in practice most users will have it. The default could be lz4 if lz4 > support is built, otherwise pglz. This would be similar to other parameters > where the default is the best one depending on the build configuration. +1. That makes sense here to flip the default depending on what the code is built with, with lz4 if available, pglz otherwise. -- Michael
Вложения
On Wed, Nov 26, 2025, at 1:35 AM, Michael Paquier wrote: > On Fri, Nov 21, 2025 at 12:11:38PM +0100, Peter Eisentraut wrote: >> I suppose one issue is that lz4 support is not compiled-in by default, but >> in practice most users will have it. The default could be lz4 if lz4 >> support is built, otherwise pglz. This would be similar to other parameters >> where the default is the best one depending on the build configuration. > > +1. That makes sense here to flip the default depending on what the > code is built with, with lz4 if available, pglz otherwise. > Since we have an agreement that $SUBJECT is ok, I wrote a patch for it. It selects the compression method based on USE_LZ4. It also adjusts the postgresql.conf if required. -- Euler Taveira EDB https://www.enterprisedb.com/
Вложения
Hi Euler, > Since we have an agreement that $SUBJECT is ok, I wrote a patch for it. It > selects the compression method based on USE_LZ4. It also adjusts the > postgresql.conf if required. Many thanks for working on this. Unfortunately the patch is incomplete. I think we also agreed that lz4 should be opt-out. -- Best regards, Aleksander Alekseev
On Thu, Nov 27, 2025, at 6:44 AM, Aleksander Alekseev wrote:
> Many thanks for working on this. Unfortunately the patch is
> incomplete. I think we also agreed that lz4 should be opt-out.
>
Aleksander, I missed this point.
Here it is v2. It enforces the lz4 dependency. It works the same as other
dependencies (readline, icu, zlib); error if the dependency could not be found.
Regarding meson, I'm confused. If any of the referred dependencies (icu,
readline, zlib) is not found, there is no hard error. Instead, the feature is
disabled. I searched for a discussion about this decision but couldn't find.
For this patch, I decided to use the same pattern (no error) but I added a
warning message (similar to zlib).
$ meson setup build --prefix=/tmp/pg | grep -i warning
meson.build:1100: WARNING: did not find lz4
meson.build:1675: WARNING: did not find zlib
The other alternative is to always 'enabled' lz4 in meson_options.txt. This
requires you to explicitly enable/disable lz4 if you are using -Dauto_features
option.
Should it report an error (like autoconf) instead of silently disable the
feature that is a requirement? If yes, then we should add error messages for
these 3 dependencies.
$ ./configure --prefix=/tmp/pg
.
.
checking whether to build with ICU support... yes
checking for icu-uc icu-i18n... no
configure: error: Package requirements (icu-uc icu-i18n) were not met:
Package 'icu-uc', required by 'virtual:world', not found
Package 'icu-i18n', required by 'virtual:world', not found
Consider adjusting the PKG_CONFIG_PATH environment variable if you
installed software in a non-standard prefix.
Alternatively, you may set the environment variables ICU_CFLAGS
and ICU_LIBS to avoid the need to call pkg-config.
See the pkg-config man page for more details.
$ git clean -dfxq
$ meson setup build --prefix=/tmp/pg
.
.
Run-time dependency icu-uc found: NO (tried pkgconfig and cmake)
Run-time dependency icu found: NO (tried cmake)
.
.
Run-time dependency readline found: NO (tried pkgconfig and cmake)
Library readline found: NO
.
.
External libraries
bonjour : NO
bsd_auth : NO
docs : NO
docs_pdf : NO
gss : NO
icu : NO
ldap : NO
libcurl : NO
libnuma : NO
liburing : NO
libxml : NO
libxslt : NO
llvm : NO
lz4 : NO
nls : YES
openssl : NO
pam : NO
plperl : NO 5.40.1
plpython : NO
pltcl : NO
readline : NO
selinux : NO
systemd : NO
uuid : NO
zlib : NO
zstd : NO
User defined options
prefix : /tmp/pg
Found ninja-1.12.1 at /usr/bin/ninja
--
Euler Taveira
EDB https://www.enterprisedb.com/
Вложения
On Thu, Dec 04, 2025 at 05:11:15PM -0300, Euler Taveira wrote: > Here it is v2. It enforces the lz4 dependency. It works the same as other > dependencies (readline, icu, zlib); error if the dependency could not be found. > Regarding meson, I'm confused. If any of the referred dependencies (icu, > readline, zlib) is not found, there is no hard error. Instead, the feature is > disabled. I searched for a discussion about this decision but couldn't find. > For this patch, I decided to use the same pattern (no error) but I added a > warning message (similar to zlib). Another thing to be careful of is that this would immediately break the CI task CompilerWarnings for mingw: [02:03:19.626] checking whether to build with LZ4 support... yes [02:03:19.626] checking for liblz4... no [02:03:19.694] configure: error: Package requirements (liblz4) were not met: So this had better be adjusted in one go, in the shape of a tweak in mingw_cross_warning_script with the addition of a --without-lz4, same way as for ICU. > Should it report an error (like autoconf) instead of silently disable the > feature that is a requirement? If yes, then we should add error messages for > these 3 dependencies. Hmm. It seems to me that we should just set not_found_dep if lz4 cannot be found, leaving meson_options.txt as it is currently, no? One could always force meson's hand with -Dlz4=disabled. As a whole, I'd value more consistency with how icu is handled if we want to force LZ4 across the board in the backend. The case with zlib is different in the backend: we only use it on a wanted-basis for compression specifications in server-side compressed base backups, so it seems to me that there's a case for being a tad more aggressive with LZ4 as it relates to TOAST compression, forcing an error rather than ignoring it silently if it cannot be found. -- Michael
Вложения
On Thu, Dec 4, 2025, at 11:23 PM, Michael Paquier wrote: > > So this had better be adjusted in one go, in the shape of a tweak in > mingw_cross_warning_script with the addition of a --without-lz4, same > way as for ICU. > Done. > Hmm. It seems to me that we should just set not_found_dep if lz4 > cannot be found, leaving meson_options.txt as it is currently, no? > Done. Changes from last version: - adjust CI - set not_found_dep instead of warning message - remove quotes from default_toast_compression value It is aligned with ICU and readline behavior. -- Euler Taveira EDB https://www.enterprisedb.com/
Вложения
On Thu, Feb 19, 2026 at 01:12:09PM -0300, Euler Taveira wrote:
> Changes from last version:
>
> - adjust CI
> - set not_found_dep instead of warning message
> - remove quotes from default_toast_compression value
>
> It is aligned with ICU and readline behavior.
Works for me, and I would like to apply this one.
diff --git a/meson.build b/meson.build
index 055e96315d0..13ef9c18477 100644
--- a/meson.build
+++ b/meson.build
@@ -1133,6 +1133,8 @@ if not lz4opt.disabled()
if lz4.found()
cdata.set('USE_LZ4', 1)
cdata.set('HAVE_LIBLZ4', 1)
+ else
+ lz4 = not_found_dep
endif
Hmm. Isn't this bit something that we should actually backpatch? It
feels wrong to not enforce lz4 to not_found_dep in this meson path.
So this looks unrelated to the switch of the default value.
--
Michael
Вложения
On 19.02.26 17:12, Euler Taveira wrote: > On Thu, Dec 4, 2025, at 11:23 PM, Michael Paquier wrote: >> >> So this had better be adjusted in one go, in the shape of a tweak in >> mingw_cross_warning_script with the addition of a --without-lz4, same >> way as for ICU. >> > Done. > >> Hmm. It seems to me that we should just set not_found_dep if lz4 >> cannot be found, leaving meson_options.txt as it is currently, no? >> > Done. > > Changes from last version: > > - adjust CI > - set not_found_dep instead of warning message > - remove quotes from default_toast_compression value > > It is aligned with ICU and readline behavior. Your patch is adding documentation for the configure option --without-lz4, but you are leaving the documentation for --with-lz4 in place. You should delete the latter.
On 24.02.26 09:01, Michael Paquier wrote:
> diff --git a/meson.build b/meson.build
> index 055e96315d0..13ef9c18477 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1133,6 +1133,8 @@ if not lz4opt.disabled()
> if lz4.found()
> cdata.set('USE_LZ4', 1)
> cdata.set('HAVE_LIBLZ4', 1)
> + else
> + lz4 = not_found_dep
> endif
>
> Hmm. Isn't this bit something that we should actually backpatch? It
> feels wrong to not enforce lz4 to not_found_dep in this meson path.
> So this looks unrelated to the switch of the default value.
I don't think this change is correct, or at least it's not required, and
you won't find it in the handling of other dependencies.
The point of assigning not_found_dep is to make lz4.found() return
false, but in that particular code location lz4.found() is already false.
On Tue, Feb 24, 2026 at 10:52:39PM +0100, Peter Eisentraut wrote: > Your patch is adding documentation for the configure option --without-lz4, > but you are leaving the documentation for --with-lz4 in place. You should > delete the latter. Ah, right. This is what we have done for ICU in fcb21b3acdcb. While --with-lz4 is still allowed for backward-compatibility, we show only --without-icu in the output of configure --help. Reflecting the documentation to show up the same contents as --help makes sense. Could you update the patch, Euler? This includes the blip in meson.build regarding the addition of not_found_dep not really required. -- Michael
Вложения
On Fri, Feb 27, 2026 at 10:10:20AM +0900, Michael Paquier wrote: > Ah, right. This is what we have done for ICU in fcb21b3acdcb. While > --with-lz4 is still allowed for backward-compatibility, we show only > --without-icu in the output of configure --help. Reflecting the > documentation to show up the same contents as --help makes sense. It would have been a waste to not see this change happening, so I have looked a bit more in depth at it. You are right that we should remove the reference to --with-lz4, but it seems to me that this should apply to all the references of the switch we have in the docs. The change in meson.build was not necessary. Another thing not yet included was the mandatory autoreconf. With all that done, applied. Now, let's see how much this breaks the buildfarm.. Because things will break. I'll send a poke to the buildfarm mailing list. -- Michael
Вложения
> On Mar 4, 2026, at 12:09, Michael Paquier <michael@paquier.xyz> wrote: > > On Fri, Feb 27, 2026 at 10:10:20AM +0900, Michael Paquier wrote: >> Ah, right. This is what we have done for ICU in fcb21b3acdcb. While >> --with-lz4 is still allowed for backward-compatibility, we show only >> --without-icu in the output of configure --help. Reflecting the >> documentation to show up the same contents as --help makes sense. > > It would have been a waste to not see this change happening, so I have > looked a bit more in depth at it. You are right that we should remove > the reference to --with-lz4, but it seems to me that this should apply > to all the references of the switch we have in the docs. The change > in meson.build was not necessary. Another thing not yet included was > the mandatory autoreconf. With all that done, applied. > > Now, let's see how much this breaks the buildfarm.. Because things > will break. I'll send a poke to the buildfarm mailing list. > -- > Michael Hi Michael, As you have pushed this patch, could you please take a look at [1] that fixes an edge-case bug of in astreamer for the lz4decompressor. [1] https://www.postgresql.org/message-id/0594CC79-1544-45DD-8AA4-26270DE777A7%40gmail.com Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
Michael Paquier <michael@paquier.xyz> writes:
> Now, let's see how much this abreaks the buildfarm.. Because things
> will break. I'll send a poke to the buildfarm mailing list.
I don't think this idea is really fully baked yet. What happens
if I build with liblz4, and run for a while with that so that
there are lz4-compressed TOAST values, and then try to update
to a build without LZ4?
I think it might be a good idea to add a field to pg_control
showing whether the cluster has ever been run with lz4 toast
compression, so that a non-lz4-enabled build could refuse
to start rather than encountering unexpected errors later.
regards, tom lane
On Wed, 4 Mar 2026 at 06:05, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Michael Paquier <michael@paquier.xyz> writes: > > Now, let's see how much this abreaks the buildfarm.. Because things > > will break. I'll send a poke to the buildfarm mailing list. > > I don't think this idea is really fully baked yet. What happens > if I build with liblz4, and run for a while with that so that > there are lz4-compressed TOAST values, and then try to update > to a build without LZ4? That doesn't seem like a new problem to me. That's already possible since PG14. To me it actually seems more likely to happen, because we default to allowing builds without lz4 (which we stopped doing on master). The fact that we haven't had (many) people complain indicates to me that it isn't a big problem in practice.