On Sat, Jan 15, 2011 at 11:52 AM, Greg Smith <g...@2ndquadrant.com> wrote:
> Where I was expecting that setting to be "4" instead for 32kB.  So there's
> probably some minor bug left in where I inserted this into the
> initialization sequence.

So I started taking a look at this patch tonight with an eye to
committing it, but ended up having to whack it around fairly hard to
fix the problem described above: the problem is, I believe, that the
initialization code in question doesn't run in every backend.  My
first thought was "gee, it's a bad idea for us to be manipulating the
value of the GUC variable directly, I should use an assign_hook".
But of course that turns out to be a bad idea, because there's no
guarantee that wal_buffers will be initialized after shared_buffers.
So I put it back the way you had it, and jiggered things so that the
copy of the value that's actually used for memory allocation gets
stored in XLogCtl.  That made it possible to define a show_hook that
DTRT, but that wasn't entirely straightforward either: the show_hook
has to deliver the finished string, not just a replacement integer for
guc.c to format.  So I exposed the relevant formatting logic from
guc.c as a separate function (duplicating that code didn't seem
smart), and now it all seems to work.  See attached, which also
includes some other rewriting of code and docs that I hope constitute
improvements.

Barring screams of agony^W^W^Whelpful suggestions for how to code this
more neatly, I'll go ahead and commit this.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8e2a2c5..bb67131 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1638,12 +1638,24 @@ SET ENABLE_SEQSCAN TO OFF;
       </indexterm>
       <listitem>
        <para>
-        The amount of memory used in shared memory for WAL data.  The
-        default is 64 kilobytes (<literal>64kB</>).  The setting need only
-        be large enough to hold the amount of WAL data generated by one
-        typical transaction, since the data is written out to disk at
-        every transaction commit.  This parameter can only be set at server
-        start.
+        The amount of shared memory used for storing WAL data.  The
+        default setting of -1 selects a size equal to 1/32 of
+        <varname>shared_buffers</varname> (about 3%), but not less than
+        <literal>64kB</literal> or more than than the size of one WAL segment,
+        typically <literal>16MB</literal>.  This value may be manually
+        changed if it is too large or too small, but any positive value
+        less than <literal>32kB</literal> will be treated as
+        <literal>32kB</literal>.
+       </para>
+
+       <para>
+        The contents of these buffers are written out to disk at every
+        transaction commit, so extremely large values are unlikely to
+        provide a significant benefit.  However, setting this value to at
+        least a few megabytes can improve write performance on a busy
+        server where many clients are committing at once.  The auto-tuning
+        selected by the default setting of -1 should give reasonable
+        results in most cases.
        </para>
 
        <para>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5b6a230..a5df3bb 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -69,7 +69,8 @@
 /* User-settable parameters */
 int			CheckPointSegments = 3;
 int			wal_keep_segments = 0;
-int			XLOGbuffers = 8;
+int			wal_buffers = -1;				/* raw GUC setting */
+int			XLOGbuffers = -1;				/* buffers actually allocated */
 int			XLogArchiveTimeout = 0;
 bool		XLogArchiveMode = false;
 char	   *XLogArchiveCommand = NULL;
@@ -381,6 +382,7 @@ typedef struct XLogCtlData
 	int			XLogCacheBlck;	/* highest allocated xlog buffer index */
 	TimeLineID	ThisTimeLineID;
 	TimeLineID	RecoveryTargetTLI;
+	int			wal_buffers;	/* so backends can see auto-tuned value */
 
 	/*
 	 * archiveCleanupCommand is read from recovery.conf but needs to be in
@@ -4778,6 +4780,47 @@ GetSystemIdentifier(void)
 }
 
 /*
+ * Auto-tune the number of XLOG buffers.
+ *
+ * When wal_buffers == -1, we auto-tune to about 3% of shared_buffers, with
+ * a maximum of one XLOG segment and a minimum of 8 blocks (the default value
+ * prior to PostgreSQL 9.1, when auto-tuning was added).  We also clamp the
+ * manually set minimum to four blocks (prior to PostgreSQL 9.1, a smaller
+ * value would have been out of range, but since the minimum is now -1 for
+ * auto-tuning, we just silently treat such values as a request for the
+ * minimum).
+ */
+static void
+XLOGTuneNumBuffers()
+{
+	XLOGbuffers = wal_buffers;
+
+	if (XLOGbuffers == -1)
+	{
+		XLOGbuffers = NBuffers / 32;
+		if (XLOGbuffers > XLOG_SEG_SIZE / XLOG_BLCKSZ)
+			XLOGbuffers = XLOG_SEG_SIZE / XLOG_BLCKSZ;
+		if (XLOGbuffers < 8)
+			XLOGbuffers = 8;
+	}
+	else if (XLOGbuffers < 4)
+		XLOGbuffers = 4;
+}
+
+/*
+ * Show the number of WAL buffers; we want this to display the auto-tuned value
+ * from shared memory.
+ */
+const char *
+show_wal_buffers(void)
+{
+	static char buf[GUC_OUTPUT_BUFSIZE];
+
+	format_integer_guc_value(buf, XLogCtl->wal_buffers, GUC_UNIT_XBLOCKS);
+	return buf;
+}
+
+/*
  * Initialization of shared memory for XLOG
  */
 Size
@@ -4785,6 +4828,10 @@ XLOGShmemSize(void)
 {
 	Size		size;
 
+	/* Figure out how many XLOG buffers we need. */
+	XLOGTuneNumBuffers();
+	Assert(XLOGbuffers > 0);
+
 	/* XLogCtl */
 	size = sizeof(XLogCtlData);
 	/* xlblocks array */
@@ -4848,6 +4895,7 @@ XLOGShmemInit(void)
 	XLogCtl->XLogCacheBlck = XLOGbuffers - 1;
 	XLogCtl->SharedRecoveryInProgress = true;
 	XLogCtl->Insert.currpage = (XLogPageHeader) (XLogCtl->pages);
+	XLogCtl->wal_buffers = XLOGbuffers;
 	SpinLockInit(&XLogCtl->info_lck);
 	InitSharedLatch(&XLogCtl->recoveryWakeupLatch);
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ffff601..2f58f76 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1764,8 +1764,8 @@ static struct config_int ConfigureNamesInt[] =
 			NULL,
 			GUC_UNIT_XBLOCKS
 		},
-		&XLOGbuffers,
-		8, 4, INT_MAX, NULL, NULL
+		&wal_buffers,
+		-1, -1, INT_MAX, NULL, show_wal_buffers
 	},
 
 	{
@@ -6743,7 +6743,7 @@ show_all_settings(PG_FUNCTION_ARGS)
 static char *
 _ShowOption(struct config_generic * record, bool use_units)
 {
-	char		buffer[256];
+	char		buffer[GUC_OUTPUT_BUFSIZE];
 	const char *val;
 
 	switch (record->vartype)
@@ -6767,84 +6767,8 @@ _ShowOption(struct config_generic * record, bool use_units)
 					val = (*conf->show_hook) ();
 				else
 				{
-					/*
-					 * Use int64 arithmetic to avoid overflows in units
-					 * conversion.
-					 */
-					int64		result = *conf->variable;
-					const char *unit;
-
-					if (use_units && result > 0 &&
-						(record->flags & GUC_UNIT_MEMORY))
-					{
-						switch (record->flags & GUC_UNIT_MEMORY)
-						{
-							case GUC_UNIT_BLOCKS:
-								result *= BLCKSZ / 1024;
-								break;
-							case GUC_UNIT_XBLOCKS:
-								result *= XLOG_BLCKSZ / 1024;
-								break;
-						}
-
-						if (result % KB_PER_GB == 0)
-						{
-							result /= KB_PER_GB;
-							unit = "GB";
-						}
-						else if (result % KB_PER_MB == 0)
-						{
-							result /= KB_PER_MB;
-							unit = "MB";
-						}
-						else
-						{
-							unit = "kB";
-						}
-					}
-					else if (use_units && result > 0 &&
-							 (record->flags & GUC_UNIT_TIME))
-					{
-						switch (record->flags & GUC_UNIT_TIME)
-						{
-							case GUC_UNIT_S:
-								result *= MS_PER_S;
-								break;
-							case GUC_UNIT_MIN:
-								result *= MS_PER_MIN;
-								break;
-						}
-
-						if (result % MS_PER_D == 0)
-						{
-							result /= MS_PER_D;
-							unit = "d";
-						}
-						else if (result % MS_PER_H == 0)
-						{
-							result /= MS_PER_H;
-							unit = "h";
-						}
-						else if (result % MS_PER_MIN == 0)
-						{
-							result /= MS_PER_MIN;
-							unit = "min";
-						}
-						else if (result % MS_PER_S == 0)
-						{
-							result /= MS_PER_S;
-							unit = "s";
-						}
-						else
-						{
-							unit = "ms";
-						}
-					}
-					else
-						unit = "";
-
-					snprintf(buffer, sizeof(buffer), INT64_FORMAT "%s",
-							 result, unit);
+					format_integer_guc_value(buffer, *conf->variable,
+											 use_units ? record->flags : 0);
 					val = buffer;
 				}
 			}
@@ -6898,6 +6822,84 @@ _ShowOption(struct config_generic * record, bool use_units)
 	return pstrdup(val);
 }
 
+/*
+ * Format an integer GUC value using appropriate units.
+ */
+void
+format_integer_guc_value(char *buffer, int val, int flags)
+{
+	int64		result = val;
+	const char *unit;
+
+	if (result > 0 && (flags & GUC_UNIT_MEMORY))
+	{
+		switch (flags & GUC_UNIT_MEMORY)
+		{
+			case GUC_UNIT_BLOCKS:
+				result *= BLCKSZ / 1024;
+				break;
+			case GUC_UNIT_XBLOCKS:
+				result *= XLOG_BLCKSZ / 1024;
+				break;
+		}
+
+		if (result % KB_PER_GB == 0)
+		{
+			result /= KB_PER_GB;
+			unit = "GB";
+		}
+		else if (result % KB_PER_MB == 0)
+		{
+			result /= KB_PER_MB;
+			unit = "MB";
+		}
+		else
+		{
+			unit = "kB";
+		}
+	}
+	else if (result > 0 && (flags & GUC_UNIT_TIME))
+	{
+		switch (flags & GUC_UNIT_TIME)
+		{
+			case GUC_UNIT_S:
+				result *= MS_PER_S;
+				break;
+			case GUC_UNIT_MIN:
+				result *= MS_PER_MIN;
+				break;
+		}
+
+		if (result % MS_PER_D == 0)
+		{
+				result /= MS_PER_D;
+				unit = "d";
+		}
+		else if (result % MS_PER_H == 0)
+		{
+				result /= MS_PER_H;
+				unit = "h";
+		}
+		else if (result % MS_PER_MIN == 0)
+		{
+				result /= MS_PER_MIN;
+				unit = "min";
+		}
+		else if (result % MS_PER_S == 0)
+		{
+				result /= MS_PER_S;
+				unit = "s";
+		}
+		else
+		{
+				unit = "ms";
+		}
+	}
+	else
+		unit = "";
+
+	snprintf(buffer, GUC_OUTPUT_BUFSIZE, INT64_FORMAT "%s", result, unit);
+}
 
 /*
  * Attempt (badly) to detect if a proposed new GUC setting is the same
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index f436b83..6c6f9a9 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -162,7 +162,7 @@
 					#   fsync_writethrough
 					#   open_sync
 #full_page_writes = on			# recover from partial page writes
-#wal_buffers = 64kB			# min 32kB
+#wal_buffers = -1			# min 32kB, -1 sets based on shared_buffers
 					# (change requires restart)
 #wal_writer_delay = 200ms		# 1-10000 milliseconds
 
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 74d3427..48eec74 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -193,6 +193,7 @@ extern XLogRecPtr XactLastRecEnd;
 extern int	CheckPointSegments;
 extern int	wal_keep_segments;
 extern int	XLOGbuffers;
+extern int	wal_buffers;
 extern int	XLogArchiveTimeout;
 extern bool XLogArchiveMode;
 extern char *XLogArchiveCommand;
@@ -307,6 +308,7 @@ extern XLogRecPtr GetInsertRecPtr(void);
 extern XLogRecPtr GetFlushRecPtr(void);
 extern void GetNextXidAndEpoch(TransactionId *xid, uint32 *epoch);
 extern TimeLineID GetRecoveryTargetTLI(void);
+extern const char *show_wal_buffers(void);
 
 extern void HandleStartupProcInterrupts(void);
 extern void StartupProcessMain(void);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index c07f263..1f9c62e 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -294,6 +294,9 @@ extern bool set_config_option(const char *name, const char *value,
 extern char *GetConfigOptionByName(const char *name, const char **varname);
 extern void GetConfigOptionByNum(int varnum, const char **values, bool *noshow);
 extern int	GetNumConfigOptions(void);
+extern void	format_integer_guc_value(char *buffer, int val, int flags);
+
+#define GUC_OUTPUT_BUFSIZE		256
 
 extern void SetPGVariable(const char *name, List *args, bool is_local);
 extern void GetPGVariable(const char *name, DestReceiver *dest);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to