On 02.12.23 15:06, Bharath Rupireddy wrote:
I enabled this code by compiling with the WAL_DEBUG macro and setting
wal_debug GUC to on. Firstly, the compilation on Windows failed
because XL_ROUTINE was passed inappropriately for XLogReaderAllocate()
used.
This kind of thing could be mostly avoided if we didn't hide all the
WAL_DEBUG behind #ifdefs. For example, in the attached patch, I instead
changed it so that
if (XLOG_DEBUG)
resolves to
if (false)
in the normal case. That way, we don't need to wrap that in #ifdef
WAL_DEBUG, and the compiler can see the disabled code and make sure it
continues to build.
From 93c74462c3e253f3ce9fd1beba1f09290988de8c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Wed, 6 Dec 2023 12:22:38 +0100
Subject: [PATCH] Make WAL_DEBUG code harder to break accidentally
---
src/backend/access/transam/generic_xlog.c | 2 --
src/backend/access/transam/xlog.c | 10 ----------
src/backend/access/transam/xlogrecovery.c | 7 -------
src/include/access/xlog.h | 2 ++
4 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/src/backend/access/transam/generic_xlog.c
b/src/backend/access/transam/generic_xlog.c
index abd9e1c749..5b910f1024 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -248,7 +248,6 @@ computeDelta(PageData *pageData, Page curpage, Page
targetpage)
* If xlog debug is enabled, then check produced delta. Result of delta
* application to curpage should be equivalent to targetpage.
*/
-#ifdef WAL_DEBUG
if (XLOG_DEBUG)
{
PGAlignedBlock tmp;
@@ -260,7 +259,6 @@ computeDelta(PageData *pageData, Page curpage, Page
targetpage)
BLCKSZ - targetUpper) != 0)
elog(ERROR, "result of generic xlog apply does not
match");
}
-#endif
}
/*
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index 2d603d8dee..3a7a38a100 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -644,9 +644,7 @@ static bool updateMinRecoveryPoint = true;
static int MyLockNo = 0;
static bool holdingAllLocks = false;
-#ifdef WAL_DEBUG
static MemoryContext walDebugCxt = NULL;
-#endif
static void CleanupAfterArchiveRecovery(TimeLineID EndOfLogTLI,
XLogRecPtr EndOfLog,
@@ -997,7 +995,6 @@ XLogInsertRecord(XLogRecData *rdata,
}
}
-#ifdef WAL_DEBUG
if (XLOG_DEBUG)
{
static XLogReaderState *debug_reader = NULL;
@@ -1061,7 +1058,6 @@ XLogInsertRecord(XLogRecData *rdata,
pfree(recordBuf.data);
MemoryContextSwitchTo(oldCxt);
}
-#endif
/*
* Update our global variables
@@ -1997,13 +1993,11 @@ AdvanceXLInsertBuffer(XLogRecPtr upto, TimeLineID tli,
bool opportunistic)
}
LWLockRelease(WALBufMappingLock);
-#ifdef WAL_DEBUG
if (XLOG_DEBUG && npages > 0)
{
elog(DEBUG1, "initialized %d pages, up to %X/%X",
npages, LSN_FORMAT_ARGS(NewPageEndPtr));
}
-#endif
}
/*
@@ -2641,13 +2635,11 @@ XLogFlush(XLogRecPtr record)
if (record <= LogwrtResult.Flush)
return;
-#ifdef WAL_DEBUG
if (XLOG_DEBUG)
elog(LOG, "xlog flush request %X/%X; write %X/%X; flush %X/%X",
LSN_FORMAT_ARGS(record),
LSN_FORMAT_ARGS(LogwrtResult.Write),
LSN_FORMAT_ARGS(LogwrtResult.Flush));
-#endif
START_CRIT_SECTION();
@@ -2903,14 +2895,12 @@ XLogBackgroundFlush(void)
WriteRqst.Flush = 0;
}
-#ifdef WAL_DEBUG
if (XLOG_DEBUG)
elog(LOG, "xlog bg flush request write %X/%X; flush: %X/%X,
current is write %X/%X; flush %X/%X",
LSN_FORMAT_ARGS(WriteRqst.Write),
LSN_FORMAT_ARGS(WriteRqst.Flush),
LSN_FORMAT_ARGS(LogwrtResult.Write),
LSN_FORMAT_ARGS(LogwrtResult.Flush));
-#endif
START_CRIT_SECTION();
diff --git a/src/backend/access/transam/xlogrecovery.c
b/src/backend/access/transam/xlogrecovery.c
index c61566666a..26ba990b71 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -396,9 +396,7 @@ static bool read_tablespace_map(List **tablespaces);
static void xlogrecovery_redo(XLogReaderState *record, TimeLineID replayTLI);
static void CheckRecoveryConsistency(void);
static void rm_redo_error_callback(void *arg);
-#ifdef WAL_DEBUG
static void xlog_outrec(StringInfo buf, XLogReaderState *record);
-#endif
static void xlog_block_info(StringInfo buf, XLogReaderState *record);
static void checkTimeLineSwitch(XLogRecPtr lsn, TimeLineID newTLI,
TimeLineID
prevTLI, TimeLineID replayTLI);
@@ -1703,7 +1701,6 @@ PerformWalRecovery(void)
ereport_startup_progress("redo in progress,
elapsed time: %ld.%02d s, current LSN: %X/%X",
LSN_FORMAT_ARGS(xlogreader->ReadRecPtr));
-#ifdef WAL_DEBUG
if (XLOG_DEBUG ||
(record->xl_rmid == RM_XACT_ID &&
trace_recovery_messages <= DEBUG2) ||
(record->xl_rmid != RM_XACT_ID &&
trace_recovery_messages <= DEBUG3))
@@ -1720,7 +1717,6 @@ PerformWalRecovery(void)
elog(LOG, "%s", buf.data);
pfree(buf.data);
}
-#endif
/* Handle interrupt signals of startup process */
HandleStartupProcInterrupts();
@@ -2256,8 +2252,6 @@ xlog_outdesc(StringInfo buf, XLogReaderState *record)
rmgr.rm_desc(buf, record);
}
-#ifdef WAL_DEBUG
-
static void
xlog_outrec(StringInfo buf, XLogReaderState *record)
{
@@ -2270,7 +2264,6 @@ xlog_outrec(StringInfo buf, XLogReaderState *record)
xlog_block_info(buf, record);
}
-#endif /* WAL_DEBUG */
/*
* Returns a string giving information about all the blocks in an
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index a14126d164..698dd79c2d 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -125,6 +125,8 @@ extern PGDLLIMPORT int wal_level;
#ifdef WAL_DEBUG
extern PGDLLIMPORT bool XLOG_DEBUG;
+#else
+#define XLOG_DEBUG false
#endif
/*
--
2.43.0
