Thread: make "wal_debug" GUC var a boolean

The "wal_debug" GUC variable was of type "integer", but it was used in
the code effectively as a boolean: the code only tested whether it was
zero or non-zero. This patch makes it a proper boolean.

-Neil

Neil Conway wrote:
> The "wal_debug" GUC variable was of type "integer", but it was used
> in the code effectively as a boolean: the code only tested whether it
> was zero or non-zero. This patch makes it a proper boolean.

I agree with this, but would it be possible to fold it into the normal
debug output mechanisms?  I don't think debugging WAL is any more
important nowadays than any other component.

Peter Eisentraut <peter_e@gmx.net> writes:
> I agree with this, but would it be possible to fold it into the normal
> debug output mechanisms?  I don't think debugging WAL is any more
> important nowadays than any other component.

I agree.  Having a special debug variable for WAL seems like a
short-term thing whose time has passed.  You could probably even
make that code a compile-time option like the others at the
bottom of pg_config_manual.h.

            regards, tom lane

Tom Lane wrote:
> I agree.  Having a special debug variable for WAL seems like a
> short-term thing whose time has passed.  You could probably even
> make that code a compile-time option like the others at the
> bottom of pg_config_manual.h.

That seems right.  It's never something that a user would need, because
by the time you'd tell him to turn it on, the events won't be
repeatable.

Btw., that list in pg_config_manual.h is totally incomplete and seems
unnecessary because most of these things only affect one file and are
better turned on or off there.

Tom Lane <tgl@sss.pgh.pa.us> writes:
> I agree.  Having a special debug variable for WAL seems like a
> short-term thing whose time has passed.

ISTM that there is little distinguishing wal_debug and the following
GUC vars:

log_btree_build_stats
trace_notify
trace_locks
trace_userlocks
trace_lwlocks
debug_deadlocks
trace_lock_oidmin
trace_lock_table
debug_shared_buffers

Should these be changed as well?

-Neil

Neil Conway <neilc@samurai.com> writes:
> ISTM that there is little distinguishing wal_debug and the following
> GUC vars:

> log_btree_build_stats
> trace_notify
> trace_locks
> trace_userlocks
> trace_lwlocks
> debug_deadlocks
> trace_lock_oidmin
> trace_lock_table
> debug_shared_buffers

> Should these be changed as well?

Well, Jan just put in debug_shared_buffers recently, so I'd assume
that's still a live debugging option.

The others are already #ifdef'd out by default, which is more or
less what I was suggesting you do with the wal_debug code.

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:
> The others are already #ifdef'd out by default, which is more or
> less what I was suggesting you do with the wal_debug code.

(FWIW, trace_notify is not #ifdef'd out.)

But I thought that you (and Peter) were suggesting that this shouldn't
be made a GUC variable at all, just a preprocessor definition.

-Neil

Neil Conway <neilc@samurai.com> writes:
> Tom Lane <tgl@sss.pgh.pa.us> writes:
>> The others are already #ifdef'd out by default, which is more or
>> less what I was suggesting you do with the wal_debug code.

> (FWIW, trace_notify is not #ifdef'd out.)

My mistake --- if you feel like making it so, go to it.

> But I thought that you (and Peter) were suggesting that this shouldn't
> be made a GUC variable at all, just a preprocessor definition.

I think the combination of #ifdef and GUC variable is appropriate.
Just because you want to debug something doesn't necessarily mean you
want the log messages on full strength every instant, especially for
high-volume messages such as the WAL ones.  You might for instance want
to log just one backend's WAL activity.

            regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> writes:
> I think the combination of #ifdef and GUC variable is appropriate.

The attached patch implements this: the wal_debug GUC var is now a
boolean, and is encloded in `#ifdef WAL_DEBUG` blocks (and the
WAL_DEBUG symbol is present in pg_config_manual.h, just commented
out).

Unless anyone objects, I intend to apply this in 24 hours or so.

-Neil

Neil Conway <neilc@samurai.com> writes:
> Unless anyone objects, I intend to apply this in 24 hours or so.

Patch applied.

-Neil