Bring some sanity to the trace_recovery_messages code and docs.

Per gripe from Fujii Masao, though this is not exactly his proposed patch.
Categorize as DEVELOPER_OPTIONS and set context PGC_SIGHUP, as per Fujii,
but set the default to LOG because higher values aren't really sensible
(see the code for trace_recovery()).  Fix the documentation to agree with
the code and to try to explain what the variable actually does.  Get rid
of no-op calls trace_recovery(LOG), which accomplish nothing except to
demonstrate that this option confuses even its author.

4 files changed, 48 insertions, 38 deletions

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f7a004c2387..97d13305707 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.294.2.4 2010/08/17 04:37:15 petere Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.294.2.5 2010/08/19 22:55:09 tgl Exp $ -->
 
 <chapter Id="runtime-config">
   <title>Server Configuration</title>
@@ -5689,6 +5689,31 @@ plruby.use_strict = true        # generates error: unknown class name
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-trace-recovery-messages" xreflabel="trace_recovery_messages">
+      <term><varname>trace_recovery_messages</varname> (<type>enum</type>)</term>
+      <indexterm>
+       <primary><varname>trace_recovery_messages</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        Enables logging of recovery-related debugging output that otherwise
+        would not be logged. This parameter allows the user to override the
+        normal setting of <xref linkend="guc-log-min-messages">, but only for
+        specific messages. This is intended for use in debugging Hot Standby.
+        Valid values are <literal>DEBUG5</>, <literal>DEBUG4</>,
+        <literal>DEBUG3</>, <literal>DEBUG2</>, <literal>DEBUG1</>, and
+        <literal>LOG</>.  The default, <literal>LOG</>, does not affect
+        logging decisions at all.  The other values cause recovery-related
+        debug messages of that priority or higher to be logged as though they
+        had <literal>LOG</> priority; for common settings of
+        <varname>log_min_messages</> this results in unconditionally sending
+        them to the server log.
+        This parameter can only be set in the <filename>postgresql.conf</>
+        file or on the server command line.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-trace-sort" xreflabel="trace_sort">
       <term><varname>trace_sort</varname> (<type>boolean</type>)</term>
       <indexterm>
@@ -5871,32 +5896,6 @@ LOG:  CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-trace-recovery-messages" xreflabel="trace_recovery_messages">
-      <term><varname>trace_recovery_messages</varname> (<type>enum</type>)</term>
-      <indexterm>
-       <primary><varname>trace_recovery_messages</> configuration parameter</primary>
-      </indexterm>
-      <listitem>
-       <para>
-        Controls which message levels are written to the server log
-        for system modules needed for recovery processing. This allows
-        the user to override the normal setting of log_min_messages,
-        but only for specific messages. This is intended for use in
-        debugging Hot Standby.
-        Valid values are <literal>DEBUG5</>, <literal>DEBUG4</>,
-        <literal>DEBUG3</>, <literal>DEBUG2</>, <literal>DEBUG1</>,
-        <literal>INFO</>, <literal>NOTICE</>, <literal>WARNING</>,
-        <literal>ERROR</>, <literal>LOG</>, <literal>FATAL</>, and
-        <literal>PANIC</>.  Each level includes all the levels that
-        follow it.  The later the level, the fewer messages are sent
-        to the log.  The default is <literal>WARNING</>.  Note that
-        <literal>LOG</> has a different rank here than in
-        <varname>client_min_messages</>.
-        Parameter should be set in <filename>postgresql.conf</filename> only.
-       </para>
-      </listitem>
-     </varlistentry>
-
     <varlistentry id="guc-zero-damaged-pages" xreflabel="zero_damaged_pages">
       <term><varname>zero_damaged_pages</varname> (<type>boolean</type>)</term>
       <indexterm>
diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 386e712e785..9f71c1a156a 100644
--- a/src/backend/storage/ipc/standby.c
+++ b/src/backend/storage/ipc/standby.c
@@ -11,7 +11,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/storage/ipc/standby.c,v 1.27.2.1 2010/08/12 23:25:45 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/standby.c,v 1.27.2.2 2010/08/19 22:55:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -591,7 +591,7 @@ StandbyReleaseLocks(TransactionId xid)
 				 lock->xid, lock->dbOid, lock->relOid);
 			SET_LOCKTAG_RELATION(locktag, lock->dbOid, lock->relOid);
 			if (!LockRelease(&locktag, AccessExclusiveLock, true))
-				elog(trace_recovery(LOG),
+				elog(LOG,
 					 "RecoveryLockList contains entry for lock no longer recorded by lock manager: xid %u database %u relation %u",
 					 lock->xid, lock->dbOid, lock->relOid);
 
@@ -655,7 +655,7 @@ StandbyReleaseLocksMany(TransactionId removeXid, bool keepPreparedXacts)
 				 lock->xid, lock->dbOid, lock->relOid);
 			SET_LOCKTAG_RELATION(locktag, lock->dbOid, lock->relOid);
 			if (!LockRelease(&locktag, AccessExclusiveLock, true))
-				elog(trace_recovery(LOG),
+				elog(LOG,
 					 "RecoveryLockList contains entry for lock no longer recorded by lock manager: xid %u database %u relation %u",
 					 lock->xid, lock->dbOid, lock->relOid);
 			RecoveryLockList = list_delete_cell(RecoveryLockList, cell, prev);
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 2cafc31b2cd..feb0bd51fc9 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -42,7 +42,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.224.2.1 2010/07/18 23:43:37 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.224.2.2 2010/08/19 22:55:10 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2798,12 +2798,19 @@ is_log_level_output(int elevel, int log_min_level)
 }
 
 /*
- * If trace_recovery_messages is set to make this visible, then show as LOG,
- * else display as whatever level is set. It may still be shown, but only
- * if log_min_messages is set lower than trace_recovery_messages.
+ * Adjust the level of a recovery-related message per trace_recovery_messages.
+ *
+ * The argument is the default log level of the message, eg, DEBUG2.  (This
+ * should only be applied to DEBUGn log messages, otherwise it's a no-op.)
+ * If the level is >= trace_recovery_messages, we return LOG, causing the
+ * message to be logged unconditionally (for most settings of
+ * log_min_messages).  Otherwise, we return the argument unchanged.
+ * The message will then be shown based on the setting of log_min_messages.
  *
  * Intention is to keep this for at least the whole of the 9.0 production
  * release, so we can more easily diagnose production problems in the field.
+ * It should go away eventually, though, because it's an ugly and
+ * hard-to-explain kluge.
  */
 int
 trace_recovery(int trace_level)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 08704d48cf0..4e55b161f6c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.561 2010/07/06 22:55:26 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.561.2.1 2010/08/19 22:55:10 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -2785,13 +2785,17 @@ static struct config_enum ConfigureNamesEnum[] =
 	},
 
 	{
-		{"trace_recovery_messages", PGC_SUSET, LOGGING_WHEN,
-			gettext_noop("Sets the message levels that are logged during recovery."),
+		{"trace_recovery_messages", PGC_SIGHUP, DEVELOPER_OPTIONS,
+			gettext_noop("Enables logging of recovery-related debugging information."),
 			gettext_noop("Each level includes all the levels that follow it. The later"
 						 " the level, the fewer messages are sent.")
 		},
 		&trace_recovery_messages,
-		DEBUG1, server_message_level_options, NULL, NULL
+		/*
+		 * client_message_level_options allows too many values, really,
+		 * but it's not worth having a separate options array for this.
+		 */
+		LOG, client_message_level_options, NULL, NULL
 	},
 
 	{