Re: Printing LSN made easy

From: Alexey Kondratov <a(dot)kondratov(at)postgrespro(dot)ru>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Craig Ringer <craig(dot)ringer(at)enterprisedb(dot)com>
Subject: Re: Printing LSN made easy
Date: 2020-11-27 14:24:20
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 2020-11-27 13:40, Ashutosh Bapat wrote:
> 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.

If usage of macros in elog/ereport can cause problems for translation,
then even with this patch life is not get simpler significantly. For
example, instead of just doing like:

- "xlog min recovery request %X/%X is past current point
- (uint32) (lsn >> 32), (uint32) lsn,
- (uint32) (newMinRecoveryPoint >> 32),
- (uint32) newMinRecoveryPoint);
+ "xlog min recovery request " LSN_FORMAT " is past
current point " LSN_FORMAT,
+ LSN_FORMAT_ARG(newMinRecoveryPoint));

we have to either declare two additional local buffers, which is
verbose; or use pg_lsn_out_internal() and rely on memory contexts (or do
pfree() manually, which is verbose again) to prevent memory leaks.

> 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.

It seems that this topic has been extensively discussed off-list, but
still strong +1 for the patch. I always wanted LSN printing to be more

I have just tried new printing utilities in a couple of new places and
it looks good to me.

+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);

Would it be a bit more straightforward if we palloc buf initially and
just return a pointer instead of doing pstrdup()?

Alexey Kondratov

Postgres Professional
Russian Postgres Company

Attachment Content-Type Size
0001-Make-it-easy-to-print-LSN.patch text/x-patch 6.7 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2020-11-27 14:37:05 Re: proposal: unescape_text function
Previous Message Peter Eisentraut 2020-11-27 13:57:25 Re: Improving spin-lock implementation on ARM.