Update maintenance_work_mem/autovacuum_work_mem to reflect the 1GB limitation with VACUUM
Hi,
There's a well known limitation in the amount of memory that any
VACUUM process can use, capped at 1GB now. This is not reflected in
the documentation
https://www.postgresql.org/docs/current/runtime-config-resource.html
We should add a line that indicates that there is a limitation (that
should be IMO, backported to documentation of earlier versions as it
affects all supported versions), at least until such limitation is
lifted.
Kind regards, Martín
--
Martín Marqués
It’s not that I have something to hide,
it’s that I have nothing I want you to see
On Mon, 2021-05-03 at 13:48 -0300, Martín Marqués wrote:
We should add a line that indicates that there is a limitation (that
should be IMO, backported to documentation of earlier versions as it
affects all supported versions), at least until such limitation is
lifted.
Here is a patch for that,
Yours,
Laurenz Albe
Attachments:
0001-Document-the-1GB-memory-limit-for-VACUUM.patchtext/x-patch; charset=UTF-8; name=0001-Document-the-1GB-memory-limit-for-VACUUM.patchDownload
From 69ba1f6bef0209fa1093c39ba2e731641d7f5188 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Wed, 5 May 2021 12:02:11 +0200
Subject: [PATCH] Document the 1GB memory limit for VACUUM
---
doc/src/sgml/config.sgml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3a062a145c..9219b5c90a 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1901,6 +1901,11 @@ include_dir 'conf.d'
be used instead. The setting has no effect on the behavior of
<command>VACUUM</command> when run in other contexts.
</para>
+ <para>
+ Node that <command>VACUUM</command> has a hard-coded limit of 1GB
+ for the amount of memory used, so setting
+ <varname>autovacuum_work_mem</varname> higher than that has no effect.
+ </para>
</listitem>
</varlistentry>
--
2.26.3
On Wed, 2021-05-05 at 12:03 +0200, Laurenz Albe wrote:
On Mon, 2021-05-03 at 13:48 -0300, Martín Marqués wrote:
We should add a line that indicates that there is a limitation (that
should be IMO, backported to documentation of earlier versions as it
affects all supported versions), at least until such limitation is
lifted.Here is a patch for that
Just sending a reply to -hackers so I can add it to the commitfest.
Yours,
Laurenz Albe
s/Node/Note/
Other than that, +1 to the patch and +1 to backpatching.
The new status of this patch is: Waiting on Author
On Wed, May 5, 2021 at 7:03 PM Laurenz Albe <laurenz.albe@cybertec.at> wrote:
On Mon, 2021-05-03 at 13:48 -0300, Martín Marqués wrote:
We should add a line that indicates that there is a limitation (that
should be IMO, backported to documentation of earlier versions as it
affects all supported versions), at least until such limitation is
lifted.Here is a patch for that,
The patch adds the description in the autovacuum_work_mem section.
Isn't it better to add it in mantenance_work section or VACUUM command
section since this limitation is not only for autovacuum?
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
On Wed, 2021-06-02 at 18:16 +0900, Masahiko Sawada wrote:
We should add a line that indicates that there is a limitation (that
should be IMO, backported to documentation of earlier versions as it
affects all supported versions), at least until such limitation is
lifted.Here is a patch for that,
The patch adds the description in the autovacuum_work_mem section.
Isn't it better to add it in mantenance_work section or VACUUM command
section since this limitation is not only for autovacuum?
You are right; theoretically, the correct place to document that
would be the VACUUM documentation. But I guess that most people who
are curious about VACUUM's memory usage will read the documentation for
"maintenance_work_mem" or "autovacuum_work_mem".
I have fixed a typo and added a similar paragraph to "maintenance_work_mem".
Yours,
Laurenz Albe
Attachments:
0001-Document-the-1GB-memory-limit-for-VACUUM.v2.patchtext/x-patch; charset=UTF-8; name=0001-Document-the-1GB-memory-limit-for-VACUUM.v2.patchDownload
From a94811e45df96846cb0531615e9d773f69d2cc2d Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.albe@cybertec.at>
Date: Fri, 4 Jun 2021 14:30:08 +0200
Subject: [PATCH] Document the 1GB memory limit for VACUUM
---
doc/src/sgml/config.sgml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 7e32b0686c..0976494506 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1882,6 +1882,12 @@ include_dir 'conf.d'
too high. It may be useful to control for this by separately
setting <xref linkend="guc-autovacuum-work-mem"/>.
</para>
+ <para>
+ Also, note that <command>VACUUM</command> has a hard-coded limit of 1GB
+ for the amount of memory used, so setting
+ <varname>maintenance_work_mem</varname> higher than that has no effect
+ as far as <command>VACUUM</command> is concerned.
+ </para>
</listitem>
</varlistentry>
@@ -1901,6 +1907,11 @@ include_dir 'conf.d'
be used instead. The setting has no effect on the behavior of
<command>VACUUM</command> when run in other contexts.
</para>
+ <para>
+ Note that <command>VACUUM</command> has a hard-coded limit of 1GB
+ for the amount of memory used, so setting
+ <varname>autovacuum_work_mem</varname> higher than that has no effect.
+ </para>
</listitem>
</varlistentry>
--
2.26.3
Should we say "currently has"?
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Latest patch looks fine to me, to be clear.
The new status of this patch is: Ready for Committer
On Fri, 21 May 2021 at 03:52, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
Just sending a reply to -hackers so I can add it to the commitfest.
I had a look at the patch in [1]/messages/by-id/514fe5ce4714b7b33cb0a611f0c7b72df413bef5.camel@cybertec.at and I find it a bit weird that we'd
write the following about autovacuum_work_mem in our docs:
+ <para>
+ Note that <command>VACUUM</command> has a hard-coded limit of 1GB
+ for the amount of memory used, so setting
+ <varname>autovacuum_work_mem</varname> higher than that has no effect.
+ </para>Since that setting is *only* used for auto vacuum, why don't we just
limit the range of the GUC to 1GB?
Of course, it wouldn't be wise to backpatch the reduced limit of
autovacuum_work_mem as it might upset people who have higher values in
their postgresql.conf when their database fails to restart after an
upgrade. I think what might be best is just to reduce the limit in
master and apply the doc patch for just maintenance_work_mem in all
supported versions. We could just ignore doing anything with
autovacuum_work_mem in the back branches and put it down to a
historical mistake that can't easily be fixed now.
I've attached what I came up with.
What do you think?
[1]: /messages/by-id/514fe5ce4714b7b33cb0a611f0c7b72df413bef5.camel@cybertec.at
Attachments:
limit_vacuum_memory_to_1gb.patchapplication/octet-stream; name=limit_vacuum_memory_to_1gb.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6098f6b020..a7281c8e73 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1893,6 +1893,11 @@ include_dir 'conf.d'
too high. It may be useful to control for this by separately
setting <xref linkend="guc-autovacuum-work-mem"/>.
</para>
+ <para>
+ Additionally, <command>VACUUM</command> is only able to utilize up to
+ a maximum of 1GB of memory, so <varname>maintenance_work_mem</varname>
+ values higher than this have no effect on <command>VACUUM</command>.
+ </para>
</listitem>
</varlistentry>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 480e8cd199..bb76baa72d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3340,7 +3340,8 @@ static struct config_int ConfigureNamesInt[] =
GUC_UNIT_KB
},
&autovacuum_work_mem,
- -1, -1, MAX_KILOBYTES,
+ /* see compute_max_dead_tuples if you need to change the max value */
+ -1, -1, 1024 * 1024,
check_autovacuum_work_mem, NULL, NULL
},
On Fri, 2021-07-02 at 23:31 +1200, David Rowley wrote:
I had a look at the patch in [1] and I find it a bit weird that we'd
write the following about autovacuum_work_mem in our docs:+ <para> + Note that <command>VACUUM</command> has a hard-coded limit of 1GB + for the amount of memory used, so setting + <varname>autovacuum_work_mem</varname> higher than that has no effect. + </para>Since that setting is *only* used for auto vacuum, why don't we just
limit the range of the GUC to 1GB?Of course, it wouldn't be wise to backpatch the reduced limit of
autovacuum_work_mem as it might upset people who have higher values in
their postgresql.conf when their database fails to restart after an
upgrade. I think what might be best is just to reduce the limit in
master and apply the doc patch for just maintenance_work_mem in all
supported versions. We could just ignore doing anything with
autovacuum_work_mem in the back branches and put it down to a
historical mistake that can't easily be fixed now.I've attached what I came up with.
What do you think?
[1] /messages/by-id/514fe5ce4714b7b33cb0a611f0c7b72df413bef5.camel@cybertec.at
I think that is much better.
I am fine with that patch.
Yours,
Laurenz Albe
On Sat, 3 Jul 2021 at 00:40, Laurenz Albe <laurenz.albe@cybertec.at> wrote:
On Fri, 2021-07-02 at 23:31 +1200, David Rowley wrote:
I had a look at the patch in [1] and I find it a bit weird that we'd
write the following about autovacuum_work_mem in our docs:+ <para> + Note that <command>VACUUM</command> has a hard-coded limit of 1GB + for the amount of memory used, so setting + <varname>autovacuum_work_mem</varname> higher than that has no effect. + </para>Since that setting is *only* used for auto vacuum, why don't we just
limit the range of the GUC to 1GB?Of course, it wouldn't be wise to backpatch the reduced limit of
autovacuum_work_mem as it might upset people who have higher values in
their postgresql.conf when their database fails to restart after an
upgrade. I think what might be best is just to reduce the limit in
master and apply the doc patch for just maintenance_work_mem in all
supported versions. We could just ignore doing anything with
autovacuum_work_mem in the back branches and put it down to a
historical mistake that can't easily be fixed now.I think that is much better.
I am fine with that patch.
Thanks for looking. I've pushed the doc fix patch for
maintenance_work_mem and back-patched to 9.6.
I could do with a 2nd opinion about if we should just adjust the
maximum value for the autovacuum_work_mem GUC to 1GB in master.
I'm also not sure if since we'd not backpatch the GUC max value
adjustment if we need to document the upper limit in the manual.
David
Attachments:
set_maxvalue_for_autovacuum_work_mem_to_1gb.patchapplication/octet-stream; name=set_maxvalue_for_autovacuum_work_mem_to_1gb.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 480e8cd199..bb76baa72d 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -3340,7 +3340,8 @@ static struct config_int ConfigureNamesInt[] =
GUC_UNIT_KB
},
&autovacuum_work_mem,
- -1, -1, MAX_KILOBYTES,
+ /* see compute_max_dead_tuples if you need to change the max value */
+ -1, -1, 1024 * 1024,
check_autovacuum_work_mem, NULL, NULL
},
On Sun, 4 Jul 2021 at 22:38, David Rowley <dgrowleyml@gmail.com> wrote:
I could do with a 2nd opinion about if we should just adjust the
maximum value for the autovacuum_work_mem GUC to 1GB in master.I'm also not sure if since we'd not backpatch the GUC max value
adjustment if we need to document the upper limit in the manual.
I was just looking at this again and I see that GIN indexes are able
to use more than 1GB of memory during VACUUM. That discovery makes me
think having the docs say that vacuum cannot use more than 1GB of
memory is at best misleading and more likely just incorrect.
Right now I'm considering if it might just be better to revert
ec34040af and call it quits here.
David
On Wed, Jul 7, 2021 at 8:44 PM David Rowley <dgrowleyml@gmail.com> wrote:
On Sun, 4 Jul 2021 at 22:38, David Rowley <dgrowleyml@gmail.com> wrote:
I could do with a 2nd opinion about if we should just adjust the
maximum value for the autovacuum_work_mem GUC to 1GB in master.I'm also not sure if since we'd not backpatch the GUC max value
adjustment if we need to document the upper limit in the manual.I was just looking at this again and I see that GIN indexes are able
to use more than 1GB of memory during VACUUM.
I think you meant that autovacuums can use more than 1GB of memory
during cleaning up a gin pending list (in ginInsertCleanup()). The
description updated by that commit is not true as of now as you
pointed out but IIUC it uses maintenance_work_mem *in addition to* the
same amount memory used by lazy vacuum. This memory usage seems rather
weird to me. Is it worh considering having gin pending list cleanup
use work_mem instead of maintenance_work_mem also in autovacuum cases
like btree indexes do? If we do that, the description will become
true, although we might need to update work_mem section somewhat.
Regards,
--
Masahiko Sawada
EDB: https://www.enterprisedb.com/
On Wed, 7 Jul 2021 at 23:44, David Rowley <dgrowleyml@gmail.com> wrote:
On Sun, 4 Jul 2021 at 22:38, David Rowley <dgrowleyml@gmail.com> wrote:
I could do with a 2nd opinion about if we should just adjust the
maximum value for the autovacuum_work_mem GUC to 1GB in master.I'm also not sure if since we'd not backpatch the GUC max value
adjustment if we need to document the upper limit in the manual.I was just looking at this again and I see that GIN indexes are able
to use more than 1GB of memory during VACUUM. That discovery makes me
think having the docs say that vacuum cannot use more than 1GB of
memory is at best misleading and more likely just incorrect.
The attached patch aims to put right where I went wrong with the
documentation about vacuum/autovacuum only using maintainance_work_mem
memory for dead tuple collection.
I plan to push this and backpatch to 9.6 shortly unless there are any
better ideas.
What's in there right now is wrong and I want that fixed before the
cut-off for the next set of bug fix releases.
David
Attachments:
fix_wrong_documentation_on_vacuum_mem_limits.patchapplication/octet-stream; name=fix_wrong_documentation_on_vacuum_mem_limits.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 3eefe3811a..30bca45657 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1894,10 +1894,9 @@ include_dir 'conf.d'
setting <xref linkend="guc-autovacuum-work-mem"/>.
</para>
<para>
- Additionally, <command>VACUUM</command> is only able to utilize up to
- a maximum of <literal>1GB</literal> of memory, so
- <varname>maintenance_work_mem</varname> values higher than this have
- no effect on <command>VACUUM</command>.
+ Note that for the collection of dead tuple identifiers,
+ <command>VACUUM</command> is only able to utilize up to a maximum of
+ <literal>1GB</literal> of memory.
</para>
</listitem>
</varlistentry>
@@ -1921,6 +1920,14 @@ include_dir 'conf.d'
<filename>postgresql.conf</filename> file or on the server command
line.
</para>
+ <para>
+ For the collection of dead tuple identifiers,
+ autovacuum is only able to utilize up to a maximum of
+ <literal>1GB</literal> of memory, so setting
+ <varname>autovacuum_work_mem</varname> to a value higher than that has
+ no effect on the number of dead tuples that autovacuum can collect
+ while scanning a table.
+ </para>
</listitem>
</varlistentry>
On Mon, 9 Aug 2021 at 14:44, David Rowley <dgrowleyml@gmail.com> wrote:
I plan to push this and backpatch to 9.6 shortly unless there are any
better ideas.
I pushed this patch. I've now marked the entry in the commitfest app
as committed too.
David