Re: pg_logical_slot_get_changes waits continously for a partial WAL record spanning across 2 pages

On Thu, Jun 26, 2025 at 05:25:42PM +0530, vignesh C wrote:
> On Thu, 26 Jun 2025 at 06:22, Michael Paquier <michael@paquier.xyz> wrote:
>> So you are suggesting the addition of an extra ReadPageInternal() that
>> forces a read of only the read, perform the checks on the header, then
>> read the rest.  After reading SizeOfXLogShortPHD worth of data,
>> shouldn't the checks on xlp_rem_len be done a bit earlier than what
>> you are proposing in this patch?
>
> Modified

It seems to me that this assert can be moved after the second page
read:
Assert(SizeOfXLogShortPHD <= readOff);

Coming back to the point of Kuroda-san about performance, did you do
some checks related to that and did you measure any difference?  I
suspect none of that because in most cases we are just going to fetch
the next page and we would trigger the fast-exit path of
ReadPageInternal() on the second call when fetching the rest.  I still
need to get an idea of all that by myself, probably with various
lengths of logical message records.

Perhaps this code could be improved in the future with less page
reads.  Anyway, what you are doing here is simple enough that it is a
no-brainer for the back-branches because we are just forcing our way
through with a new short header validation, so logically that's sound
as far as I can see.
--
Michael