On Mon, Mar 27, 2023 at 9:11 AM Kyotaro Horiguchi
<horikyota....@gmail.com> wrote:
>
> At Sat, 25 Mar 2023 12:12:50 +0900, Michael Paquier <mich...@paquier.xyz> 
> wrote in
> > On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> >         OUT reltablespace oid,
> >         OUT reldatabase oid,
> >         OUT relfilenode oid,
> >         OUT relblocknumber int8,
> > +       OUT blockid int2,
> > +    OUT start_lsn pg_lsn,
> > +    OUT end_lsn pg_lsn,
> > +    OUT prev_lsn pg_lsn,
> >
> > I'd still put the LSN data before the three OIDs for consistency with
> > the structures, though my opinion does not seem to count much..
>
> I agree with Michael on this point. Also, although it may not be
> significant for SQL, the rows are output in lsn order from the
> function.

Done that way.

On Sat, Mar 25, 2023 at 8:42 AM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Thu, Mar 23, 2023 at 10:54:40PM +0530, Bharath Rupireddy wrote:
> > Please see the attached v4 patch set addressing all the review comments.
>
> -   desc = GetRmgr(XLogRecGetRmid(record));
> -   id = desc.rm_identify(XLogRecGetInfo(record));
> -
> -   if (id == NULL)
> -       id = psprintf("UNKNOWN (%x)", XLogRecGetInfo(record) & 
> ~XLR_INFO_MASK);
> -
> -   initStringInfo(&rec_desc);
> -   desc.rm_desc(&rec_desc, record);
> -
> -   /* Block references. */
> -   initStringInfo(&rec_blk_ref);
> -   XLogRecGetBlockRefInfo(record, false, true, &rec_blk_ref, &fpi_len);
> -
> -   main_data_len = XLogRecGetDataLen(record);
>
> I don't see any need to move this block of code?  This leads to
> unnecessary diffs, potentially making backpatch a bit harder.  Either
> way is not a big deal, still..  Except for this bit, 0001 looks fine
> by me.

It's a cosmetic change - I wanted to keep the calculation of column
values closer to where they're assigned to Datum values. I agree to
not cause too much diff and removed them.

Please see the attached v5 patch set.

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 601c47f0e56e721da7a5fb7e0427faa926ad982c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 27 Mar 2023 03:50:44 +0000
Subject: [PATCH v5] Few optimizations in pg_walinspect

1. Emits null values in output columns (description and block_ref)
if WAL records have no data to present.
2. Function to get block references is skipped for the records
that don't have block references.
---
 contrib/pg_walinspect/pg_walinspect.c | 39 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index 3b3215daf5..90fb8eaed1 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -187,7 +187,6 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
 	uint32		fpi_len = 0;
 	StringInfoData rec_desc;
 	StringInfoData rec_blk_ref;
-	uint32		main_data_len;
 	int			i = 0;
 
 	desc = GetRmgr(XLogRecGetRmid(record));
@@ -199,11 +198,11 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
 	initStringInfo(&rec_desc);
 	desc.rm_desc(&rec_desc, record);
 
-	/* Block references. */
-	initStringInfo(&rec_blk_ref);
-	XLogRecGetBlockRefInfo(record, false, true, &rec_blk_ref, &fpi_len);
-
-	main_data_len = XLogRecGetDataLen(record);
+	if (XLogRecHasAnyBlockRefs(record))
+	{
+		initStringInfo(&rec_blk_ref);
+		XLogRecGetBlockRefInfo(record, false, true, &rec_blk_ref, &fpi_len);
+	}
 
 	values[i++] = LSNGetDatum(record->ReadRecPtr);
 	values[i++] = LSNGetDatum(record->EndRecPtr);
@@ -212,10 +211,18 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
 	values[i++] = CStringGetTextDatum(desc.rm_name);
 	values[i++] = CStringGetTextDatum(id);
 	values[i++] = UInt32GetDatum(XLogRecGetTotalLen(record));
-	values[i++] = UInt32GetDatum(main_data_len);
+	values[i++] = UInt32GetDatum(XLogRecGetDataLen(record));
 	values[i++] = UInt32GetDatum(fpi_len);
-	values[i++] = CStringGetTextDatum(rec_desc.data);
-	values[i++] = CStringGetTextDatum(rec_blk_ref.data);
+
+	if (rec_desc.len > 0)
+		values[i++] = CStringGetTextDatum(rec_desc.data);
+	else
+		nulls[i++] = true;
+
+	if (XLogRecHasAnyBlockRefs(record))
+		values[i++] = CStringGetTextDatum(rec_blk_ref.data);
+	else
+		nulls[i++] = true;
 
 	Assert(i == ncols);
 }
@@ -232,6 +239,8 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
 	int			block_id;
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 
+	Assert(XLogRecHasAnyBlockRefs(record));
+
 	for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++)
 	{
 		DecodedBkpBlock *blk;
@@ -377,6 +386,11 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
 	while (ReadNextXLogRecord(xlogreader) &&
 		   xlogreader->EndRecPtr <= end_lsn)
 	{
+		CHECK_FOR_INTERRUPTS();
+
+		if (!XLogRecHasAnyBlockRefs(xlogreader))
+			continue;
+
 		/* Use the tmp context so we can clean up after each tuple is done */
 		old_cxt = MemoryContextSwitchTo(tmp_cxt);
 
@@ -385,8 +399,6 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
 		/* clean up and switch back */
 		MemoryContextSwitchTo(old_cxt);
 		MemoryContextReset(tmp_cxt);
-
-		CHECK_FOR_INTERRUPTS();
 	}
 
 	MemoryContextDelete(tmp_cxt);
@@ -483,8 +495,6 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 #define PG_GET_WAL_RECORDS_INFO_COLS 11
 	XLogReaderState *xlogreader;
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
-	Datum		values[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
-	bool		nulls[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
 	MemoryContext old_cxt;
 	MemoryContext tmp_cxt;
 
@@ -501,6 +511,9 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 	while (ReadNextXLogRecord(xlogreader) &&
 		   xlogreader->EndRecPtr <= end_lsn)
 	{
+		Datum		values[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
+		bool		nulls[PG_GET_WAL_RECORDS_INFO_COLS] = {0};
+
 		/* Use the tmp context so we can clean up after each tuple is done */
 		old_cxt = MemoryContextSwitchTo(tmp_cxt);
 
-- 
2.34.1

From 6c3c2966d12768633bd158e145d70e15d6d6652c Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 27 Mar 2023 03:59:02 +0000
Subject: [PATCH v5] Emit WAL record info via pg_get_wal_block_info

---
 .../pg_walinspect/pg_walinspect--1.0--1.1.sql | 12 ++-
 contrib/pg_walinspect/pg_walinspect.c         | 55 ++++++++---
 doc/src/sgml/pgwalinspect.sgml                | 99 ++++++++++++-------
 3 files changed, 117 insertions(+), 49 deletions(-)

diff --git a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
index 586c3b4467..f13d3a8fd3 100644
--- a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
+++ b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
@@ -12,12 +12,20 @@ DROP FUNCTION pg_get_wal_stats_till_end_of_wal(pg_lsn, boolean);
 --
 CREATE FUNCTION pg_get_wal_block_info(IN start_lsn pg_lsn,
 	IN end_lsn pg_lsn,
-	OUT lsn pg_lsn,
-	OUT blockid int2,
+    OUT start_lsn pg_lsn,
+    OUT end_lsn pg_lsn,
+    OUT prev_lsn pg_lsn,
 	OUT reltablespace oid,
 	OUT reldatabase oid,
 	OUT relfilenode oid,
 	OUT relblocknumber int8,
+	OUT blockid int2,
+    OUT xid xid,
+    OUT resource_manager text,
+    OUT record_type text,
+    OUT record_length int4,
+    OUT main_data_length int4,
+    OUT description text,
 	OUT forkname text,
 	OUT blockdata bytea,
 	OUT fpi bytea,
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index 90fb8eaed1..c8d3ece910 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -177,6 +177,10 @@ ReadNextXLogRecord(XLogReaderState *xlogreader)
 
 /*
  * Get a single WAL record info.
+ *
+ * Note that the per-record information fetching code is same in both
+ * GetWALBlockInfo() and GetWALRecordInfo(), it's only the output columns order
+ * that differs. Try to keep this code in sync.
  */
 static void
 GetWALRecordInfo(XLogReaderState *record, Datum *values,
@@ -229,18 +233,34 @@ GetWALRecordInfo(XLogReaderState *record, Datum *values,
 
 
 /*
- * Store a set of block information from a single record (FPI and block
- * information).
+ * Get a single WAL record info along with its block information (block data
+ * and FPI).
+ *
+ * Note that the per-record information fetching code is same in both
+ * GetWALBlockInfo() and GetWALRecordInfo(), it's only the output columns order
+ * that differs. Try to keep this code in sync.
  */
 static void
 GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
 {
-#define PG_GET_WAL_BLOCK_INFO_COLS 11
+#define PG_GET_WAL_BLOCK_INFO_COLS 19
 	int			block_id;
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+	RmgrData	desc;
+	const char *id;
+	StringInfoData rec_desc;
 
 	Assert(XLogRecHasAnyBlockRefs(record));
 
+	desc = GetRmgr(XLogRecGetRmid(record));
+	id = desc.rm_identify(XLogRecGetInfo(record));
+
+	if (id == NULL)
+		id = psprintf("UNKNOWN (%x)", XLogRecGetInfo(record) & ~XLR_INFO_MASK);
+
+	initStringInfo(&rec_desc);
+	desc.rm_desc(&rec_desc, record);
+
 	for (block_id = 0; block_id <= XLogRecMaxBlockId(record); block_id++)
 	{
 		DecodedBkpBlock *blk;
@@ -260,11 +280,23 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
 										  &rnode, &fork, &blkno, NULL);
 
 		values[i++] = LSNGetDatum(record->ReadRecPtr);
-		values[i++] = Int16GetDatum(block_id);
+		values[i++] = LSNGetDatum(record->EndRecPtr);
+		values[i++] = LSNGetDatum(XLogRecGetPrev(record));
 		values[i++] = ObjectIdGetDatum(blk->rlocator.spcOid);
 		values[i++] = ObjectIdGetDatum(blk->rlocator.dbOid);
 		values[i++] = ObjectIdGetDatum(blk->rlocator.relNumber);
 		values[i++] = Int64GetDatum((int64) blkno);
+		values[i++] = Int16GetDatum(block_id);
+		values[i++] = TransactionIdGetDatum(XLogRecGetXid(record));
+		values[i++] = CStringGetTextDatum(desc.rm_name);
+		values[i++] = CStringGetTextDatum(id);
+		values[i++] = UInt32GetDatum(XLogRecGetTotalLen(record));
+		values[i++] = UInt32GetDatum(XLogRecGetDataLen(record));
+
+		if (rec_desc.len > 0)
+			values[i++] = CStringGetTextDatum(rec_desc.data);
+		else
+			nulls[i++] = true;
 
 		if (fork >= 0 && fork <= MAX_FORKNUM)
 			values[i++] = CStringGetTextDatum(forkNames[fork]);
@@ -358,11 +390,12 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
 }
 
 /*
- * Get information about all the blocks saved in WAL records between start
- * and end LSNs. This produces information about the full page images with
- * their relation information, and the data saved in each block associated
- * to a record. Decompression is applied to the full page images, if
- * necessary.
+ * Get info of all WAL records having block references between start LSN and
+ * end LSN. It outputs one row for each WAL record's block reference (note that
+ * a single WAL record can have one or more blocks associated with it). It
+ * outputs WAL record information as well as block information (block data
+ * and/or FPI). Decompression is applied to the full page images, if necessary.
+ * It omits WAL records that contain no block references.
  */
 Datum
 pg_get_wal_block_info(PG_FUNCTION_ARGS)
@@ -486,7 +519,7 @@ ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
 }
 
 /*
- * Get info and data of all WAL records between start LSN and end LSN.
+ * Get info of all WAL records between start LSN and end LSN.
  */
 static void
 GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
@@ -538,7 +571,7 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
 }
 
 /*
- * Get info and data of all WAL records between start LSN and end LSN.
+ * Get info of all WAL records between start LSN and end LSN.
  */
 Datum
 pg_get_wal_records_info(PG_FUNCTION_ARGS)
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 9a0241a8d6..ffcb39bb27 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -94,9 +94,10 @@ block_ref        | blkref #0: rel 1663/5/60221 fork main blk 2
      <para>
       Gets information of all the valid WAL records between
       <replaceable>start_lsn</replaceable> and <replaceable>end_lsn</replaceable>.
-      Returns one row per WAL record. If a future
-      <replaceable>end_lsn</replaceable> (i.e. ahead of the current LSN of
-      the server) is specified, it returns information until the end of WAL.
+      Returns one row per WAL record. If <replaceable>end_lsn</replaceable>
+      specified is in future (i.e. ahead of the current LSN of the server) or
+      <literal>FFFFFFFF/FFFFFFFF</literal> (i.e. highest possible value for
+      <type>pg_lsn</type> type), it returns information until the end of WAL.
       The function raises an error if <replaceable>start_lsn</replaceable>
       is not available. For example, usage of the function is as follows:
 <screen>
@@ -133,11 +134,13 @@ block_ref        |
       <replaceable>end_lsn</replaceable>. By default, it returns one row per
       <replaceable>resource_manager</replaceable> type. When
       <replaceable>per_record</replaceable> is set to <literal>true</literal>,
-      it returns one row per <replaceable>record_type</replaceable>. If a
-      future <replaceable>end_lsn</replaceable> (i.e. ahead of the current
-      LSN of the server) is specified, it returns statistics until the end
-      of WAL. An error is raised if <replaceable>start_lsn</replaceable> is
-      not available. For example, usage of the function is as follows:
+      it returns one row per <replaceable>record_type</replaceable>.
+      If <replaceable>end_lsn</replaceable> specified is in future (i.e. ahead
+      of the current LSN of the server) or <literal>FFFFFFFF/FFFFFFFF</literal>
+      (i.e. highest possible value for <type>pg_lsn</type> type), it returns
+      statistics until the end of WAL. An error is raised if
+      <replaceable>start_lsn</replaceable> is not available. For example, usage
+      of the function is as follows:
 <screen>
 postgres=# SELECT * FROM pg_get_wal_stats('0/1E847D00', '0/1E84F500')
              WHERE count > 0 LIMIT 1 AND
@@ -164,38 +167,62 @@ combined_size_percentage     | 2.8634072910530795
 
     <listitem>
      <para>
-      Gets a copy of the block information stored in WAL records. This includes
-      copies of the block data (<literal>NULL</literal> if none) and full page
-      images as <type>bytea</type> values (after
-      applying decompression when necessary, or <literal>NULL</literal> if none)
-      and their information associated with all the valid WAL records between
-      <replaceable>start_lsn</replaceable> and
-      <replaceable>end_lsn</replaceable>. Returns one row per block registered
-      in a WAL record. If a future <replaceable>end_lsn</replaceable> (i.e.
-      ahead of the current LSN of the server) is specified, it returns
-      statistics until the end of WAL. An error is raised if
-      <replaceable>start_lsn</replaceable> is not available. For example,
-      usage of the function is as follows:
+      Gets both WAL record and block information of all the valid WAL records
+      between <replaceable>start_lsn</replaceable> and
+      <replaceable>end_lsn</replaceable>. It omits WAL records that contain no
+      block references. The block information includes copies of the block data
+      (<literal>NULL</literal> if none) and full page images as
+      <type>bytea</type> values (after applying decompression when necessary,
+      or <literal>NULL</literal> if none). Returns one row per block registered
+      in a WAL record. If <replaceable>end_lsn</replaceable> specified is in
+      future (i.e. ahead of the current LSN of the server) or
+      <literal>FFFFFFFF/FFFFFFFF</literal> (i.e. highest possible value for
+      <type>pg_lsn</type> type), it returns information until the end of WAL.
+      An error is raised if <replaceable>start_lsn</replaceable> is not
+      available. For example, usage of the function is as follows:
 <screen>
-postgres=# SELECT lsn, blockid, reltablespace, reldatabase, relfilenode,
-                  relblocknumber, forkname,
-                  substring(blockdata for 24) as block_trimmed,
+postgres=# SELECT start_lsn, end_lsn, prev_lsn, reltablespace,
+                  reldatabase, relfilenode, relblocknumber,
+                  blockid, xid, resource_manager, record_type,
+                  record_length, main_data_length, description,
+                  forkname, substring(blockdata for 24) as block_trimmed,
                   substring(fpi for 24) as fpi_trimmed, fpilen, fpiinfo
-             FROM pg_get_wal_block_info('0/1871080', '0/1871440');
--[ RECORD 1 ]--+---------------------------------------------------
-lsn            | 0/18712F8
-blockid        | 0
-reltablespace  | 1663
-reldatabase    | 16384
-relfilenode    | 16392
-relblocknumber | 0
-forkname       | main
-block_trimmed  | \x02800128180164000000
-fpi_trimmed    | \x0000000050108701000000002c00601f00200420e0020000
-fpilen         | 204
-fpiinfo        | {HAS_HOLE,APPLY}
+             FROM pg_get_wal_block_info('0/14BF938', '0/14BFBA0');
+-[ RECORD 1 ]----+---------------------------------------------------
+start_lsn        | 0/14BFA20
+end_lsn          | 0/14BFB38
+prev_lsn         | 0/14BF9A8
+reltablespace    | 1663
+reldatabase      | 5
+relfilenode      | 16391
+relblocknumber   | 9
+blockid          | 0
+xid              | 735
+resource_manager | Heap
+record_type      | HOT_UPDATE
+record_length    | 279
+main_data_length | 14
+description      | off 1 xmax 735 flags 0x10 ; new off 5 xmax 0
+forkname         | main
+block_trimmed    | \x02800128180102000000
+fpi_trimmed      | \x00000000d0f84b01000000002c00601f00200420df020000
+fpilen           | 204
+fpiinfo          | {HAS_HOLE,APPLY}
 </screen>
      </para>
+     <note>
+      <para>
+       Note that although <function>pg_get_wal_records_info</function>
+       and <function>pg_get_wal_block_info</function> functions return
+       information of WAL records in common, the
+       <function>pg_get_wal_records_info</function> outputs block information
+       (without block data and FPI data) as a text column
+       (<literal>NULL</literal> if none), and
+       <function>pg_get_wal_block_info</function> outputs one row for each block
+       (including block data and FPI data) associated with each WAL record, omitting
+       WAL records that contain no block references.
+      </para>
+     </note>
     </listitem>
    </varlistentry>
 
-- 
2.34.1

Reply via email to