On Friday, September 15, 2023 8:33 PM Kuroda, Hayato/黒田 隼人 <kuroda.hay...@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