agora inbox for pgsql-hackers@postgresql.org  
help / color / mirror / Atom feed
From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@lists.postgresql.org, "Imseih (AWS), Sami" <simseih@amazon.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: allow changing autovacuum_max_workers without restarting
Date: Mon, 6 Jan 2025 20:44:37 -0600
Message-ID: <Z3yVFfKG_RAh8FgP@nathan> (raw)
In-Reply-To: <1359669.1736206603@sss.pgh.pa.us>
References: <Z2XJjI8fO8ksOv4E@nathan>
	<Z3v8UFk0sOgFgWeZ@nathan>
	<Z3xFE7DQTSRJw2FM@nathan>
	<1346002.1736198977@sss.pgh.pa.us>
	<Z3xQIPX3aeri4DiT@nathan>
	<Z3xV_b3wp-XQR3Rm@nathan>
	<7B8590B5-3A23-4CF6-B21D-4AA8C0177C2C@anarazel.de>
	<1353350.1736202977@sss.pgh.pa.us>
	<Z3xlNmC3qtwf1lz1@nathan>
	<1359669.1736206603@sss.pgh.pa.us>

On Mon, Jan 06, 2025 at 06:36:43PM -0500, Tom Lane wrote:
> Nathan Bossart <nathandbossart@gmail.com> writes:
>> My first instinct was just to set it to the lowest default we'd consider
>> during the max_connections tests (which I'm assuming is 3 due to the
>> current default for autovacuum_max_workers).  That way, the max_connections
>> default won't change from version to version on affected systems, but you
>> might get some extra autovacuum slots.
> 
> My only objection to this algorithm is it adds cycles to initdb,
> in the form of at least one additional "postgres --check" step.
> Admittedly that's not hugely expensive, but it'll add up over time
> in the buildfarm, and I'm not sure this issue is worth that.
> We already changed the max_connections default for affected systems
> as a consequence of 38da05346, so I don't think the argument about not
> changing it holds much water.

I see.  Here's a version that uses your max_connections / 8 idea.  I've
lowered the initial default value of autovacuum_worker_slots to 12 to keep
the code as simple as possible.  I considered trying 16 in the first
iteration or constructing a complicated formula like

	autovacuum_worker_slots = (max_connections * 13) / 75 - 1

but this stuff is pretty fragile already, so I felt that simplicity was
desirable in this case.

-- 
nathan
From a2cb9161ccd7bf08c5273e4b4e012861a410ce51 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 6 Jan 2025 17:14:54 -0600
Subject: [PATCH v2 1/1] Lower default value of autovacuum_worker_slots in
 initdb as needed.

---
 doc/src/sgml/config.sgml                      |  5 +--
 src/backend/utils/misc/guc_tables.c           |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/bin/initdb/initdb.c                       | 34 +++++++++++++++----
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..a47c4dbfcb2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8639,8 +8639,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       <listitem>
        <para>
         Specifies the number of backend slots to reserve for autovacuum worker
-        processes.  The default is 16.  This parameter can only be set at server
-        start.
+        processes.  The default is typically 12 slots, but might be less if
+        your kernel settings will not support it (as determined during initdb).
+        This parameter can only be set at server start.
        </para>
        <para>
         When changing this value, consider also adjusting
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c9d8cd796a8..8781495a622 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3472,7 +3472,7 @@ struct config_int ConfigureNamesInt[] =
 			NULL
 		},
 		&autovacuum_worker_slots,
-		16, 1, MAX_BACKENDS,
+		12, 1, MAX_BACKENDS,
 		NULL, NULL, NULL
 	},
 	{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index b2bc43383db..3405ca0e726 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -661,7 +661,7 @@
 
 #autovacuum = on			# Enable autovacuum subprocess?  'on'
 					# requires track_counts to also be on.
-autovacuum_worker_slots = 16	# autovacuum worker slots to allocate
+autovacuum_worker_slots = 12	# autovacuum worker slots to allocate
 					# (change requires restart)
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 #autovacuum_naptime = 1min		# time between autovacuum runs
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..b5023484771 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -196,6 +196,7 @@ static char *pgdata_native;
 
 /* defaults */
 static int	n_connections = 10;
+static int	n_av_slots = 12;
 static int	n_buffers = 50;
 static const char *dynamic_shared_memory_type = NULL;
 static const char *default_timezone = NULL;
@@ -273,7 +274,8 @@ static void check_input(char *path);
 static void write_version_file(const char *extrapath);
 static void set_null_conf(void);
 static void test_config_settings(void);
-static bool test_specific_config_settings(int test_conns, int test_buffs);
+static bool test_specific_config_settings(int test_conns, int test_av_slots,
+										  int test_buffs);
 static void setup_config(void);
 static void bootstrap_template1(void);
 static void setup_auth(FILE *cmdfd);
@@ -1118,6 +1120,12 @@ test_config_settings(void)
 	 */
 #define MIN_BUFS_FOR_CONNS(nconns)	((nconns) * 10)
 
+	/*
+	 * This macro defines the default value of autovacuum_worker_slots we want
+	 * for a given max_connections value.
+	 */
+#define AV_SLOTS_FOR_CONNS(nconns)	((nconns) / 8)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1145,7 +1153,8 @@ test_config_settings(void)
 
 	/*
 	 * Probe for max_connections before shared_buffers, since it is subject to
-	 * more constraints than shared_buffers.
+	 * more constraints than shared_buffers.  We also choose the default
+	 * autovacuum_worker_slots here.
 	 */
 	printf(_("selecting default \"max_connections\" ... "));
 	fflush(stdout);
@@ -1154,8 +1163,9 @@ test_config_settings(void)
 	{
 		test_conns = trial_conns[i];
 		test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
+		n_av_slots = AV_SLOTS_FOR_CONNS(test_conns);
 
-		if (test_specific_config_settings(test_conns, test_buffs))
+		if (test_specific_config_settings(test_conns, n_av_slots, test_buffs))
 		{
 			ok_buffers = test_buffs;
 			break;
@@ -1167,6 +1177,13 @@ test_config_settings(void)
 
 	printf("%d\n", n_connections);
 
+	/*
+	 * We chose the default for autovacuum_worker_slots during the
+	 * max_connections tests above, but we print a progress message anyway.
+	 */
+	printf(_("selecting default \"autovacuum_worker_slots\" ... %d\n"),
+		   n_av_slots);
+
 	printf(_("selecting default \"shared_buffers\" ... "));
 	fflush(stdout);
 
@@ -1180,7 +1197,7 @@ test_config_settings(void)
 			break;
 		}
 
-		if (test_specific_config_settings(n_connections, test_buffs))
+		if (test_specific_config_settings(n_connections, n_av_slots, test_buffs))
 			break;
 	}
 	n_buffers = test_buffs;
@@ -1200,7 +1217,7 @@ test_config_settings(void)
  * Test a specific combination of configuration settings.
  */
 static bool
-test_specific_config_settings(int test_conns, int test_buffs)
+test_specific_config_settings(int test_conns, int test_av_slots, int test_buffs)
 {
 	PQExpBufferData cmd;
 	_stringlist *gnames,
@@ -1213,10 +1230,11 @@ test_specific_config_settings(int test_conns, int test_buffs)
 	printfPQExpBuffer(&cmd,
 					  "\"%s\" --check %s %s "
 					  "-c max_connections=%d "
+					  "-c autovacuum_worker_slots=%d "
 					  "-c shared_buffers=%d "
 					  "-c dynamic_shared_memory_type=%s",
 					  backend_exec, boot_options, extra_options,
-					  test_conns, test_buffs,
+					  test_conns, test_av_slots, test_buffs,
 					  dynamic_shared_memory_type);
 
 	/* Add any user-given setting overrides */
@@ -1280,6 +1298,10 @@ setup_config(void)
 	conflines = replace_guc_value(conflines, "max_connections",
 								  repltok, false);
 
+	snprintf(repltok, sizeof(repltok), "%d", n_av_slots);
+	conflines = replace_guc_value(conflines, "autovacuum_worker_slots",
+								  repltok, false);
+
 	if ((n_buffers * (BLCKSZ / 1024)) % 1024 == 0)
 		snprintf(repltok, sizeof(repltok), "%dMB",
 				 (n_buffers * (BLCKSZ / 1024)) / 1024);
-- 
2.39.5 (Apple Git-154)

Attachments:

  [text/plain] v2-0001-Lower-default-value-of-autovacuum_worker_slots-in.patch (6.1K, ../Z3yVFfKG_RAh8FgP@nathan/2-v2-0001-Lower-default-value-of-autovacuum_worker_slots-in.patch)
  download | inline diff:
From a2cb9161ccd7bf08c5273e4b4e012861a410ce51 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 6 Jan 2025 17:14:54 -0600
Subject: [PATCH v2 1/1] Lower default value of autovacuum_worker_slots in
 initdb as needed.

---
 doc/src/sgml/config.sgml                      |  5 +--
 src/backend/utils/misc/guc_tables.c           |  2 +-
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/bin/initdb/initdb.c                       | 34 +++++++++++++++----
 4 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..a47c4dbfcb2 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8639,8 +8639,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       <listitem>
        <para>
         Specifies the number of backend slots to reserve for autovacuum worker
-        processes.  The default is 16.  This parameter can only be set at server
-        start.
+        processes.  The default is typically 12 slots, but might be less if
+        your kernel settings will not support it (as determined during initdb).
+        This parameter can only be set at server start.
        </para>
        <para>
         When changing this value, consider also adjusting
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c9d8cd796a8..8781495a622 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -3472,7 +3472,7 @@ struct config_int ConfigureNamesInt[] =
 			NULL
 		},
 		&autovacuum_worker_slots,
-		16, 1, MAX_BACKENDS,
+		12, 1, MAX_BACKENDS,
 		NULL, NULL, NULL
 	},
 	{
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index b2bc43383db..3405ca0e726 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -661,7 +661,7 @@
 
 #autovacuum = on			# Enable autovacuum subprocess?  'on'
 					# requires track_counts to also be on.
-autovacuum_worker_slots = 16	# autovacuum worker slots to allocate
+autovacuum_worker_slots = 12	# autovacuum worker slots to allocate
 					# (change requires restart)
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 #autovacuum_naptime = 1min		# time between autovacuum runs
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..b5023484771 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -196,6 +196,7 @@ static char *pgdata_native;
 
 /* defaults */
 static int	n_connections = 10;
+static int	n_av_slots = 12;
 static int	n_buffers = 50;
 static const char *dynamic_shared_memory_type = NULL;
 static const char *default_timezone = NULL;
@@ -273,7 +274,8 @@ static void check_input(char *path);
 static void write_version_file(const char *extrapath);
 static void set_null_conf(void);
 static void test_config_settings(void);
-static bool test_specific_config_settings(int test_conns, int test_buffs);
+static bool test_specific_config_settings(int test_conns, int test_av_slots,
+										  int test_buffs);
 static void setup_config(void);
 static void bootstrap_template1(void);
 static void setup_auth(FILE *cmdfd);
@@ -1118,6 +1120,12 @@ test_config_settings(void)
 	 */
 #define MIN_BUFS_FOR_CONNS(nconns)	((nconns) * 10)
 
+	/*
+	 * This macro defines the default value of autovacuum_worker_slots we want
+	 * for a given max_connections value.
+	 */
+#define AV_SLOTS_FOR_CONNS(nconns)	((nconns) / 8)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1145,7 +1153,8 @@ test_config_settings(void)
 
 	/*
 	 * Probe for max_connections before shared_buffers, since it is subject to
-	 * more constraints than shared_buffers.
+	 * more constraints than shared_buffers.  We also choose the default
+	 * autovacuum_worker_slots here.
 	 */
 	printf(_("selecting default \"max_connections\" ... "));
 	fflush(stdout);
@@ -1154,8 +1163,9 @@ test_config_settings(void)
 	{
 		test_conns = trial_conns[i];
 		test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
+		n_av_slots = AV_SLOTS_FOR_CONNS(test_conns);
 
-		if (test_specific_config_settings(test_conns, test_buffs))
+		if (test_specific_config_settings(test_conns, n_av_slots, test_buffs))
 		{
 			ok_buffers = test_buffs;
 			break;
@@ -1167,6 +1177,13 @@ test_config_settings(void)
 
 	printf("%d\n", n_connections);
 
+	/*
+	 * We chose the default for autovacuum_worker_slots during the
+	 * max_connections tests above, but we print a progress message anyway.
+	 */
+	printf(_("selecting default \"autovacuum_worker_slots\" ... %d\n"),
+		   n_av_slots);
+
 	printf(_("selecting default \"shared_buffers\" ... "));
 	fflush(stdout);
 
@@ -1180,7 +1197,7 @@ test_config_settings(void)
 			break;
 		}
 
-		if (test_specific_config_settings(n_connections, test_buffs))
+		if (test_specific_config_settings(n_connections, n_av_slots, test_buffs))
 			break;
 	}
 	n_buffers = test_buffs;
@@ -1200,7 +1217,7 @@ test_config_settings(void)
  * Test a specific combination of configuration settings.
  */
 static bool
-test_specific_config_settings(int test_conns, int test_buffs)
+test_specific_config_settings(int test_conns, int test_av_slots, int test_buffs)
 {
 	PQExpBufferData cmd;
 	_stringlist *gnames,
@@ -1213,10 +1230,11 @@ test_specific_config_settings(int test_conns, int test_buffs)
 	printfPQExpBuffer(&cmd,
 					  "\"%s\" --check %s %s "
 					  "-c max_connections=%d "
+					  "-c autovacuum_worker_slots=%d "
 					  "-c shared_buffers=%d "
 					  "-c dynamic_shared_memory_type=%s",
 					  backend_exec, boot_options, extra_options,
-					  test_conns, test_buffs,
+					  test_conns, test_av_slots, test_buffs,
 					  dynamic_shared_memory_type);
 
 	/* Add any user-given setting overrides */
@@ -1280,6 +1298,10 @@ setup_config(void)
 	conflines = replace_guc_value(conflines, "max_connections",
 								  repltok, false);
 
+	snprintf(repltok, sizeof(repltok), "%d", n_av_slots);
+	conflines = replace_guc_value(conflines, "autovacuum_worker_slots",
+								  repltok, false);
+
 	if ((n_buffers * (BLCKSZ / 1024)) % 1024 == 0)
 		snprintf(repltok, sizeof(repltok), "%dMB",
 				 (n_buffers * (BLCKSZ / 1024)) / 1024);
-- 
2.39.5 (Apple Git-154)

view thread (71+ messages)  latest in thread

Message-ID: <Z3yVFfKG_RAh8FgP@nathan>
Permalink:  ../Z3yVFfKG_RAh8FgP@nathan/
Also on:    postgresql.org/message-id/Z3yVFfKG_RAh8FgP@nathan

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, andres@anarazel.de, simseih@amazon.com, pryzby@telsasoft.com
  Subject: Re: allow changing autovacuum_max_workers without restarting
  In-Reply-To: <Z3yVFfKG_RAh8FgP@nathan>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox