Hi All,
An LSN is printed using format "%X/%X" and passing (uint32) (lsn >>
32), (uint32) lsn as arguments to that format specifier. This pattern
is repeated at 180 places according to Cscope. I find it hard to
remember this pattern every time I have to print LSN. Usually I end up
copying from some other place. Finding such a place takes a few
minutes and might be error prone esp when the lsn to be printed is an
expression. If we ever have to change this pattern in future, we will
need to change all those 180 places.
The solution seems to be simple though. In the attached patch, I have
added two macros
#define LSN_FORMAT "%X/%X"
#define LSN_FORMAT_ARG(lsn) (uint32) ((lsn) >> 32), (uint32) (lsn)
which can be used instead.
E.g. the following change in the patch
@@ -261,8 +261,7 @@ page_header(PG_FUNCTION_ARGS)
{
char lsnchar[64];
- snprintf(lsnchar, sizeof(lsnchar), "%X/%X",
- (uint32) (lsn >> 32), (uint32) lsn);
+ snprintf(lsnchar, sizeof(lsnchar), LSN_FORMAT,
LSN_FORMAT_ARG(lsn));
values[0] = CStringGetTextDatum(lsnchar);
LSN_FORMAT_ARG expands to two comma separated arguments and is kinda
open at both ends but it's handy that way.
Off list Peter Eisentraut pointed out that we can not use these macros
in elog/ereport since it creates problems for translations. He
suggested adding functions which return strings and use %s when doing
so.
The patch has two functions pg_lsn_out_internal() which takes an LSN
as input and returns a palloc'ed string containing the string
representation of LSN. This may not be suitable in performance
critical paths and also may leak memory if not freed. So there's
another function pg_lsn_out_buffer() which takes LSN and a char array
as input, fills the char array with the string representation and
returns the pointer to the char array. This allows the function to be
used as an argument in printf/elog etc. Macro MAXPG_LSNLEN has been
extern'elized for this purpose.
Off list Craig Ringer suggested introducing a new format specifier
similar to %m for LSN but I did not get time to take a look at the
relevant code. AFAIU it's available only to elog/ereport, so may not
be useful generally. But teaching printf variants about the new format
would be the best solution. However, I didn't find any way to do that.
In that patch I have changed some random code to use one of the above
methods appropriately, demonstrating their usage. Given that we have
grown 180 places already, I think that something like would have been
welcome earlier. But given that replication code is being actively
worked upon, it may not be too late. As a precedence lfirst_node() and
its variants arrived when the corresponding pattern had been repeated
at so many places.
I think we should move pg_lsn_out_internal() and pg_lsn_out_buffer()
somewhere else. Ideally xlogdefs.c would have been the best place but
there's no such file. May be we add those functions in pg_lsn.c and
add their declarations i xlogdefs.h.
--
Best Wishes,
Ashutosh Bapat
From 698e481f5f55b967b5c60dba4bc577f8baa20ff4 Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <ashutosh.ba...@2ndquadrant.com>
Date: Fri, 16 Oct 2020 17:09:29 +0530
Subject: [PATCH] Make it easy to print LSN
The commit introduces following macros and functions to make it easy to
use LSNs in printf variants, elog, ereport and appendStringInfo
variants.
LSN_FORMAT - macro representing the format in which LSN is printed
LSN_FORMAT_ARG - macro to pass LSN as an argument to the above format
pg_lsn_out_internal - a function which returns palloc'ed char array
containing string representation of given LSN.
pg_lsn_out_buffer - similar to above but accepts and returns a char
array of size (MAXPG_LSNLEN + 1)
The commit also has some example usages of these.
Ashutosh Bapat
---
contrib/pageinspect/rawpage.c | 3 +-
src/backend/access/rmgrdesc/replorigindesc.c | 5 +-
src/backend/access/rmgrdesc/xlogdesc.c | 3 +-
src/backend/access/transam/xlog.c | 8 ++--
src/backend/utils/adt/pg_lsn.c | 49 ++++++++++++++------
src/include/access/xlogdefs.h | 7 +++
src/include/utils/pg_lsn.h | 3 ++
7 files changed, 55 insertions(+), 23 deletions(-)
diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index c0181506a5..2cd055a5f0 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -261,8 +261,7 @@ page_header(PG_FUNCTION_ARGS)
{
char lsnchar[64];
- snprintf(lsnchar, sizeof(lsnchar), "%X/%X",
- (uint32) (lsn >> 32), (uint32) lsn);
+ snprintf(lsnchar, sizeof(lsnchar), LSN_FORMAT, LSN_FORMAT_ARG(lsn));
values[0] = CStringGetTextDatum(lsnchar);
}
else
diff --git a/src/backend/access/rmgrdesc/replorigindesc.c b/src/backend/access/rmgrdesc/replorigindesc.c
index 19e14f910b..a3f49b5750 100644
--- a/src/backend/access/rmgrdesc/replorigindesc.c
+++ b/src/backend/access/rmgrdesc/replorigindesc.c
@@ -29,10 +29,9 @@ replorigin_desc(StringInfo buf, XLogReaderState *record)
xlrec = (xl_replorigin_set *) rec;
- appendStringInfo(buf, "set %u; lsn %X/%X; force: %d",
+ appendStringInfo(buf, "set %u; lsn " LSN_FORMAT "; force: %d",
xlrec->node_id,
- (uint32) (xlrec->remote_lsn >> 32),
- (uint32) xlrec->remote_lsn,
+ LSN_FORMAT_ARG(xlrec->remote_lsn),
xlrec->force);
break;
}
diff --git a/src/backend/access/rmgrdesc/xlogdesc.c b/src/backend/access/rmgrdesc/xlogdesc.c
index 3200f777f5..22bdae5d9a 100644
--- a/src/backend/access/rmgrdesc/xlogdesc.c
+++ b/src/backend/access/rmgrdesc/xlogdesc.c
@@ -89,8 +89,7 @@ xlog_desc(StringInfo buf, XLogReaderState *record)
XLogRecPtr startpoint;
memcpy(&startpoint, rec, sizeof(XLogRecPtr));
- appendStringInfo(buf, "%X/%X",
- (uint32) (startpoint >> 32), (uint32) startpoint);
+ appendStringInfo(buf, LSN_FORMAT, LSN_FORMAT_ARG(startpoint));
}
else if (info == XLOG_PARAMETER_CHANGE)
{
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 13f1d8c3dc..2436af5244 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -79,6 +79,7 @@
#include "utils/pg_rusage.h"
#include "utils/snapmgr.h"
#include "utils/timestamp.h"
+#include "utils/pg_lsn.h"
extern uint32 bootstrap_data_checksum_version;
@@ -5886,15 +5887,16 @@ recoveryStopsAfter(XLogReaderState *record)
recoveryTargetInclusive &&
record->ReadRecPtr >= recoveryTargetLSN)
{
+ char buf[MAXPG_LSNLEN + 1];
+
recoveryStopAfter = true;
recoveryStopXid = InvalidTransactionId;
recoveryStopLSN = record->ReadRecPtr;
recoveryStopTime = 0;
recoveryStopName[0] = '\0';
ereport(LOG,
- (errmsg("recovery stopping after WAL location (LSN) \"%X/%X\"",
- (uint32) (recoveryStopLSN >> 32),
- (uint32) recoveryStopLSN)));
+ (errmsg("recovery stopping after WAL location (LSN) \"%s\"",
+ pg_lsn_out_buffer(recoveryStopLSN, buf))));
return true;
}
diff --git a/src/backend/utils/adt/pg_lsn.c b/src/backend/utils/adt/pg_lsn.c
index ad0a7bd869..403a20c568 100644
--- a/src/backend/utils/adt/pg_lsn.c
+++ b/src/backend/utils/adt/pg_lsn.c
@@ -19,7 +19,6 @@
#include "utils/numeric.h"
#include "utils/pg_lsn.h"
-#define MAXPG_LSNLEN 17
#define MAXPG_LSNCOMPONENT 8
/*----------------------------------------------------------
@@ -81,18 +80,7 @@ Datum
pg_lsn_out(PG_FUNCTION_ARGS)
{
XLogRecPtr lsn = PG_GETARG_LSN(0);
- char buf[MAXPG_LSNLEN + 1];
- char *result;
- uint32 id,
- off;
-
- /* Decode ID and offset */
- id = (uint32) (lsn >> 32);
- off = (uint32) lsn;
-
- snprintf(buf, sizeof buf, "%X/%X", id, off);
- result = pstrdup(buf);
- PG_RETURN_CSTRING(result);
+ PG_RETURN_CSTRING(pg_lsn_out_internal(lsn));
}
Datum
@@ -317,3 +305,38 @@ pg_lsn_mii(PG_FUNCTION_ARGS)
/* Convert to pg_lsn */
return DirectFunctionCall1(numeric_pg_lsn, res);
}
+
+/*
+ * pg_lsn_out_internal
+ *
+ * Returns palloc'ed string representation of an LSN. Used by pg_lsn_out() but
+ * is useful to print LSN in non-performance critical paths. The caller may
+ * pfree the returned memory chunk.
+ */
+char *
+pg_lsn_out_internal(XLogRecPtr lsn)
+{
+ char buf[MAXPG_LSNLEN + 1];
+
+ snprintf(buf, sizeof(buf), LSN_FORMAT, LSN_FORMAT_ARG(lsn));
+
+ return pstrdup(buf);
+}
+
+/*
+ * pg_lsn_out_buffer
+ *
+ * Similar to the above function but saves the string representation of LSN in
+ * the given buffer which is expected to be at least MAXPG_LSNLEN long. It can
+ * be used in performance critical paths to avoid expensive memory allocation.
+ *
+ * The function returns pointer to the input buffer so that it can be used in
+ * printf variants.
+ */
+char *
+pg_lsn_out_buffer(XLogRecPtr lsn, char *buf)
+{
+ snprintf(buf, sizeof(buf), LSN_FORMAT, LSN_FORMAT_ARG(lsn));
+
+ return buf;
+}
diff --git a/src/include/access/xlogdefs.h b/src/include/access/xlogdefs.h
index e1f5812213..c704ca2454 100644
--- a/src/include/access/xlogdefs.h
+++ b/src/include/access/xlogdefs.h
@@ -35,6 +35,13 @@ typedef uint64 XLogRecPtr;
*/
#define FirstNormalUnloggedLSN ((XLogRecPtr) 1000)
+/* Handy macros to print LSN */
+#define LSN_FORMAT "%X/%X"
+#define LSN_FORMAT_ARG(lsn) (uint32) ((lsn) >> 32), (uint32) (lsn)
+
+/* Maximum length of string representation of an LSN */
+#define MAXPG_LSNLEN 17
+
/*
* XLogSegNo - physical log file sequence number.
*/
diff --git a/src/include/utils/pg_lsn.h b/src/include/utils/pg_lsn.h
index 25d6c5b38e..7a8637d2bd 100644
--- a/src/include/utils/pg_lsn.h
+++ b/src/include/utils/pg_lsn.h
@@ -26,4 +26,7 @@
extern XLogRecPtr pg_lsn_in_internal(const char *str, bool *have_error);
+extern char *pg_lsn_out_internal(XLogRecPtr lsn);
+extern char *pg_lsn_out_buffer(XLogRecPtr lsn, char *buf);
+
#endif /* PG_LSN_H */
--
2.17.1
