So we've had several rounds of discussions about simplifying replication
configuration in general and the wal_level setting in particular. [0][1]
 Let's get something going.  While we have not reached a complete
consensus yet, a few things have become clear:

- We would like to have fewer or easier to change settings.

- It looks like some folks would prefer a switchover to a completely new
and better system, but my experience in these kinds of matters is that
it's better to take smaller steps or we won't get anything changed.

- The distinction between wal_level settings "archive" and "hot_standby"
is in the way of automation or better intelligence, because the primary
cannot tell what the receiver intends to do with the WAL.

So here is a patch to get rid of the distinction.

Bike-shedding:  In this patch, I removed "archive" and kept
"hot_standby", because that's what the previous discussions suggested.
Historically and semantically, it would be more correct the other way
around.  On the other hand, keeping "hot_standby" would probably require
fewer configuration files to be changed.  Or we could keep both, but
that would be confusing (for users and in the code).

I kept the distinction between XLogIsNeeded() and
XLogStandbyInfoActive(), because it is kind of nice for documentation
(although the names are terrible).

The changed comment in xlog_redo() could probably use some review or a
bit more detailed reasoning.

There were a couple of places that I felt were overly coupled with the
wal_level settings.  The XLogArchiving*() macros shouldn't really care
what wal_level is, because that is checked elsewhere.  I replaced that
with assertions.  The check in CheckSlotRequirements() seems
unnecessary.  Why can I not add and remove slots while wal_level is minimal?

The documentation and error messages could also use more overhaul.  Some
parts say things like "archive or higher", implying that the user knows
about the ordering, other parts list all the allowed options, possibly
implying that they are mutually exclusive variants.

Maybe some of these things could be split out into separate patches.


[0]:
http://www.postgresql.org/message-id/20150203124317.ga24...@awork2.anarazel.de
[1]: http://www.postgresql.org/message-id/55d31e0d.8060...@gmx.net
>From 612dee3ccb5b49f21c2195c5b9d13fc87d1f9d58 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Mon, 31 Aug 2015 22:36:08 -0400
Subject: [PATCH] Remove wal_level "archive", fold into "hot_standby"

This distinction exists only because at the time "hot_standby" was
added, there was some uncertainty about stability.  This is now a long
time ago.  We would like to move forward with simplifying the
replication configuration, but this distinction is in the way, because a
primary server cannot tell (without asking a standby or predicting the
future) which one of these would be the appropriate level.
---
 doc/src/sgml/backup.sgml                      |  2 +-
 doc/src/sgml/config.sgml                      | 19 ++++---------------
 src/backend/access/rmgrdesc/xlogdesc.c        |  1 -
 src/backend/access/transam/xlog.c             | 12 +++++-------
 src/backend/access/transam/xlogfuncs.c        |  2 +-
 src/backend/postmaster/postmaster.c           |  2 +-
 src/backend/replication/slot.c                |  5 -----
 src/backend/utils/misc/postgresql.conf.sample |  2 +-
 src/bin/pg_basebackup/t/010_pg_basebackup.pl  |  2 +-
 src/bin/pg_controldata/pg_controldata.c       |  2 --
 src/include/access/xlog.h                     |  7 +++----
 11 files changed, 17 insertions(+), 39 deletions(-)

diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 75dabe9..7f52232 100644
--- a/doc/src/sgml/backup.sgml
+++ b/doc/src/sgml/backup.sgml
@@ -1285,7 +1285,7 @@ <title>Standalone Hot Backups</title>
       If more flexibility in copying the backup files is needed, a lower
       level process can be used for standalone hot backups as well.
       To prepare for low level standalone hot backups, set <varname>wal_level</> to
-      <literal>archive</> or higher, <varname>archive_mode</> to
+      <literal>hot_standby</> or higher, <varname>archive_mode</> to
       <literal>on</>, and set up an <varname>archive_command</> that performs
       archiving only when a <emphasis>switch file</> exists.  For example:
 <programlisting>
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e3dc23b..ad30a9e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1952,8 +1952,8 @@ <title>Settings</title>
         <varname>wal_level</> determines how much information is written
         to the WAL. The default value is <literal>minimal</>, which writes
         only the information needed to recover from a crash or immediate
-        shutdown. <literal>archive</> adds logging required for WAL archiving;
-        <literal>hot_standby</> further adds information required to run
+        shutdown. <literal>hot_standby</> adds logging required for WAL
+        archiving as well as information required to run
         read-only queries on a standby server; and, finally
         <literal>logical</> adds information necessary to support logical
         decoding.  Each level includes the information logged at all lower
@@ -1972,22 +1972,11 @@ <title>Settings</title>
          transaction</member>
         </simplelist>
         But minimal WAL does not contain enough information to reconstruct the
-        data from a base backup and the WAL logs, so <literal>archive</> or
+        data from a base backup and the WAL logs, so <literal>hot_standby</> or
         higher must be used to enable WAL archiving
         (<xref linkend="guc-archive-mode">) and streaming replication.
        </para>
        <para>
-        In <literal>hot_standby</> level, the same information is logged as
-        with <literal>archive</>, plus information needed to reconstruct
-        the status of running transactions from the WAL. To enable read-only
-        queries on a standby server, <varname>wal_level</> must be set to
-        <literal>hot_standby</> or higher on the primary, and
-        <xref linkend="guc-hot-standby"> must be enabled in the standby. It is
-        thought that there is little measurable difference in performance
-        between using <literal>hot_standby</> and <literal>archive</> levels,
-        so feedback is welcome if any production impacts are noticeable.
-       </para>
-       <para>
         In <literal>logical</> level, the same information is logged as
         with <literal>hot_standby</>, plus information needed to allow
         extracting logical changesets from the WAL. Using a level of
@@ -2655,7 +2644,7 @@ <title>Sending Server(s)</title>
         higher than the maximum number of expected clients so disconnected
         clients can immediately reconnect.  This parameter can only
         be set at server start. <varname>wal_level</> must be set to
-        <literal>archive</> or higher to allow connections from standby
+        <literal>hot_standby</> or higher to allow connections from standby
         servers.
        </para>
        </listitem>
diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index 4f29136..035cefd 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -25,7 +25,6 @@
  */
 const struct config_enum_entry wal_level_options[] = {
 	{"minimal", WAL_LEVEL_MINIMAL, false},
-	{"archive", WAL_LEVEL_ARCHIVE, false},
 	{"hot_standby", WAL_LEVEL_HOT_STANDBY, false},
 	{"logical", WAL_LEVEL_LOGICAL, false},
 	{NULL, 0, false}
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 68e33eb..14e63fe 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5847,7 +5847,7 @@ static void
 CheckRequiredParameterValues(void)
 {
 	/*
-	 * For archive recovery, the WAL must be generated with at least 'archive'
+	 * For archive recovery, the WAL must be generated with at least 'hot_standby'
 	 * wal_level.
 	 */
 	if (ArchiveRecoveryRequested && ControlFile->wal_level == WAL_LEVEL_MINIMAL)
@@ -9480,10 +9480,8 @@ xlog_redo(XLogReaderState *record)
 		/*
 		 * Update minRecoveryPoint to ensure that if recovery is aborted, we
 		 * recover back up to this point before allowing hot standby again.
-		 * This is particularly important if wal_level was set to 'archive'
-		 * before, and is now 'hot_standby', to ensure you don't run queries
-		 * against the WAL preceding the wal_level change. Same applies to
-		 * decreasing max_* settings.
+		 * This is important if the max_* settings are decreased, to ensure
+		 * you don't run queries against the WAL preceding the change.
 		 */
 		minRecoveryPoint = ControlFile->minRecoveryPoint;
 		minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
@@ -9830,7 +9828,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p,
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 			  errmsg("WAL level not sufficient for making an online backup"),
-				 errhint("wal_level must be set to \"archive\", \"hot_standby\", or \"logical\" at server start.")));
+				 errhint("wal_level must be set to \"hot_standby\" or \"logical\" at server start.")));
 
 	if (strlen(backupidstr) > MAXPGPATH)
 		ereport(ERROR,
@@ -10301,7 +10299,7 @@ do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 			  errmsg("WAL level not sufficient for making an online backup"),
-				 errhint("wal_level must be set to \"archive\", \"hot_standby\", or \"logical\" at server start.")));
+				 errhint("wal_level must be set to \"hot_standby\" or \"logical\" at server start.")));
 
 	/*
 	 * OK to update backup counters and forcePageWrites
diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c
index 329bb8c..e29726c 100644
--- a/src/backend/access/transam/xlogfuncs.c
+++ b/src/backend/access/transam/xlogfuncs.c
@@ -154,7 +154,7 @@ pg_create_restore_point(PG_FUNCTION_ARGS)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 			 errmsg("WAL level not sufficient for creating a restore point"),
-				 errhint("wal_level must be set to \"archive\", \"hot_standby\", or \"logical\" at server start.")));
+				 errhint("wal_level must be set to \"hot_standby\" or \"logical\" at server start.")));
 
 	restore_name_str = text_to_cstring(restore_name);
 
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 000524d..9b6aa45 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -855,7 +855,7 @@ PostmasterMain(int argc, char *argv[])
 				(errmsg("WAL archival cannot be enabled when wal_level is \"minimal\"")));
 	if (max_wal_senders > 0 && wal_level == WAL_LEVEL_MINIMAL)
 		ereport(ERROR,
-				(errmsg("WAL streaming (max_wal_senders > 0) requires wal_level \"archive\", \"hot_standby\", or \"logical\"")));
+				(errmsg("WAL streaming (max_wal_senders > 0) requires wal_level \"hot_standby\" or \"logical\"")));
 
 	/*
 	 * Other one-time internal sanity checks can go here, if they are fast.
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index c66619c..1383339 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -775,11 +775,6 @@ CheckSlotRequirements(void)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 (errmsg("replication slots can only be used if max_replication_slots > 0"))));
-
-	if (wal_level < WAL_LEVEL_ARCHIVE)
-		ereport(ERROR,
-				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("replication slots can only be used if wal_level >= archive")));
 }
 
 /*
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 695a88f..2c32bfb 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -172,7 +172,7 @@
 
 # - Settings -
 
-#wal_level = minimal			# minimal, archive, hot_standby, or logical
+#wal_level = minimal			# minimal, hot_standby, or logical
 					# (change requires restart)
 #fsync = on				# turns forced synchronization on or off
 #synchronous_commit = on		# synchronization level;
diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index dc96bbf..35472a3 100644
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -36,7 +36,7 @@
 open CONF, ">>$tempdir/pgdata/postgresql.conf";
 print CONF "max_replication_slots = 10\n";
 print CONF "max_wal_senders = 10\n";
-print CONF "wal_level = archive\n";
+print CONF "wal_level = hot_standby\n";
 close CONF;
 restart_test_server;
 
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index 046480c..8368d8e 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -74,8 +74,6 @@ wal_level_str(WalLevel wal_level)
 	{
 		case WAL_LEVEL_MINIMAL:
 			return "minimal";
-		case WAL_LEVEL_ARCHIVE:
-			return "archive";
 		case WAL_LEVEL_HOT_STANDBY:
 			return "hot_standby";
 		case WAL_LEVEL_LOGICAL:
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 6dacee2..c74064a 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -120,7 +120,6 @@ extern int	XLogArchiveMode;
 typedef enum WalLevel
 {
 	WAL_LEVEL_MINIMAL = 0,
-	WAL_LEVEL_ARCHIVE,
 	WAL_LEVEL_HOT_STANDBY,
 	WAL_LEVEL_LOGICAL
 } WalLevel;
@@ -128,17 +127,17 @@ extern int	wal_level;
 
 /* Is WAL archiving enabled (always or only while server is running normally)? */
 #define XLogArchivingActive() \
-	(XLogArchiveMode > ARCHIVE_MODE_OFF && wal_level >= WAL_LEVEL_ARCHIVE)
+	(AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_HOT_STANDBY), XLogArchiveMode > ARCHIVE_MODE_OFF)
 /* Is WAL archiving enabled always (even during recovery)? */
 #define XLogArchivingAlways() \
-	(XLogArchiveMode == ARCHIVE_MODE_ALWAYS && wal_level >= WAL_LEVEL_ARCHIVE)
+	(AssertMacro(XLogArchiveMode == ARCHIVE_MODE_OFF || wal_level >= WAL_LEVEL_HOT_STANDBY), XLogArchiveMode == ARCHIVE_MODE_ALWAYS)
 #define XLogArchiveCommandSet() (XLogArchiveCommand[0] != '\0')
 
 /*
  * Is WAL-logging necessary for archival or log-shipping, or can we skip
  * WAL-logging if we fsync() the data before committing instead?
  */
-#define XLogIsNeeded() (wal_level >= WAL_LEVEL_ARCHIVE)
+#define XLogIsNeeded() (wal_level >= WAL_LEVEL_HOT_STANDBY)
 
 /*
  * Is a full-page image needed for hint bit updates?
-- 
2.5.1

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