Обсуждение: 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