Date: Fri, 11 Mar 2022 11:52:49 +0900 (JST)
Message-Id: <20220311.115249.268200339858571765.horikyota.ntt@gmail.com>
To: bharath.rupireddyforpostgres@gmail.com
Cc: pgsql@j-davis.com, ashu.coek88@gmail.com, andrew@dunslane.net,
 robertmhaas@gmail.com, stark@mit.edu, schneider@ardentperf.com,
 bruce@momjian.us, pgsql-hackers@lists.postgresql.org,
 satyanarlapuram@gmail.com, marvin_liang@qq.com, actyzhang@outlook.com
Subject: Re: pg_walinspect - a new extension to get raw WAL data and WAL
 stats
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20220311.113822.1784045072426345189.horikyota.ntt@gmail.com>
References: <39c13407c8b7311e9a3b24f2df814696bfa2ebff.camel@j-davis.com>
	<CALj2ACWhcbW_s4BXLyCpLWcCppZN9ncTXHbn4dv1F0Vpe0kxqA@mail.gmail.com>
	<20220311.113822.1784045072426345189.horikyota.ntt@gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20220311.115249.268200339858571765.horikyota.ntt%40gmail.com>
Precedence: bulk

Sorry, some minor non-syntactical corrections.

At Fri, 11 Mar 2022 11:38:22 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> I played with this a bit, and would like to share some thoughts on it.
> 
> It seems to me too rigorous that pg_get_wal_records_info/stats()
> reject future LSNs as end-LSN and I think WARNING or INFO and stop at
> the real end-of-WAL is more kind to users.  I think the same with the
> restriction that start and end LSN are required to be different.
> 
> The definition of end-lsn is fuzzy here.  If I fed a future LSN to the
> functions, they tell me the beginning of the current insertion point
> in error message.  On the other hand they don't accept the same
> value as end-LSN.  I think it is right that they tell the current
> insertion point and they should take the end-LSN as the LSN to stop
> reading.
> 
> I think pg_get_wal_stats() is worth to have but I think it should be
> implemented in SQL.  Currently pg_get_wal_records_info() doesn't tell
> about FPI since pg_waldump doesn't but it is internally collected (of
> course!) and easily revealed. If we do that, the
> pg_get_wal_records_stats() would be reduced to the following SQL
> statement
> 
> SELECT resource_manager resmgr,
>        count(*) AS N,
> 	   (count(*) * 100 / sum(count(*)) OVER tot)::numeric(5,2) AS "%N",
> 	   sum(total_length) AS "combined size",
> 	   (sum(total_length) * 100 / sum(sum(total_length)) OVER tot)::numeric(5,2) AS "%combined size",
> 	   sum(fpi_len) AS fpilen,
> 	   (sum(fpi_len) * 100 / sum(sum(fpi_len)) OVER tot)::numeric(5,2) AS "%fpilen"
> 	   FROM pg_get_wal_records_info('0/1000000', '0/175DD7f')
>    	   GROUP by resource_manager
> 	   WINDOW tot AS ()
> 	   ORDER BY "combined size" desc;
> 
> The only difference with pg_waldump is the statement above doesn't
> show lines for the resource managers that don't contained in the
> result of pg_get_wal_records_info(). But I don't think that matters.
> 
> 
> Sometimes the field description has very long (28kb long) content. It
> makes the result output almost unreadable and I had a bit hard time
> struggling with the output full of '-'s.  I would like have a default
> limit on the length of such fields that can be long but I'm not sure
> we want that.
> 
> 
- The difference between pg_get_wal_record_info and _records_ other than
- the number of argument is the former accepts incorrect LSNs.

The discussion is somewhat confused after some twists and turns..  It
should be something like the following.

pg_get_wal_record_info and pg_get_wal_records_info are almost same
since the latter can show a single record.  However it is a bit
annoying to do that. Since, other than it doens't accept same LSNs for
start and end, it doesn't show a record when there' no record in the
specfied LSN range.  But I don't think there's no usefulness of the
behavior.

The following works,
 pg_get_wal_record_info('0/1000000');
 pg_get_wal_records_info('0/1000000');

but this doesn't
 pg_get_wal_records_info('0/1000000', '0/1000000');
> ERROR:  WAL start LSN must be less than end LSN

And the following shows no records.
 pg_get_wal_records_info('0/1000000', '0/1000001');
 pg_get_wal_records_info('0/1000000', '0/1000028');

But the following works
  pg_get_wal_records_info('0/1000000', '0/1000029');
> 0/1000028 | 0/0      |   0


> So I think we can consolidate the two functions as:
> 
> - pg_get_wal_records_info('0/1000000');
> 
>   (current behavior) find the first record and show all records
>   thereafter.
> 
> - pg_get_wal_records_info('0/1000000', '0/1000000');
> 
>   finds the first record since the start lsn and show it.
> 
> - pg_get_wal_records_info('0/1000000', '0/1000030');
> 
>   finds the first record since the start lsn then show records up to
>   the end-lsn.
> 
> 
> And about pg_get_raw_wal_record(). I don't see any use-case of the
> function alone on SQL interface.  Even if we need to inspect broken
> WAL files, it needs profound knowledge of WAL format and tools that
> doesn't work on SQL interface.
> 
> However like pageinspect, if we separate the WAL-record fetching and
> parsing it could be thought as useful.
> 
> pg_get_wal_records_info woule be like:
> 
> SELECT * FROM pg_walinspect_parse(raw)
>  FROM (SELECT * FROM pg_walinspect_get_raw(start_lsn, end_lsn));
> 
> And pg_get_wal_stats woule be like:
> 
> SELECT * FROM pg_walinpect_stat(pg_walinspect_parse(raw))
>  FROM (SELECT * FROM pg_walinspect_get_raw(start_lsn, end_lsn)));


Regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center