public inbox for [email protected]  
help / color / mirror / Atom feed
From: Peter Eisentraut <[email protected]>
To: Bharath Rupireddy <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: Re: Is WAL_DEBUG related code still relevant today?
Date: Wed, 6 Dec 2023 12:27:17 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <CALj2ACVF14WKQMFwcJ=3okVDhiXpuK5f7YdT+BdYXbbypMHqWA@mail.gmail.com>
References: <CALj2ACVF14WKQMFwcJ=3okVDhiXpuK5f7YdT+BdYXbbypMHqWA@mail.gmail.com>

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 <[email protected]>
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



Attachments:

  [text/plain] 0001-Make-WAL_DEBUG-code-harder-to-break-accidentally.patch (5.1K, ../[email protected]/2-0001-Make-WAL_DEBUG-code-harder-to-break-accidentally.patch)
  download | inline diff:
From 93c74462c3e253f3ce9fd1beba1f09290988de8c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <[email protected]>
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



view thread (6+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Is WAL_DEBUG related code still relevant today?
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox