trace_recovery_messages

Started by Fujii Masaoover 15 years ago7 messages
#1Fujii Masao
masao.fujii@gmail.com
1 attachment(s)

Hi,

The explanation of trace_recovery_messages in the document
is inconsistent with the definition of it in guc.c.

In the document,

* trace_recovery_messages is categorized into DEVELOPER_OPTIONS
* The default is WARNING
* Parameter should be set in the postgresql.conf only

But, in guc.c

* trace_recovery_messages is categorized into LOGGING_WHEN
* The default is DEBUG1
* The context is PGC_SUSET

ISTM the right is

* Categorized into DEVELOPER_OPTIONS
* The default is DEBUG1
* The context is PGC_SIGHUP

We should apply the attached patch which changes the document
and guc.c as above?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

trace_recovery_messages_v1.patchapplication/octet-stream; name=trace_recovery_messages_v1.patchDownload
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 5969,5975 **** LOG:  CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
         <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</>,
--- 5969,5975 ----
         <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 <literal>log_min_messages</>,
          but only for specific messages. This is intended for use in
          debugging Hot Standby.
          Valid values are <literal>DEBUG5</>, <literal>DEBUG4</>,
***************
*** 5978,5987 **** LOG:  CleanUpLock: deleting: lock(0xb7acd844) id(24688,24696,0,0,0,1)
          <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 the postgresql.conf only.
         </para>
        </listitem>
       </varlistentry>
--- 5978,5988 ----
          <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>DEBUG1</>.  Note that
          <literal>LOG</> has a different rank here than in
          <varname>client_min_messages</>.
!         This parameter can only be set in the <filename>postgresql.conf</>
!         file or on the server command line.
         </para>
        </listitem>
       </varlistentry>
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 375,381 **** int			log_min_messages = WARNING;
  int			client_min_messages = NOTICE;
  int			log_min_duration_statement = -1;
  int			log_temp_files = -1;
! int			trace_recovery_messages = LOG;
  
  int			num_temp_buffers = 1000;
  
--- 375,381 ----
  int			client_min_messages = NOTICE;
  int			log_min_duration_statement = -1;
  int			log_temp_files = -1;
! int			trace_recovery_messages = DEBUG1;
  
  int			num_temp_buffers = 1000;
  
***************
*** 2825,2831 **** static struct config_enum ConfigureNamesEnum[] =
  	},
  
  	{
! 		{"trace_recovery_messages", PGC_SUSET, LOGGING_WHEN,
  			gettext_noop("Sets the message levels that are logged during recovery."),
  			gettext_noop("Each level includes all the levels that follow it. The later"
  						 " the level, the fewer messages are sent.")
--- 2825,2831 ----
  	},
  
  	{
! 		{"trace_recovery_messages", PGC_SIGHUP, DEVELOPER_OPTIONS,
  			gettext_noop("Sets the message levels that are logged during recovery."),
  			gettext_noop("Each level includes all the levels that follow it. The later"
  						 " the level, the fewer messages are sent.")
#2Simon Riggs
simon@2ndQuadrant.com
In reply to: Fujii Masao (#1)
Re: trace_recovery_messages

On Tue, 2010-08-10 at 23:28 +0900, Fujii Masao wrote:

ISTM the right is

* Categorized into DEVELOPER_OPTIONS
* The default is DEBUG1
* The context is PGC_SIGHUP

Don't think we should go live with default of DEBUG1.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services

#3Fujii Masao
masao.fujii@gmail.com
In reply to: Simon Riggs (#2)
Re: trace_recovery_messages

On Wed, Aug 11, 2010 at 5:26 PM, Simon Riggs <simon@2ndquadrant.com> wrote:

On Tue, 2010-08-10 at 23:28 +0900, Fujii Masao wrote:

ISTM the right is

* Categorized into DEVELOPER_OPTIONS
* The default is DEBUG1
* The context is PGC_SIGHUP

Don't think we should go live with default of DEBUG1.

You think the default should be WARNING as described,
and guc.c should be changed accordingly? I have no
objection to it.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fujii Masao (#1)
Re: trace_recovery_messages

Fujii Masao <masao.fujii@gmail.com> writes:

The explanation of trace_recovery_messages in the document
is inconsistent with the definition of it in guc.c.

Setting the default to WARNING is confusing and useless, because
there are no trace_recovery calls with that debug level. IMO the
default setting should be LOG, which makes trace_recovery() a clear
no-op (rather than not clearly a no-op). There is circumstantial
evidence in the code that this was the original intention:

int trace_recovery_messages = LOG;

The documentation of the parameter is about as clear as mud, too.
We need to explain what it does rather than just copy-and-paste
a lot of text from log_min_messages.

regards, tom lane

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Fujii Masao (#1)
Re: trace_recovery_messages

Fujii Masao <masao.fujii@gmail.com> writes:

The explanation of trace_recovery_messages in the document
is inconsistent with the definition of it in guc.c.

I've applied a patch for this.

I was tempted to propose that we just rip out trace_recovery_messages
altogether, but I suppose Simon would object.

regards, tom lane

#6Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#5)
Re: trace_recovery_messages

On Thu, 2010-08-19 at 19:06 -0400, Tom Lane wrote:

Fujii Masao <masao.fujii@gmail.com> writes:

The explanation of trace_recovery_messages in the document
is inconsistent with the definition of it in guc.c.

I've applied a patch for this.

I was tempted to propose that we just rip out trace_recovery_messages
altogether, but I suppose Simon would object.

Thanks for keeping it in, hopefully it will help diagnose any errors.

I laughed when I saw the commit message, so thanks for that.

This is definitely a stop-gap facility. If you were to propose a more
general facility for increasing log level of specific modules, I'm sure
the rest of us would see that implemented across the rest of the code.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Training and Services

#7Robert Haas
robertmhaas@gmail.com
In reply to: Simon Riggs (#6)
Re: trace_recovery_messages

On Wed, Aug 25, 2010 at 5:36 AM, Simon Riggs <simon@2ndquadrant.com> wrote:

This is definitely a stop-gap facility. If you were to propose a more
general facility for increasing log level of specific modules, I'm sure
the rest of us would see that implemented across the rest of the code.

Yeah, I was thinking about that, too.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company