Add TLI number to name of files generated by pg_waldump --save-fullpage
Hi all,
(Fujii-san and David in CC.)
Fujii-san has reported on Twitter that we had better add the TLI
number to what pg_waldump --save-fullpage generates for the file names
of the blocks, as it could be possible that we overwrite some blocks.
This information can be added thanks to ws_tli, that tracks the TLI of
the opened segment.
Attached is a patch to fix this issue, adding an open item assigned to
me. The file format is documented in the TAP test and the docs, the
two only places that would need a refresh.
Thoughts or comments?
--
Michael
Attachments:
v1-0001-Add-timeline-to-file-names-generated-with-pg_wald.patchtext/x-diff; charset=us-asciiDownload
From 35dc549115e8bf054a5f756c652b07f3f49d0bce Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Tue, 27 Jun 2023 15:04:49 +0900
Subject: [PATCH v1] Add timeline to file names generated with pg_waldump
--save-fullpage
Reported-by: Fujii Masao
---
src/bin/pg_waldump/pg_waldump.c | 3 ++-
src/bin/pg_waldump/t/002_save_fullpage.pl | 9 +++++----
doc/src/sgml/ref/pg_waldump.sgml | 9 ++++++++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c6d3ae6344..96845e1a1a 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -518,7 +518,8 @@ XLogRecordSaveFPWs(XLogReaderState *record, const char *savepath)
else
pg_fatal("invalid fork number: %u", fork);
- snprintf(filename, MAXPGPATH, "%s/%08X-%08X.%u.%u.%u.%u%s", savepath,
+ snprintf(filename, MAXPGPATH, "%s/%08X-%08X-%08X.%u.%u.%u.%u%s", savepath,
+ record->seg.ws_tli,
LSN_FORMAT_ARGS(record->ReadRecPtr),
rnode.spcOid, rnode.dbOid, rnode.relNumber, blk, forkname);
diff --git a/src/bin/pg_waldump/t/002_save_fullpage.pl b/src/bin/pg_waldump/t/002_save_fullpage.pl
index 831ffdefef..dc53a6f5b4 100644
--- a/src/bin/pg_waldump/t/002_save_fullpage.pl
+++ b/src/bin/pg_waldump/t/002_save_fullpage.pl
@@ -79,15 +79,16 @@ $node->command_ok(
'pg_waldump with --save-fullpage runs');
# This regexp will match filenames formatted as:
-# XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
-# - WAL LSN in hex format,
-# - Tablespace OID (0 for global)
+# TLI-XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
+# - Timeline number in hex format.
+# - WAL LSN in hex format.
+# - Tablespace OID (0 for global).
# - Database OID.
# - Relfilenode.
# - Block number.
# - Fork this block came from (vm, init, fsm, or main).
my $file_re =
- qr/^([0-9A-F]{8})-([0-9A-F]{8})[.][0-9]+[.][0-9]+[.][0-9]+[.][0-9]+(?:_vm|_init|_fsm|_main)?$/;
+ qr/^[0-9A-F]{8}-([0-9A-F]{8})-([0-9A-F]{8})[.][0-9]+[.][0-9]+[.][0-9]+[.][0-9]+(?:_vm|_init|_fsm|_main)?$/;
my $file_count = 0;
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index e5f9637847..4592d6016a 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -283,7 +283,7 @@ PostgreSQL documentation
</para>
<para>
The full page images are saved with the following file name format:
- <literal><replaceable>LSN</replaceable>.<replaceable>RELTABLESPACE</replaceable>.<replaceable>DATOID</replaceable>.<replaceable>RELNODE</replaceable>.<replaceable>BLKNO</replaceable><replaceable>FORK</replaceable></literal>
+ <literal><replaceable>TIMELINE</replaceable>-<replaceable>LSN</replaceable>.<replaceable>RELTABLESPACE</replaceable>.<replaceable>DATOID</replaceable>.<replaceable>RELNODE</replaceable>.<replaceable>BLKNO</replaceable><replaceable>FORK</replaceable></literal>
The file names are composed of the following parts:
<informaltable>
@@ -296,6 +296,13 @@ PostgreSQL documentation
</thead>
<tbody>
+ <row>
+ <entry>TIMELINE</entry>
+ <entry>The timeline of the WAL segment file where the record
+ is located formatted as one 8-character hexadecimal number
+ <literal>%08X</literal></entry>
+ </row>
+
<row>
<entry>LSN</entry>
<entry>The <acronym>LSN</acronym> of the record with this image,
--
2.40.1
At Tue, 27 Jun 2023 15:12:43 +0900, Michael Paquier <michael@paquier.xyz> wrote in
Hi all,
(Fujii-san and David in CC.)Fujii-san has reported on Twitter that we had better add the TLI
number to what pg_waldump --save-fullpage generates for the file names
of the blocks, as it could be possible that we overwrite some blocks.
This information can be added thanks to ws_tli, that tracks the TLI of
the opened segment.Attached is a patch to fix this issue, adding an open item assigned to
me. The file format is documented in the TAP test and the docs, the
two only places that would need a refresh.Thoughts or comments?
It's sensible to add TLI to the file name. So +1 from me.
+# - Timeline number in hex format.
Arn't we reffering to it as "Timeline ID"? (I remember there was a
discussion about redefining the "timeline ID" to use non-orderable
IDs. That is, making it non-numbers.)
Otherwise it looks fine to me.
By the way, somewhat irrelevant to this patch, regading the the file
name for the output.
The file name was "LSNh-LSNl.spcOid.dbOid.relNumber.blk_forkname", but
the comment in the TAP script read as:
-# XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
which looks wrong. I'm not sure it is a business of this patch, though..
# Documentation looks coorect.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Tue, Jun 27, 2023 at 03:44:04PM +0900, Kyotaro Horiguchi wrote:
+# - Timeline number in hex format.
Arn't we reffering to it as "Timeline ID"? (I remember there was a
discussion about redefining the "timeline ID" to use non-orderable
IDs. That is, making it non-numbers.)
Using ID is fine by me.
The file name was "LSNh-LSNl.spcOid.dbOid.relNumber.blk_forkname", but
the comment in the TAP script read as:-# XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
which looks wrong. I'm not sure it is a business of this patch, though..
This part is getting changed here anyway, so improving it is fine by
me with the terms you are suggesting for these two 4-byte values in
this comment.
--
Michael
At Tue, 27 Jun 2023 15:58:38 +0900, Michael Paquier <michael@paquier.xyz> wrote in
On Tue, Jun 27, 2023 at 03:44:04PM +0900, Kyotaro Horiguchi wrote:
The file name was "LSNh-LSNl.spcOid.dbOid.relNumber.blk_forkname", but
the comment in the TAP script read as:-# XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
which looks wrong. I'm not sure it is a business of this patch, though..
This part is getting changed here anyway, so improving it is fine by
me with the terms you are suggesting for these two 4-byte values in
this comment.
I meant that the name is structured as
TLIh-TLIl.<tablespace>.<database>.<relnumber>.<blk>._<fork>, which
appears to be inconsistent with the comment. (And I'm not sure what
"TLOID" is..)
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
Of course, it's wrong.
At Tue, 27 Jun 2023 16:39:52 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail
.com> wrote in
I meant that the name is structured as
- TLIh-TLIl.<tablespace>.<database>.<relnumber>.<blk>._<fork>, which
+ LSNh-LSNl.<tablespace>.<database>.<relnumber>.<blk>._<fork>, whichShow quoted text
appears to be inconsistent with the comment. (And I'm not sure what
"TLOID" is..)regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Tue, Jun 27, 2023 at 1:12 AM Michael Paquier <michael@paquier.xyz> wrote:
Hi all,
(Fujii-san and David in CC.)Fujii-san has reported on Twitter that we had better add the TLI
number to what pg_waldump --save-fullpage generates for the file names
of the blocks, as it could be possible that we overwrite some blocks.
This information can be added thanks to ws_tli, that tracks the TLI of
the opened segment.Attached is a patch to fix this issue, adding an open item assigned to
me. The file format is documented in the TAP test and the docs, the
two only places that would need a refresh.Thoughts or comments?
Patch looks good, but agreed that that comment should also be fixed.
Thanks!
David
On Tue, Jun 27, 2023 at 11:53:10AM -0500, David Christensen wrote:
Patch looks good, but agreed that that comment should also be fixed.
Okay, thanks for checking!
--
Michael
On Tue, Jun 27, 2023 at 04:39:52PM +0900, Kyotaro Horiguchi wrote:
I meant that the name is structured as
TLIh-TLIl.<tablespace>.<database>.<relnumber>.<blk>._<fork>, which
appears to be inconsistent with the comment. (And I'm not sure what
"TLOID" is..)
Well, to be clear, it should not be TLIh-TLIl but LSNh-LSNl :)
I'm OK with these terms for the comments. This is very internal
anyway so anybody using this feature should know what that means.
And yes, the order of the items is wrong, and I agree that TLOID is a
bit confusing once the TLI is added in the set. I have just used
TBLSPCOID as term in the comment, and adjusted the XXX to be about the
LSN numbers.
Adjusted as per the v2 attached.
--
Michael
Attachments:
v2-0001-Add-timeline-to-file-names-generated-with-pg_wald.patchtext/x-diff; charset=us-asciiDownload
From f0936a95651f767e0f56ff507db5d5a02c7629b2 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Wed, 28 Jun 2023 08:46:26 +0900
Subject: [PATCH v2] Add timeline to file names generated with pg_waldump
--save-fullpage
Reported-by: Fujii Masao
---
src/bin/pg_waldump/pg_waldump.c | 3 ++-
src/bin/pg_waldump/t/002_save_fullpage.pl | 9 +++++----
doc/src/sgml/ref/pg_waldump.sgml | 9 ++++++++-
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c6d3ae6344..96845e1a1a 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -518,7 +518,8 @@ XLogRecordSaveFPWs(XLogReaderState *record, const char *savepath)
else
pg_fatal("invalid fork number: %u", fork);
- snprintf(filename, MAXPGPATH, "%s/%08X-%08X.%u.%u.%u.%u%s", savepath,
+ snprintf(filename, MAXPGPATH, "%s/%08X-%08X-%08X.%u.%u.%u.%u%s", savepath,
+ record->seg.ws_tli,
LSN_FORMAT_ARGS(record->ReadRecPtr),
rnode.spcOid, rnode.dbOid, rnode.relNumber, blk, forkname);
diff --git a/src/bin/pg_waldump/t/002_save_fullpage.pl b/src/bin/pg_waldump/t/002_save_fullpage.pl
index 831ffdefef..f0725805f2 100644
--- a/src/bin/pg_waldump/t/002_save_fullpage.pl
+++ b/src/bin/pg_waldump/t/002_save_fullpage.pl
@@ -79,15 +79,16 @@ $node->command_ok(
'pg_waldump with --save-fullpage runs');
# This regexp will match filenames formatted as:
-# XXXXXXXX-XXXXXXXX.DBOID.TLOID.NODEOID.dd_fork with the components being:
-# - WAL LSN in hex format,
-# - Tablespace OID (0 for global)
+# TLI-LSNh-LSNl.TBLSPCOID.DBOID.NODEOID.dd_fork with the components being:
+# - Timeline ID in hex format.
+# - WAL LSN in hex format, as two 8-character numbers.
+# - Tablespace OID (0 for global).
# - Database OID.
# - Relfilenode.
# - Block number.
# - Fork this block came from (vm, init, fsm, or main).
my $file_re =
- qr/^([0-9A-F]{8})-([0-9A-F]{8})[.][0-9]+[.][0-9]+[.][0-9]+[.][0-9]+(?:_vm|_init|_fsm|_main)?$/;
+ qr/^[0-9A-F]{8}-([0-9A-F]{8})-([0-9A-F]{8})[.][0-9]+[.][0-9]+[.][0-9]+[.][0-9]+(?:_vm|_init|_fsm|_main)?$/;
my $file_count = 0;
diff --git a/doc/src/sgml/ref/pg_waldump.sgml b/doc/src/sgml/ref/pg_waldump.sgml
index e5f9637847..4592d6016a 100644
--- a/doc/src/sgml/ref/pg_waldump.sgml
+++ b/doc/src/sgml/ref/pg_waldump.sgml
@@ -283,7 +283,7 @@ PostgreSQL documentation
</para>
<para>
The full page images are saved with the following file name format:
- <literal><replaceable>LSN</replaceable>.<replaceable>RELTABLESPACE</replaceable>.<replaceable>DATOID</replaceable>.<replaceable>RELNODE</replaceable>.<replaceable>BLKNO</replaceable><replaceable>FORK</replaceable></literal>
+ <literal><replaceable>TIMELINE</replaceable>-<replaceable>LSN</replaceable>.<replaceable>RELTABLESPACE</replaceable>.<replaceable>DATOID</replaceable>.<replaceable>RELNODE</replaceable>.<replaceable>BLKNO</replaceable><replaceable>FORK</replaceable></literal>
The file names are composed of the following parts:
<informaltable>
@@ -296,6 +296,13 @@ PostgreSQL documentation
</thead>
<tbody>
+ <row>
+ <entry>TIMELINE</entry>
+ <entry>The timeline of the WAL segment file where the record
+ is located formatted as one 8-character hexadecimal number
+ <literal>%08X</literal></entry>
+ </row>
+
<row>
<entry>LSN</entry>
<entry>The <acronym>LSN</acronym> of the record with this image,
--
2.40.1
Adjusted as per the v2 attached.
+1
At Tue, 27 Jun 2023 18:58:39 -0500, David Christensen <david.christensen@crunchydata.com> wrote in
Adjusted as per the v2 attached.
+1
+1
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
On Wed, Jun 28, 2023 at 09:20:27AM +0900, Kyotaro Horiguchi wrote:
At Tue, 27 Jun 2023 18:58:39 -0500, David Christensen <david.christensen@crunchydata.com> wrote in
Adjusted as per the v2 attached.
+1
+1
Okay, cool. Both of you seem happy with it, so I have applied it.
Thanks for the quick checks.
--
Michael