Re: Is WAL_DEBUG related code still relevant today?

On Thu, Dec 7, 2023 at 8:10 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Dec 06, 2023 at 11:32:19PM -0300, Euler Taveira wrote:
> > IIUC trace_recovery_messages was a debugging aid in the 9.0 era when the HS was
> > introduced. I'm also wondering if anyone used it in the past years.
>
> FWIW, I'd be +1 for getting rid of entirely, with

+1 for removing trace_recovery_messages. Firstly, it doesn't cover all
the recovery related messages as it promises, so it's an incomplete
feature. Secondly, it needs a bit of understanding as to how it gels
with client_min_messages and log_min_messages.

> its conditional
> block in PerformWalRecovery(), as it does not bring any additional
> value now that it is possible to achieve much more with pg_waldump
> (pg_xlogdump before that) introduced a couple of years later in 9.3.

And, I agree that the functionality (description of the WAL record
being applied) of conditional trace_recovery_messages code under
WAL_DEBUG macro in PerformWalRecovery's main redo apply loop can more
easily be achieved with either pg_walinspect or pg_waldump. That's my
point as well for getting rid of WAL_DEBUG macro related code after
converting a few messages to DEBUGX level.

The comment atop trace_recovery [1] function says it should go away
eventually and seems to have served the purpose when the recovery
related code was introduced in PG 9.0.

FWIW, the attached patch is what I've left with after removing
trace_recovery_messages related code, 9 files changed, 19
insertions(+), 97 deletions(-).

[1]
* 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)

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com