Date: Thu, 27 Feb 2025 21:35:51 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Laurenz Albe <laurenz.albe@cybertec.at>,
	Gurjeet Singh <gurjeet@singh.im>,
	Postgres Hackers <pgsql-hackers@postgresql.org>,
	Will Storey <will@summercat.com>
Subject: Re: Disabling vacuum truncate for autovacuum
Message-ID: <Z8EvFyf1D1AMxjOg@nathan>
References: <Z2DE4lDX4tHqNGZt@dev.null>
 <fa83a77bbca5da4d4864a49c8a27df323f23d28b.camel@cybertec.at>
 <CABwTF4U3xkF=ZRi2pztUDxohoN8h6XL10=QmTtuTXoMjzu5-zg@mail.gmail.com>
 <f82fc7fbb69a8c8000ad74ced50381257fe57838.camel@cybertec.at>
 <CA+TgmoZMXN19eorKdeiiFCv3AJFVaUAfkzRuamnt8A9U8uJSqg@mail.gmail.com>
 <Z7Tl2d7HrG1AQEOc@nathan>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="WPB+5+kPQlKEwC0K"
Content-Disposition: inline
In-Reply-To: <Z7Tl2d7HrG1AQEOc@nathan>
Archived-At: <https://www.postgresql.org/message-id/Z8EvFyf1D1AMxjOg%40nathan>
Precedence: bulk


--WPB+5+kPQlKEwC0K
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Feb 18, 2025 at 01:56:09PM -0600, Nathan Bossart wrote:
> On Mon, Jan 27, 2025 at 03:38:39PM -0500, Robert Haas wrote:
>> Also, how sure are we that turning this off globally is a solid idea?
>> Off-hand, it doesn't sound that bad: there are probably situations in
>> which autovacuum never truncates anything anyway just because the tail
>> blocks of the relations always contain at least 1 tuple. But we should
>> be careful not to add a setting that is far more likely to get people
>> into trouble than to get them out of it. It would be good to hear what
>> other people think about the risk vs. reward tradeoff in this case.
> 
> My first reaction is that a global setting is probably fine most of the
> time.  I'm sure it's possible to get into bad situations if you try hard
> enough, but that's not a unique characteristic.  There are probably many
> situations where the truncation is wasted effort because we'll just end up
> extending the relation shortly afterwards, anyway.  In any case, it's
> already possible to achieve $SUBJECT with a trivial script that sets
> storage parameters on all tables, so IMHO it would be silly to withhold a
> global setting that does the same thing just on principle.

I spent some time on this one today.  A couple of notes:

* Since the reloption is a Boolean, there isn't a good way to tell whether
  it is actually set for the table or if it just inherited the default
  value.  This is important to know because we want the reloption to
  override the GUC.  I considered a bunch of different ways to handle this,
  but everything felt like a cowboy hack.  The cleanest cowboy hack I could
  come up with is an optional offset field in relopt_parse_elt that points
  to a variable that stores whether the option was explicitly set.

* I didn't see a good GUC category for vacuum_truncate.  I suppose we could
  create a new one, but for now I've just stashed it into the autovacuum
  one.  Bikeshedding welcome.

Thoughts?

-- 
nathan

--WPB+5+kPQlKEwC0K
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-Add-vacuum_truncate-GUC.patch"

From e360b56acd1d3bd05c9df6cfc4586e51edce357e Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Thu, 27 Feb 2025 21:09:37 -0600
Subject: [PATCH v2 1/1] Add vacuum_truncate GUC.

---
 doc/src/sgml/config.sgml                      | 22 +++++++++++++++++++
 doc/src/sgml/ref/create_table.sgml            | 10 +--------
 doc/src/sgml/ref/vacuum.sgml                  |  3 ++-
 src/backend/access/common/reloptions.c        | 11 ++++++++--
 src/backend/commands/vacuum.c                 | 17 ++++++++++----
 src/backend/utils/misc/guc_tables.c           |  9 ++++++++
 src/backend/utils/misc/postgresql.conf.sample |  1 +
 src/include/access/reloptions.h               |  1 +
 src/include/commands/vacuum.h                 |  1 +
 src/include/utils/rel.h                       |  1 +
 10 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e55700f35b8..069ac35762f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8934,6 +8934,28 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
        </listitem>
       </varlistentry>
 
+      <varlistentry id="guc-vacuum-truncate" xreflabel="autovacuum">
+       <term><varname>vacuum_truncate</varname> (<type>boolean</type>)
+       <indexterm>
+        <primary><varname>vacuum_truncate</varname> configuration parameter</primary>
+       </indexterm>
+       </term>
+       <listitem>
+        <para>
+         Enables or disables vacuum to try to truncate off any empty pages at
+         the end of the table.  The default value is <literal>true</literal>.
+         If <literal>true</literal>, <command>VACUUM</command> and autovacuum
+         do the truncation and the disk space for the truncated pages is
+         returned to the operating system.  Note that the truncation requires
+         an <literal>ACCESS EXCLUSIVE</literal> lock on the table.  The
+         <literal>TRUNCATE</literal> parameter of
+         <link linkend="sql-vacuum"><command>VACUUM</command></link>, if
+         specified, overrides the value of this parameter.  The setting can be
+         overridden for individual tables by changing table storage parameters.
+        </para>
+       </listitem>
+      </varlistentry>
+
      </variablelist>
     </sect2>
 
diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml
index 0a3e520f215..3c2315b1a8e 100644
--- a/doc/src/sgml/ref/create_table.sgml
+++ b/doc/src/sgml/ref/create_table.sgml
@@ -1688,15 +1688,7 @@ WITH ( MODULUS <replaceable class="parameter">numeric_literal</replaceable>, REM
     </term>
     <listitem>
      <para>
-      Enables or disables vacuum to try to truncate off any empty pages
-      at the end of this table. The default value is <literal>true</literal>.
-      If <literal>true</literal>, <command>VACUUM</command> and
-      autovacuum do the truncation and the disk space for
-      the truncated pages is returned to the operating system.
-      Note that the truncation requires <literal>ACCESS EXCLUSIVE</literal>
-      lock on the table. The <literal>TRUNCATE</literal> parameter
-      of <link linkend="sql-vacuum"><command>VACUUM</command></link>, if specified, overrides the value
-      of this option.
+      Per-table value for <xref linkend="guc-vacuum-truncate"/> parameter.
      </para>
     </listitem>
    </varlistentry>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index 971b1237d47..bd5dcaf86a5 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -265,7 +265,8 @@ VACUUM [ ( <replaceable class="parameter">option</replaceable> [, ...] ) ] [ <re
       truncate off any empty pages at the end of the table and allow
       the disk space for the truncated pages to be returned to
       the operating system. This is normally the desired behavior
-      and is the default unless the <literal>vacuum_truncate</literal>
+      and is the default unless <xref linkend="guc-vacuum-truncate"/>
+      is set to false or the <literal>vacuum_truncate</literal>
       option has been set to false for the table to be vacuumed.
       Setting this option to false may be useful to avoid
       <literal>ACCESS EXCLUSIVE</literal> lock on the table that
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 59fb53e7707..c0a7c0832ab 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -1779,6 +1779,13 @@ fillRelOptions(void *rdopts, Size basesize,
 				char	   *itempos = ((char *) rdopts) + elems[j].offset;
 				char	   *string_val;
 
+				if (elems[j].isset_offset > 0)
+				{
+					char	   *setpos = ((char *) rdopts) + elems[j].isset_offset;
+
+					*(bool *) setpos = options[i].isset;
+				}
+
 				switch (options[i].gen->type)
 				{
 					case RELOPT_TYPE_BOOL:
@@ -1901,7 +1908,7 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		{"vacuum_index_cleanup", RELOPT_TYPE_ENUM,
 		offsetof(StdRdOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)},
+		offsetof(StdRdOptions, vacuum_truncate), offsetof(StdRdOptions, vacuum_truncate_set)},
 		{"vacuum_max_eager_freeze_failure_rate", RELOPT_TYPE_REAL,
 		offsetof(StdRdOptions, vacuum_max_eager_freeze_failure_rate)}
 	};
@@ -1968,7 +1975,7 @@ void *
 build_local_reloptions(local_relopts *relopts, Datum options, bool validate)
 {
 	int			noptions = list_length(relopts->options);
-	relopt_parse_elt *elems = palloc(sizeof(*elems) * noptions);
+	relopt_parse_elt *elems = palloc0(sizeof(*elems) * noptions);
 	relopt_value *vals;
 	void	   *opts;
 	int			i = 0;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 0239d9bae65..b12c623be0a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -78,6 +78,7 @@ int			vacuum_failsafe_age;
 int			vacuum_multixact_failsafe_age;
 double		vacuum_max_eager_freeze_failure_rate;
 bool		track_cost_delay_timing;
+bool		vacuum_truncate;
 
 /*
  * Variables for cost-based vacuum delay. The defaults differ between
@@ -2192,13 +2193,21 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
 			((StdRdOptions *) rel->rd_options)->vacuum_max_eager_freeze_failure_rate;
 
 	/*
-	 * Set truncate option based on truncate reloption if it wasn't specified
-	 * in VACUUM command, or when running in an autovacuum worker
+	 * Set truncate option based on truncate reloption or GUC if it wasn't
+	 * specified in VACUUM command, or when running in an autovacuum worker
 	 */
 	if (params->truncate == VACOPTVALUE_UNSPECIFIED)
 	{
-		if (rel->rd_options == NULL ||
-			((StdRdOptions *) rel->rd_options)->vacuum_truncate)
+		StdRdOptions *opts = (StdRdOptions *) rel->rd_options;
+
+		if (opts && opts->vacuum_truncate_set)
+		{
+			if (opts->vacuum_truncate)
+				params->truncate = VACOPTVALUE_ENABLED;
+			else
+				params->truncate = VACOPTVALUE_DISABLED;
+		}
+		else if (vacuum_truncate)
 			params->truncate = VACOPTVALUE_ENABLED;
 		else
 			params->truncate = VACOPTVALUE_DISABLED;
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index ad25cbb39c5..e38fe56d3c2 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2118,6 +2118,15 @@ struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"vacuum_truncate", PGC_USERSET, VACUUM_AUTOVACUUM,
+			gettext_noop("Enables vacuum to truncate empty pages at the end of the table."),
+		},
+		&vacuum_truncate,
+		true,
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 5362ff80519..de012617898 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -692,6 +692,7 @@ autovacuum_worker_slots = 16	# autovacuum worker slots to allocate
 #autovacuum_vacuum_cost_limit = -1	# default vacuum cost limit for
 					# autovacuum, -1 means use
 					# vacuum_cost_limit
+#vacuum_truncate = on			# enable truncation after vacuum
 
 # - Cost-Based Vacuum Delay -
 
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 43445cdcc6c..61e58a5daa3 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -152,6 +152,7 @@ typedef struct
 	const char *optname;		/* option's name */
 	relopt_type opttype;		/* option's datatype */
 	int			offset;			/* offset of field in result struct */
+	int			isset_offset;	/* if > 0, offset of "set" field */
 } relopt_parse_elt;
 
 /* Local reloption definition */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 1571a66c6bf..d6770e2d44a 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -304,6 +304,7 @@ extern PGDLLIMPORT int vacuum_multixact_freeze_table_age;
 extern PGDLLIMPORT int vacuum_failsafe_age;
 extern PGDLLIMPORT int vacuum_multixact_failsafe_age;
 extern PGDLLIMPORT bool track_cost_delay_timing;
+extern PGDLLIMPORT bool vacuum_truncate;
 
 /*
  * Relevant for vacuums implementing eager scanning. Normal vacuums may
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index db3e504c3d2..d94fddd7cef 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -344,6 +344,7 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	StdRdOptIndexCleanup vacuum_index_cleanup;	/* controls index vacuuming */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+	bool		vacuum_truncate_set;	/* whether vacuum_truncate is set */
 
 	/*
 	 * Fraction of pages in a relation that vacuum can eagerly scan and fail
-- 
2.39.5 (Apple Git-154)


--WPB+5+kPQlKEwC0K--