On Friday, September 15, 2023 8:33 PM Kuroda, Hayato/黒田 隼人 <kuroda.hayato@fujitsu.com> wrote:
>
>
> Also, I did a self-reviewing again and reworded comments.
>
> BTW, the 0002 ports some functions from pg_walinspect, it may be not
> elegant.
> Coupling degree between core/extensions should be also lower. So I made
> another patch which does not port anything and implements similar
> functionalities instead.
> I called the patch 0003, but can be applied atop 0001 (not 0002). To make cfbot
> happy, attached as txt file.
> Could you please tell me which do you like 0002 or 0003?
I think basically it's OK that we follow the same method as pg_walinspect to
read the WAL. The reasons are as follows:
There are currently two set of APIs that are used to read WALs.
a) XLogReaderAllocate()/XLogReadRecord() -- pg_walinspect and current patch uses
b) XLogReaderAllocate()/WALRead()
The first setup APIs is easier to use and are used in most of WAL reading
codes, while the second set of APIs is used more in low level places and is not
very easy to use. So I think it's better to use the first set of APIs.
Besides, our function needs to distinguish the failure and end-of-wal cases
when XLogReadRecord() returns NULL and to read the wal without waiting. So, the
WAL reader callbacks in pg_walinspect also meets this requirement which is reason that
I think we can follow the same. I also checked other public wal reader callbacks but
they either report ERRORs if XLogReadRecord() returns NULL or will wait while
reading wals.
If we agree to follow the same method of pg_walinspect, I think the left
thing is whether to port some functions like what 0002. I personally
think it's fine to make common functions to save codes.
Best Regards,
Hou zj