Subject: Update maintenance_work_mem/autovacuum_work_mem to reflect the 1GB limitation with VACUUM

From: Martín Marqués <[email-masked]>
Date: Mon, 3 May 2021 13:48:21 -0300

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

---

From: Laurenz Albe <[email-masked]>
Date: Wed, 05 May 2021 12:03:08 +0200

> affects all supported versions), at least until such limitation is

Here is a patch for that,

Yours,
Laurenz Albe
From 69ba1f6bef0209fa1093c39ba2e731641d7f5188 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email-masked]>
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

---

From: Laurenz Albe <[email-masked]>
Date: Thu, 20 May 2021 17:52:38 +0200

On Wed, 2021-05-05 at 12:03 +0200, Laurenz Albe wrote:
> Here is a patch for that

Just sending a reply to -hackers so I can add it to the commitfest.

Yours,
Laurenz Albe

---

From: Greg Sabino Mullane <[email-masked]>
Date: Tue, 01 Jun 2021 19:25:18 +0000

s/Node/Note/

Other than that, +1 to the patch and +1 to backpatching.

The new status of this patch is: Waiting on Author

---

From: Masahiko Sawada <[email-masked]>
Date: Wed, 2 Jun 2021 18:16:08 +0900

> 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/

---

From: Laurenz Albe <[email-masked]>
Date: Fri, 04 Jun 2021 14:34:42 +0200

> 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
From a94811e45df96846cb0531615e9d773f69d2cc2d Mon Sep 17 00:00:00 2001
From: Laurenz Albe <[email-masked]>
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

---

From: Greg Sabino Mullane <[email-masked]>
Date: Fri, 04 Jun 2021 15:16:36 +0000

Should we say "currently has"?

---

From: Greg Sabino Mullane <[email-masked]>
Date: Thu, 17 Jun 2021 14:56:52 +0000

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

---

From: David Rowley <[email-masked]>
Date: Fri, 2 Jul 2021 23:31:07 +1200

> Just sending a reply to -hackers so I can add it to the commitfest.

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>

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] https://www.postgresql.org/message-id/514fe5ce4714b7b33cb0a611f0c7b72df413bef5.camel%40cybertec.at

---

From: Laurenz Albe <[email-masked]>
Date: Fri, 02 Jul 2021 14:40:52 +0200

> [1] https://www.postgresql.org/message-id/514fe5ce4714b7b33cb0a611f0c7b72df413bef5.camel%40cybertec.at

I think that is much better.
I am fine with that patch.

Yours,
Laurenz Albe

---

From: David Rowley <[email-masked]>
Date: Sun, 4 Jul 2021 22:38:33 +1200

> 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

---

From: David Rowley <[email-masked]>
Date: Wed, 7 Jul 2021 23:44:16 +1200

> 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

---

From: Masahiko Sawada <[email-masked]>
Date: Mon, 19 Jul 2021 17:27:10 +0900

On Wed, Jul 7, 2021 at 8:44 PM David Rowley <[email-masked]> wrote:
> 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,

---

From: David Rowley <[email-masked]>
Date: Mon, 9 Aug 2021 14:44:13 +1200

> 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

---

From: David Rowley <[email-masked]>
Date: Mon, 9 Aug 2021 16:53:02 +1200

> I plan to push this and backpatch to 9.6 shortly unless there are any

I pushed this patch.  I've now marked the entry in the commitfest app
as committed too.

David