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.

3 files changed, 22 insertions, 11 deletions

diff --git a/src/backend/storage/ipc/standby.c b/src/backend/storage/ipc/standby.c
index 8a4be0a1dbd..502e145cdac 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.28 2010/08/12 23:24:54 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/storage/ipc/standby.c,v 1.29 2010/08/19 22:55:01 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 93f0ba0e392..181db7e4515 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.225 2010/07/18 23:43:32 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/error/elog.c,v 1.226 2010/08/19 22:55:01 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 dac704ee4cd..3be7874f08a 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.567 2010/08/13 20:10:53 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.568 2010/08/19 22:55:01 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -2829,13 +2829,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
 	},
 
 	{