($INBOX_DIR/description missing)  

help / color / mirror / Atom feed

[PATCH v2 2/4] make binaryheap available to frontend
20+ messages / 5 participants
[nested] [flat]

* [PATCH v2 2/4] make binaryheap available to frontend
@ 2023-07-20 16:45 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:45 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 15 ++++++++++++
 src/common/meson.build                   |  1 +
 src/include/lib/binaryheap.h             | 29 ++++++++++++++++++++++++
 6 files changed, 46 insertions(+), 2 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (96%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 96%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index 1737546757..7a590eec4a 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -11,10 +11,17 @@
  *-------------------------------------------------------------------------
  */
 
+#ifndef FRONTEND
 #include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -109,7 +116,11 @@ void
 binaryheap_add_unordered(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -141,7 +152,11 @@ void
 binaryheap_add(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..06e3878cf7 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -11,6 +11,35 @@
 #ifndef BINARYHEAP_H
 #define BINARYHEAP_H
 
+/*
+ * XXX: This is obviously a hack.
+ */
+#ifdef FRONTEND
+
+typedef uintptr_t Datum;
+
+/*
+ * DatumGetPointer
+ *		Returns pointer value of a datum.
+ */
+static inline Pointer
+DatumGetPointer(Datum X)
+{
+	return (Pointer) X;
+}
+
+/*
+ * PointerGetDatum
+ *		Returns datum representation for a pointer.
+ */
+static inline Datum
+PointerGetDatum(const void *X)
+{
+	return (Datum) X;
+}
+
+#endif
+
 /*
  * For a max-heap, the comparator must return <0 iff a < b, 0 iff a == b,
  * and >0 iff a > b.  For a min-heap, the conditions are reversed.
-- 
2.25.1


--qDbXVdCdHGoSgWSk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0003-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* [PATCH v2 2/4] make binaryheap available to frontend
@ 2023-07-20 16:45 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-20 16:45 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 15 ++++++++++++
 src/common/meson.build                   |  1 +
 src/include/lib/binaryheap.h             | 29 ++++++++++++++++++++++++
 6 files changed, 46 insertions(+), 2 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (96%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 96%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index 1737546757..7a590eec4a 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -11,10 +11,17 @@
  *-------------------------------------------------------------------------
  */
 
+#ifndef FRONTEND
 #include "postgres.h"
+#else
+#include "postgres_fe.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -109,7 +116,11 @@ void
 binaryheap_add_unordered(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -141,7 +152,11 @@ void
 binaryheap_add(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..06e3878cf7 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -11,6 +11,35 @@
 #ifndef BINARYHEAP_H
 #define BINARYHEAP_H
 
+/*
+ * XXX: This is obviously a hack.
+ */
+#ifdef FRONTEND
+
+typedef uintptr_t Datum;
+
+/*
+ * DatumGetPointer
+ *		Returns pointer value of a datum.
+ */
+static inline Pointer
+DatumGetPointer(Datum X)
+{
+	return (Pointer) X;
+}
+
+/*
+ * PointerGetDatum
+ *		Returns datum representation for a pointer.
+ */
+static inline Datum
+PointerGetDatum(const void *X)
+{
+	return (Datum) X;
+}
+
+#endif
+
 /*
  * For a max-heap, the comparator must return <0 iff a < b, 0 iff a == b,
  * and >0 iff a > b.  For a min-heap, the conditions are reversed.
-- 
2.25.1


--qDbXVdCdHGoSgWSk
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0003-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* [PATCH v3 2/4] make binaryheap available to frontend
@ 2023-07-22 22:04 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-22 22:04 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 17 ++++++++++++++++-
 src/common/meson.build                   |  1 +
 5 files changed, 18 insertions(+), 3 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (96%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 96%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index 1737546757..f21838b946 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -6,15 +6,22 @@
  * Portions Copyright (c) 2012-2023, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  src/backend/lib/binaryheap.c
+ *	  src/common/binaryheap.c
  *
  *-------------------------------------------------------------------------
  */
 
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
 #include "postgres.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -109,7 +116,11 @@ void
 binaryheap_add_unordered(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -141,7 +152,11 @@ void
 binaryheap_add(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
-- 
2.25.1


--sdtB3X0nJg68CQEu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0003-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* [PATCH v5 1/3] make binaryheap available to frontend
@ 2023-07-22 22:04 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-22 22:04 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 37 +++++++++++++++++-------
 src/common/meson.build                   |  1 +
 src/include/lib/binaryheap.h             | 23 ++++++++++-----
 6 files changed, 44 insertions(+), 20 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (91%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 91%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index 1737546757..a6be4b42ae 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -6,15 +6,22 @@
  * Portions Copyright (c) 2012-2023, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  src/backend/lib/binaryheap.c
+ *	  src/common/binaryheap.c
  *
  *-------------------------------------------------------------------------
  */
 
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
 #include "postgres.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -34,7 +41,7 @@ binaryheap_allocate(int capacity, binaryheap_comparator compare, void *arg)
 	int			sz;
 	binaryheap *heap;
 
-	sz = offsetof(binaryheap, bh_nodes) + sizeof(Datum) * capacity;
+	sz = offsetof(binaryheap, bh_nodes) + sizeof(bh_elem_type) * capacity;
 	heap = (binaryheap *) palloc(sz);
 	heap->bh_space = capacity;
 	heap->bh_compare = compare;
@@ -106,10 +113,14 @@ parent_offset(int i)
  * afterwards.
  */
 void
-binaryheap_add_unordered(binaryheap *heap, Datum d)
+binaryheap_add_unordered(binaryheap *heap, bh_elem_type d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -138,10 +149,14 @@ binaryheap_build(binaryheap *heap)
  * the heap property.
  */
 void
-binaryheap_add(binaryheap *heap, Datum d)
+binaryheap_add(binaryheap *heap, bh_elem_type d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
@@ -154,7 +169,7 @@ binaryheap_add(binaryheap *heap, Datum d)
  * without modifying the heap. The caller must ensure that this
  * routine is not used on an empty heap. Always O(1).
  */
-Datum
+bh_elem_type
 binaryheap_first(binaryheap *heap)
 {
 	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
@@ -169,10 +184,10 @@ binaryheap_first(binaryheap *heap)
  * that this routine is not used on an empty heap. O(log n) worst
  * case.
  */
-Datum
+bh_elem_type
 binaryheap_remove_first(binaryheap *heap)
 {
-	Datum		result;
+	bh_elem_type result;
 
 	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
 
@@ -204,7 +219,7 @@ binaryheap_remove_first(binaryheap *heap)
  * sifting the new node down.
  */
 void
-binaryheap_replace_first(binaryheap *heap, Datum d)
+binaryheap_replace_first(binaryheap *heap, bh_elem_type d)
 {
 	Assert(!binaryheap_empty(heap) && heap->bh_has_heap_property);
 
@@ -221,7 +236,7 @@ binaryheap_replace_first(binaryheap *heap, Datum d)
 static void
 sift_up(binaryheap *heap, int node_off)
 {
-	Datum		node_val = heap->bh_nodes[node_off];
+	bh_elem_type node_val = heap->bh_nodes[node_off];
 
 	/*
 	 * Within the loop, the node_off'th array entry is a "hole" that
@@ -232,7 +247,7 @@ sift_up(binaryheap *heap, int node_off)
 	{
 		int			cmp;
 		int			parent_off;
-		Datum		parent_val;
+		bh_elem_type parent_val;
 
 		/*
 		 * If this node is smaller than its parent, the heap condition is
@@ -264,7 +279,7 @@ sift_up(binaryheap *heap, int node_off)
 static void
 sift_down(binaryheap *heap, int node_off)
 {
-	Datum		node_val = heap->bh_nodes[node_off];
+	bh_elem_type node_val = heap->bh_nodes[node_off];
 
 	/*
 	 * Within the loop, the node_off'th array entry is a "hole" that
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
diff --git a/src/include/lib/binaryheap.h b/src/include/lib/binaryheap.h
index 52f7b06b25..ae0af68f0d 100644
--- a/src/include/lib/binaryheap.h
+++ b/src/include/lib/binaryheap.h
@@ -11,11 +11,20 @@
 #ifndef BINARYHEAP_H
 #define BINARYHEAP_H
 
+/*
+ * XXX: This needs a big comment.
+ */
+#ifdef FRONTEND
+#define bh_elem_type void *
+#else
+#define bh_elem_type Datum
+#endif
+
 /*
  * For a max-heap, the comparator must return <0 iff a < b, 0 iff a == b,
  * and >0 iff a > b.  For a min-heap, the conditions are reversed.
  */
-typedef int (*binaryheap_comparator) (Datum a, Datum b, void *arg);
+typedef int (*binaryheap_comparator) (bh_elem_type a, bh_elem_type b, void *arg);
 
 /*
  * binaryheap
@@ -34,7 +43,7 @@ typedef struct binaryheap
 	bool		bh_has_heap_property;	/* debugging cross-check */
 	binaryheap_comparator bh_compare;
 	void	   *bh_arg;
-	Datum		bh_nodes[FLEXIBLE_ARRAY_MEMBER];
+	bh_elem_type bh_nodes[FLEXIBLE_ARRAY_MEMBER];
 } binaryheap;
 
 extern binaryheap *binaryheap_allocate(int capacity,
@@ -42,12 +51,12 @@ extern binaryheap *binaryheap_allocate(int capacity,
 									   void *arg);
 extern void binaryheap_reset(binaryheap *heap);
 extern void binaryheap_free(binaryheap *heap);
-extern void binaryheap_add_unordered(binaryheap *heap, Datum d);
+extern void binaryheap_add_unordered(binaryheap *heap, bh_elem_type d);
 extern void binaryheap_build(binaryheap *heap);
-extern void binaryheap_add(binaryheap *heap, Datum d);
-extern Datum binaryheap_first(binaryheap *heap);
-extern Datum binaryheap_remove_first(binaryheap *heap);
-extern void binaryheap_replace_first(binaryheap *heap, Datum d);
+extern void binaryheap_add(binaryheap *heap, bh_elem_type d);
+extern bh_elem_type binaryheap_first(binaryheap *heap);
+extern bh_elem_type binaryheap_remove_first(binaryheap *heap);
+extern void binaryheap_replace_first(binaryheap *heap, bh_elem_type d);
 
 #define binaryheap_empty(h)			((h)->bh_size == 0)
 
-- 
2.25.1


--3MwIy2ne0vdjdPXF
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0002-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* [PATCH v4 2/4] make binaryheap available to frontend
@ 2023-07-22 22:04 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-22 22:04 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 17 ++++++++++++++++-
 src/common/meson.build                   |  1 +
 5 files changed, 18 insertions(+), 3 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (96%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 96%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index c7fcfc550b..400a730c85 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -6,15 +6,22 @@
  * Portions Copyright (c) 2012-2023, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  src/backend/lib/binaryheap.c
+ *	  src/common/binaryheap.c
  *
  *-------------------------------------------------------------------------
  */
 
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
 #include "postgres.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -109,7 +116,11 @@ void
 binaryheap_add_unordered(binaryheap *heap, void *d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -141,7 +152,11 @@ void
 binaryheap_add(binaryheap *heap, void *d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
-- 
2.25.1


--qMm9M+Fa2AknHoGS
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0003-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* [PATCH v3 2/4] make binaryheap available to frontend
@ 2023-07-22 22:04 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2023-07-22 22:04 UTC (permalink / raw)

---
 src/backend/lib/Makefile                 |  1 -
 src/backend/lib/meson.build              |  1 -
 src/common/Makefile                      |  1 +
 src/{backend/lib => common}/binaryheap.c | 17 ++++++++++++++++-
 src/common/meson.build                   |  1 +
 5 files changed, 18 insertions(+), 3 deletions(-)
 rename src/{backend/lib => common}/binaryheap.c (96%)

diff --git a/src/backend/lib/Makefile b/src/backend/lib/Makefile
index 9dad31398a..b6cefd9cca 100644
--- a/src/backend/lib/Makefile
+++ b/src/backend/lib/Makefile
@@ -13,7 +13,6 @@ top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
 OBJS = \
-	binaryheap.o \
 	bipartite_match.o \
 	bloomfilter.o \
 	dshash.o \
diff --git a/src/backend/lib/meson.build b/src/backend/lib/meson.build
index 974cab8776..b4e88f54ae 100644
--- a/src/backend/lib/meson.build
+++ b/src/backend/lib/meson.build
@@ -1,7 +1,6 @@
 # Copyright (c) 2022-2023, PostgreSQL Global Development Group
 
 backend_sources += files(
-  'binaryheap.c',
   'bipartite_match.c',
   'bloomfilter.c',
   'dshash.c',
diff --git a/src/common/Makefile b/src/common/Makefile
index 113029bf7b..cc5c54dcee 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -48,6 +48,7 @@ LIBS += $(PTHREAD_LIBS)
 OBJS_COMMON = \
 	archive.o \
 	base64.o \
+	binaryheap.o \
 	checksum_helper.o \
 	compression.o \
 	config_info.o \
diff --git a/src/backend/lib/binaryheap.c b/src/common/binaryheap.c
similarity index 96%
rename from src/backend/lib/binaryheap.c
rename to src/common/binaryheap.c
index 1737546757..f21838b946 100644
--- a/src/backend/lib/binaryheap.c
+++ b/src/common/binaryheap.c
@@ -6,15 +6,22 @@
  * Portions Copyright (c) 2012-2023, PostgreSQL Global Development Group
  *
  * IDENTIFICATION
- *	  src/backend/lib/binaryheap.c
+ *	  src/common/binaryheap.c
  *
  *-------------------------------------------------------------------------
  */
 
+#ifdef FRONTEND
+#include "postgres_fe.h"
+#else
 #include "postgres.h"
+#endif
 
 #include <math.h>
 
+#ifdef FRONTEND
+#include "common/logging.h"
+#endif
 #include "lib/binaryheap.h"
 
 static void sift_down(binaryheap *heap, int node_off);
@@ -109,7 +116,11 @@ void
 binaryheap_add_unordered(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_has_heap_property = false;
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
@@ -141,7 +152,11 @@ void
 binaryheap_add(binaryheap *heap, Datum d)
 {
 	if (heap->bh_size >= heap->bh_space)
+#ifdef FRONTEND
+		pg_fatal("out of binary heap slots");
+#else
 		elog(ERROR, "out of binary heap slots");
+#endif
 	heap->bh_nodes[heap->bh_size] = d;
 	heap->bh_size++;
 	sift_up(heap, heap->bh_size - 1);
diff --git a/src/common/meson.build b/src/common/meson.build
index 53942a9a61..3b97497d1a 100644
--- a/src/common/meson.build
+++ b/src/common/meson.build
@@ -3,6 +3,7 @@
 common_sources = files(
   'archive.c',
   'base64.c',
+  'binaryheap.c',
   'checksum_helper.c',
   'compression.c',
   'controldata_utils.c',
-- 
2.25.1


--sdtB3X0nJg68CQEu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0003-expand-binaryheap-api.patch"



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
@ 2025-01-06 21:50 Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Nathan Bossart @ 2025-01-06 21:50 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Jan 06, 2025 at 04:29:37PM -0500, Tom Lane wrote:
> Unsurprisingly, this has completely broken buildfarm member sawshark:
> you added 13 new semaphores to the system's default requirements,
> and we only had headroom for about 4 (cf. 38da05346).

Oh wow, I missed that the defaults were so low on some systems.

> Now maybe we should just abandon the notion that we ought to be
> able to start up under OpenBSD/NetBSD's tiny default value of SEMMNS.
> If so I'd be inclined to go revert 38da05346, at least in HEAD.
> But I kind of wonder whether this feature actually brings value
> commensurate with causing installation problems on real-world OSes.

I'm obviously biased, but I think it would be unfortunate to block features
like this one because of low settings that would otherwise be unsuitable
for any reasonable production workload.  If we do want to at least support
check-world on these systems, another option could be to simply lower the
default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
only helps until the next time more semaphores are required, but that's not
a new problem.

-- 
nathan






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-01-06 22:15 ` Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Nathan Bossart @ 2025-01-06 22:15 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Jan 06, 2025 at 03:50:24PM -0600, Nathan Bossart wrote:
> I'm obviously biased, but I think it would be unfortunate to block features
> like this one because of low settings that would otherwise be unsuitable
> for any reasonable production workload.  If we do want to at least support
> check-world on these systems, another option could be to simply lower the
> default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
> only helps until the next time more semaphores are required, but that's not
> a new problem.

I've attached a patch to lower the default to 5.  That at least gives a
little bit of wiggle room for autovacuum_max_workers (and for a couple of
new auxiliary processes).  FWIW the reason I originally set the default to
16 was to prevent most users from ever needing to think about adjusting
autovacuum_worker_slots (which requires a restart and is a completely new
parameter that most will be unfamiliar with).

-- 
nathan

From 3f3c783c025e2a931bb766dc6d13c7186c957ebc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 6 Jan 2025 16:09:49 -0600
Subject: [PATCH v1 1/1] lower default of autovacuum_worker_slots to 5

---
 doc/src/sgml/config.sgml                      | 2 +-
 src/backend/utils/misc/guc_tables.c           | 2 +-
 src/backend/utils/misc/postgresql.conf.sample | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..4fda5e6ae79 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8639,7 +8639,7 @@ 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
+        processes.  The default is 5.  This parameter can only be set at server
         start.
        </para>
        <para>
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c9d8cd796a8..c75359b6bc4 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,
+		5, 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..c7d3852c040 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 = 5		# autovacuum worker slots to allocate
 					# (change requires restart)
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 #autovacuum_naptime = 1min		# time between autovacuum runs
-- 
2.39.5 (Apple Git-154)



Attachments:

  [text/plain] v1-0001-lower-default-of-autovacuum_worker_slots-to-5.patch (2.1K, ../../Z3xV_b3wp-XQR3Rm@nathan/2-v1-0001-lower-default-of-autovacuum_worker_slots-to-5.patch)
  download | inline diff:
From 3f3c783c025e2a931bb766dc6d13c7186c957ebc Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 6 Jan 2025 16:09:49 -0600
Subject: [PATCH v1 1/1] lower default of autovacuum_worker_slots to 5

---
 doc/src/sgml/config.sgml                      | 2 +-
 src/backend/utils/misc/guc_tables.c           | 2 +-
 src/backend/utils/misc/postgresql.conf.sample | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..4fda5e6ae79 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8639,7 +8639,7 @@ 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
+        processes.  The default is 5.  This parameter can only be set at server
         start.
        </para>
        <para>
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c9d8cd796a8..c75359b6bc4 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,
+		5, 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..c7d3852c040 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 = 5		# autovacuum worker slots to allocate
 					# (change requires restart)
 #autovacuum_max_workers = 3		# max number of autovacuum subprocesses
 #autovacuum_naptime = 1min		# time between autovacuum runs
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-01-06 22:17   ` Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Andres Freund @ 2025-01-06 22:17 UTC (permalink / raw)
  To: [email protected]; Nathan Bossart <[email protected]>; Tom Lane <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Hi, 

On January 6, 2025 5:15:25 PM EST, Nathan Bossart <[email protected]> wrote:
>On Mon, Jan 06, 2025 at 03:50:24PM -0600, Nathan Bossart wrote:
>> I'm obviously biased, but I think it would be unfortunate to block features
>> like this one because of low settings that would otherwise be unsuitable
>> for any reasonable production workload.  If we do want to at least support
>> check-world on these systems, another option could be to simply lower the
>> default of autovacuum_worker_slots to 7 (or maybe lower).  Of course, that
>> only helps until the next time more semaphores are required, but that's not
>> a new problem.
>
>I've attached a patch to lower the default to 5.  That at least gives a
>little bit of wiggle room for autovacuum_max_workers (and for a couple of
>new auxiliary processes).  FWIW the reason I originally set the default to
>16 was to prevent most users from ever needing to think about adjusting
>autovacuum_worker_slots (which requires a restart and is a completely new
>parameter that most will be unfamiliar with).

How about trying the higher setting first in initdb? On any sane system that won't cost anything because it'll succeed with the higher value.

Greetings, 

Andres
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
@ 2025-01-06 22:36     ` Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2025-01-06 22:36 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: [email protected]; Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Andres Freund <[email protected]> writes:
> How about trying the higher setting first in initdb? On any sane system that won't cost anything because it'll succeed with the higher value.

That might be a good compromise.  You'd have to think about how
it should interact with initdb's probes for workable values of
max_connections.  My first thought about that is to have initdb
set autovacuum_worker_slots to max_connections / 8 or thereabouts
as it works down the list of max_connections values to try.  Or
you could do something more complicated, but I don't see a reason
to make it too complex.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
@ 2025-01-06 23:20       ` Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Nathan Bossart @ 2025-01-06 23:20 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Jan 06, 2025 at 05:36:17PM -0500, Tom Lane wrote:
> Andres Freund <[email protected]> writes:
>> How about trying the higher setting first in initdb? On any sane system
>> that won't cost anything because it'll succeed with the higher value.
> 
> That might be a good compromise.

+1, I like the idea.

> You'd have to think about how
> it should interact with initdb's probes for workable values of
> max_connections.  My first thought about that is to have initdb
> set autovacuum_worker_slots to max_connections / 8 or thereabouts
> as it works down the list of max_connections values to try.  Or
> you could do something more complicated, but I don't see a reason
> to make it too complex.

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.

-- 
nathan

From e35b4c5425cbe1120d3c38619be4f1523ae1f2c9 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Mon, 6 Jan 2025 17:14:54 -0600
Subject: [PATCH v1 1/1] Lower default value of autovacuum_worker_slots in
 initdb as needed.

---
 doc/src/sgml/config.sgml |  5 +++--
 src/bin/initdb/initdb.c  | 41 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..8683f0bdf53 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 16 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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..1214f1b492a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -197,6 +197,7 @@ static char *pgdata_native;
 /* defaults */
 static int	n_connections = 10;
 static int	n_buffers = 50;
+static int	n_av_slots = 16;
 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_buffs,
+										  int test_av_slots);
 static void setup_config(void);
 static void bootstrap_template1(void);
 static void setup_auth(FILE *cmdfd);
@@ -1118,6 +1120,13 @@ test_config_settings(void)
 	 */
 #define MIN_BUFS_FOR_CONNS(nconns)	((nconns) * 10)
 
+	/*
+	 * This macro defines the minimum default autovacuum_worker_slots we are
+	 * willing to consider.  It should be kept >= the default for
+	 * autovacuum_max_workers.
+	 */
+#define MIN_AV_WORKER_SLOTS	(3)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1155,7 +1164,9 @@ test_config_settings(void)
 		test_conns = trial_conns[i];
 		test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
 
-		if (test_specific_config_settings(test_conns, test_buffs))
+		if (test_specific_config_settings(test_conns,
+										  test_buffs,
+										  MIN_AV_WORKER_SLOTS))
 		{
 			ok_buffers = test_buffs;
 			break;
@@ -1180,7 +1191,9 @@ test_config_settings(void)
 			break;
 		}
 
-		if (test_specific_config_settings(n_connections, test_buffs))
+		if (test_specific_config_settings(n_connections,
+										  test_buffs,
+										  MIN_AV_WORKER_SLOTS))
 			break;
 	}
 	n_buffers = test_buffs;
@@ -1190,6 +1203,19 @@ test_config_settings(void)
 	else
 		printf("%dkB\n", n_buffers * (BLCKSZ / 1024));
 
+	printf(_("selecting default \"autovacuum_worker_slots\" ... "));
+	fflush(stdout);
+
+	for (; n_av_slots > MIN_AV_WORKER_SLOTS; n_av_slots--)
+	{
+		if (test_specific_config_settings(n_connections,
+										  n_buffers,
+										  n_av_slots))
+			break;
+	}
+
+	printf("%d\n", n_av_slots);
+
 	printf(_("selecting default time zone ... "));
 	fflush(stdout);
 	default_timezone = select_default_timezone(share_path);
@@ -1200,7 +1226,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_buffs, int test_av_slots)
 {
 	PQExpBufferData cmd;
 	_stringlist *gnames,
@@ -1214,9 +1240,10 @@ test_specific_config_settings(int test_conns, int test_buffs)
 					  "\"%s\" --check %s %s "
 					  "-c max_connections=%d "
 					  "-c shared_buffers=%d "
+					  "-c autovacuum_worker_slots=%d "
 					  "-c dynamic_shared_memory_type=%s",
 					  backend_exec, boot_options, extra_options,
-					  test_conns, test_buffs,
+					  test_conns, test_buffs, test_av_slots,
 					  dynamic_shared_memory_type);
 
 	/* Add any user-given setting overrides */
@@ -1289,6 +1316,10 @@ setup_config(void)
 	conflines = replace_guc_value(conflines, "shared_buffers",
 								  repltok, false);
 
+	snprintf(repltok, sizeof(repltok), "%d", n_av_slots);
+	conflines = replace_guc_value(conflines, "autovacuum_worker_slots",
+								  repltok, false);
+
 	conflines = replace_guc_value(conflines, "lc_messages",
 								  lc_messages, false);
 
-- 
2.39.5 (Apple Git-154)



Attachments:

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

---
 doc/src/sgml/config.sgml |  5 +++--
 src/bin/initdb/initdb.c  | 41 +++++++++++++++++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..8683f0bdf53 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 16 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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..1214f1b492a 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -197,6 +197,7 @@ static char *pgdata_native;
 /* defaults */
 static int	n_connections = 10;
 static int	n_buffers = 50;
+static int	n_av_slots = 16;
 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_buffs,
+										  int test_av_slots);
 static void setup_config(void);
 static void bootstrap_template1(void);
 static void setup_auth(FILE *cmdfd);
@@ -1118,6 +1120,13 @@ test_config_settings(void)
 	 */
 #define MIN_BUFS_FOR_CONNS(nconns)	((nconns) * 10)
 
+	/*
+	 * This macro defines the minimum default autovacuum_worker_slots we are
+	 * willing to consider.  It should be kept >= the default for
+	 * autovacuum_max_workers.
+	 */
+#define MIN_AV_WORKER_SLOTS	(3)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1155,7 +1164,9 @@ test_config_settings(void)
 		test_conns = trial_conns[i];
 		test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
 
-		if (test_specific_config_settings(test_conns, test_buffs))
+		if (test_specific_config_settings(test_conns,
+										  test_buffs,
+										  MIN_AV_WORKER_SLOTS))
 		{
 			ok_buffers = test_buffs;
 			break;
@@ -1180,7 +1191,9 @@ test_config_settings(void)
 			break;
 		}
 
-		if (test_specific_config_settings(n_connections, test_buffs))
+		if (test_specific_config_settings(n_connections,
+										  test_buffs,
+										  MIN_AV_WORKER_SLOTS))
 			break;
 	}
 	n_buffers = test_buffs;
@@ -1190,6 +1203,19 @@ test_config_settings(void)
 	else
 		printf("%dkB\n", n_buffers * (BLCKSZ / 1024));
 
+	printf(_("selecting default \"autovacuum_worker_slots\" ... "));
+	fflush(stdout);
+
+	for (; n_av_slots > MIN_AV_WORKER_SLOTS; n_av_slots--)
+	{
+		if (test_specific_config_settings(n_connections,
+										  n_buffers,
+										  n_av_slots))
+			break;
+	}
+
+	printf("%d\n", n_av_slots);
+
 	printf(_("selecting default time zone ... "));
 	fflush(stdout);
 	default_timezone = select_default_timezone(share_path);
@@ -1200,7 +1226,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_buffs, int test_av_slots)
 {
 	PQExpBufferData cmd;
 	_stringlist *gnames,
@@ -1214,9 +1240,10 @@ test_specific_config_settings(int test_conns, int test_buffs)
 					  "\"%s\" --check %s %s "
 					  "-c max_connections=%d "
 					  "-c shared_buffers=%d "
+					  "-c autovacuum_worker_slots=%d "
 					  "-c dynamic_shared_memory_type=%s",
 					  backend_exec, boot_options, extra_options,
-					  test_conns, test_buffs,
+					  test_conns, test_buffs, test_av_slots,
 					  dynamic_shared_memory_type);
 
 	/* Add any user-given setting overrides */
@@ -1289,6 +1316,10 @@ setup_config(void)
 	conflines = replace_guc_value(conflines, "shared_buffers",
 								  repltok, false);
 
+	snprintf(repltok, sizeof(repltok), "%d", n_av_slots);
+	conflines = replace_guc_value(conflines, "autovacuum_worker_slots",
+								  repltok, false);
+
 	conflines = replace_guc_value(conflines, "lc_messages",
 								  lc_messages, false);
 
-- 
2.39.5 (Apple Git-154)



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-01-06 23:36         ` Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2025-01-06 23:36 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Nathan Bossart <[email protected]> writes:
> On Mon, Jan 06, 2025 at 05:36:17PM -0500, Tom Lane wrote:
>> You'd have to think about how
>> it should interact with initdb's probes for workable values of
>> max_connections.  My first thought about that is to have initdb
>> set autovacuum_worker_slots to max_connections / 8 or thereabouts
>> as it works down the list of max_connections values to try.  Or
>> you could do something more complicated, but I don't see a reason
>> to make it too complex.

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

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
@ 2025-01-07 02:44           ` Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Nathan Bossart @ 2025-01-07 02:44 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Jan 06, 2025 at 06:36:43PM -0500, Tom Lane wrote:
> Nathan Bossart <[email protected]> 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 <[email protected]>
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.2K, ../../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 <[email protected]>
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)



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-01-07 03:29             ` Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2025-01-07 03:29 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Nathan Bossart <[email protected]> writes:
> On Mon, Jan 06, 2025 at 06:36:43PM -0500, Tom Lane wrote:
>> 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.

+1 for simplicity ... but on reflection, what do you think about
using max_connections / 6?  That would keep autovacuum_worker_slots
at 100 / 6 = 16 for the vast majority of systems.  For the worst case
*BSD machines, we'd select 25 / 6 = 4 which results in consuming one
more semaphore than where we were yesterday.  I'm willing to accept
that outcome though, since we still have 3 or so to spare.

Other than the specific magic number, your patch LGTM.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
@ 2025-01-07 17:23               ` Nathan Bossart <[email protected]>
  2025-01-07 19:22                 ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-04-28 11:55                 ` Re: allow changing autovacuum_max_workers without restarting Peter Eisentraut <[email protected]>
  0 siblings, 2 replies; 20+ messages in thread

From: Nathan Bossart @ 2025-01-07 17:23 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Jan 06, 2025 at 10:29:07PM -0500, Tom Lane wrote:
> +1 for simplicity ... but on reflection, what do you think about
> using max_connections / 6?  That would keep autovacuum_worker_slots
> at 100 / 6 = 16 for the vast majority of systems.  For the worst case
> *BSD machines, we'd select 25 / 6 = 4 which results in consuming one
> more semaphore than where we were yesterday.  I'm willing to accept
> that outcome though, since we still have 3 or so to spare.

WFM.  I'm kicking myself for not having thought of that...

> Other than the specific magic number, your patch LGTM.

Here's a new version of the patch with some small cosmetic changes
(including more commentary about the formula) and the constant changed to
6.  I'll go commit this shortly.

-- 
nathan

From d8b9fe6d3c166c14d9a8d17f43418be33c4e0784 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 7 Jan 2025 11:15:22 -0600
Subject: [PATCH v3 1/1] Lower default value of autovacuum_worker_slots in
 initdb as needed.

TODO

Reported-by: Tom Lane
Suggested-by: Andres Freund
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1346002.1736198977%40sss.pgh.pa.us
---
 doc/src/sgml/config.sgml |  5 +++--
 src/bin/initdb/initdb.c  | 40 ++++++++++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..8683f0bdf53 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 16 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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..f2b9d50e9b3 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 = 16;
 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,18 @@ 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.  Note that it has been carefully
+	 * crafted to provide specific values for the associated values in
+	 * trial_conns.  We want it to return autovacuum_worker_slot's initial
+	 * default value (16) for the maximum value in trial_conns (100), and we
+	 * want it to return close to the minimum value we'd consider (3, which is
+	 * the default of autovacuum_max_workers) for the minimum value in
+	 * trial_conns (25).
+	 */
+#define AV_SLOTS_FOR_CONNS(nconns)	((nconns) / 6)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1145,7 +1159,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);
@@ -1153,9 +1168,10 @@ test_config_settings(void)
 	for (i = 0; i < connslen; i++)
 	{
 		test_conns = trial_conns[i];
+		n_av_slots = AV_SLOTS_FOR_CONNS(test_conns);
 		test_buffs = MIN_BUFS_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 +1183,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 +1203,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 +1223,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 +1236,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 +1304,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] v3-0001-Lower-default-value-of-autovacuum_worker_slots-in.patch (5.6K, ../../Z31jI10tLLd6thfu@nathan/2-v3-0001-Lower-default-value-of-autovacuum_worker_slots-in.patch)
  download | inline diff:
From d8b9fe6d3c166c14d9a8d17f43418be33c4e0784 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Tue, 7 Jan 2025 11:15:22 -0600
Subject: [PATCH v3 1/1] Lower default value of autovacuum_worker_slots in
 initdb as needed.

TODO

Reported-by: Tom Lane
Suggested-by: Andres Freund
Reviewed-by: Tom Lane
Discussion: https://postgr.es/m/1346002.1736198977%40sss.pgh.pa.us
---
 doc/src/sgml/config.sgml |  5 +++--
 src/bin/initdb/initdb.c  | 40 ++++++++++++++++++++++++++++++++++------
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 740ff5d5044..8683f0bdf53 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 16 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/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 4e4b7ede190..f2b9d50e9b3 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 = 16;
 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,18 @@ 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.  Note that it has been carefully
+	 * crafted to provide specific values for the associated values in
+	 * trial_conns.  We want it to return autovacuum_worker_slot's initial
+	 * default value (16) for the maximum value in trial_conns (100), and we
+	 * want it to return close to the minimum value we'd consider (3, which is
+	 * the default of autovacuum_max_workers) for the minimum value in
+	 * trial_conns (25).
+	 */
+#define AV_SLOTS_FOR_CONNS(nconns)	((nconns) / 6)
+
 	static const int trial_conns[] = {
 		100, 50, 40, 30, 25
 	};
@@ -1145,7 +1159,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);
@@ -1153,9 +1168,10 @@ test_config_settings(void)
 	for (i = 0; i < connslen; i++)
 	{
 		test_conns = trial_conns[i];
+		n_av_slots = AV_SLOTS_FOR_CONNS(test_conns);
 		test_buffs = MIN_BUFS_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 +1183,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 +1203,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 +1223,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 +1236,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 +1304,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)



^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-01-07 19:22                 ` Tom Lane <[email protected]>
  2025-01-07 20:45                   ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  1 sibling, 1 reply; 20+ messages in thread

From: Tom Lane @ 2025-01-07 19:22 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Nathan Bossart <[email protected]> writes:
> Here's a new version of the patch with some small cosmetic changes
> (including more commentary about the formula) and the constant changed to
> 6.  I'll go commit this shortly.

This one WFM.  Thanks!

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 19:22                 ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
@ 2025-01-07 20:45                   ` Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2025-01-07 20:45 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Tue, Jan 07, 2025 at 02:22:42PM -0500, Tom Lane wrote:
> This one WFM.  Thanks!

Committed, thanks for the report/review.

-- 
nathan






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
@ 2025-04-28 11:55                 ` Peter Eisentraut <[email protected]>
  2025-04-28 13:14                   ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  1 sibling, 1 reply; 20+ messages in thread

From: Peter Eisentraut @ 2025-04-28 11:55 UTC (permalink / raw)
  To: Nathan Bossart <[email protected]>; Tom Lane <[email protected]>; +Cc: Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On 07.01.25 18:23, Nathan Bossart wrote:
> +	/*
> +	 * 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);
> +

This initdb output seems, well, kinda fake, which it is by its own
admission.  Could we do this less fake maybe like this:

     selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16

with the actual wait at the "..."?

(It doesn't seem impossible that someone will want to add more default
selecting for various worker or process slots, and this would allow adding
these easily, versus adding more "fake" output lines.)





^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-04-28 11:55                 ` Re: allow changing autovacuum_max_workers without restarting Peter Eisentraut <[email protected]>
@ 2025-04-28 13:14                   ` Tom Lane <[email protected]>
  2025-04-28 14:41                     ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  0 siblings, 1 reply; 20+ messages in thread

From: Tom Lane @ 2025-04-28 13:14 UTC (permalink / raw)
  To: Peter Eisentraut <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

Peter Eisentraut <[email protected]> writes:
> This initdb output seems, well, kinda fake, which it is by its own
> admission.

Agreed.

> Could we do this less fake maybe like this:
>      selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16
> with the actual wait at the "..."?

Perhaps that would be all right ...

> (It doesn't seem impossible that someone will want to add more default
> selecting for various worker or process slots, and this would allow adding
> these easily, versus adding more "fake" output lines.)

... but I can't see this approach scaling to three or four or five
outputs.  The line would get unreasonably long.

My own proposal given the way it works now is to just print
max_connections and not mention autovacuum_worker_slots at all.
Our choice for max_connections is worth reporting, but I don't
feel that everything derived from it needs to be reported.

			regards, tom lane






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

* Re: allow changing autovacuum_max_workers without restarting
  2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
  2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
  2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
  2025-04-28 11:55                 ` Re: allow changing autovacuum_max_workers without restarting Peter Eisentraut <[email protected]>
  2025-04-28 13:14                   ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
@ 2025-04-28 14:41                     ` Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 20+ messages in thread

From: Nathan Bossart @ 2025-04-28 14:41 UTC (permalink / raw)
  To: Tom Lane <[email protected]>; +Cc: Peter Eisentraut <[email protected]>; Andres Freund <[email protected]>; [email protected]; Imseih (AWS), Sami <[email protected]>; Justin Pryzby <[email protected]>; pgsql-hackers

On Mon, Apr 28, 2025 at 09:14:54AM -0400, Tom Lane wrote:
> Peter Eisentraut <[email protected]> writes:
>> This initdb output seems, well, kinda fake, which it is by its own
>> admission.
> 
> Agreed.
> 
>> Could we do this less fake maybe like this:
>>      selecting default "max_connections", "autovacuum_worker_slots" ... 100, 16
>> with the actual wait at the "..."?
> 
> Perhaps that would be all right ...
> 
>> (It doesn't seem impossible that someone will want to add more default
>> selecting for various worker or process slots, and this would allow adding
>> these easily, versus adding more "fake" output lines.)
> 
> ... but I can't see this approach scaling to three or four or five
> outputs.  The line would get unreasonably long.
> 
> My own proposal given the way it works now is to just print
> max_connections and not mention autovacuum_worker_slots at all.
> Our choice for max_connections is worth reporting, but I don't
> feel that everything derived from it needs to be reported.

I'm fine with either of these ideas.  If I had to choose one, I'd just
remove the autovacuum_worker_slots report for the reasons Tom noted.

However, weren't we considering reverting some of this stuff [0]?  I see
that sawshark is now choosing max_connections = 40 and
autovacuum_worker_slots = 6, and since there are no other apparent related
buildfarm failures, I'm assuming that nobody else is testing the 60
semaphores case anymore.

[0] https://postgr.es/m/618497.1742347456%40sss.pgh.pa.us

-- 
nathan






^ permalink  raw  reply  [nested|flat] 20+ messages in thread

end of thread, other threads:[~2025-04-28 14:41 UTC | newest]

Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-07-20 16:45 [PATCH v2 2/4] make binaryheap available to frontend Nathan Bossart <[email protected]>
2023-07-20 16:45 [PATCH v2 2/4] make binaryheap available to frontend Nathan Bossart <[email protected]>
2023-07-22 22:04 [PATCH v3 2/4] make binaryheap available to frontend Nathan Bossart <[email protected]>
2023-07-22 22:04 [PATCH v5 1/3] make binaryheap available to frontend Nathan Bossart <[email protected]>
2023-07-22 22:04 [PATCH v4 2/4] make binaryheap available to frontend Nathan Bossart <[email protected]>
2023-07-22 22:04 [PATCH v3 2/4] make binaryheap available to frontend Nathan Bossart <[email protected]>
2025-01-06 21:50 Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-01-06 22:15 ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-01-06 22:17   ` Re: allow changing autovacuum_max_workers without restarting Andres Freund <[email protected]>
2025-01-06 22:36     ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
2025-01-06 23:20       ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-01-06 23:36         ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
2025-01-07 02:44           ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-01-07 03:29             ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
2025-01-07 17:23               ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-01-07 19:22                 ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
2025-01-07 20:45                   ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>
2025-04-28 11:55                 ` Re: allow changing autovacuum_max_workers without restarting Peter Eisentraut <[email protected]>
2025-04-28 13:14                   ` Re: allow changing autovacuum_max_workers without restarting Tom Lane <[email protected]>
2025-04-28 14:41                     ` Re: allow changing autovacuum_max_workers without restarting Nathan Bossart <[email protected]>

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