From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexander Korotkov <aekorotkov@gmail.com>
cc: Melanie Plageman <melanieplageman@gmail.com>,
        pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Improve runtime and output of tests for replication slots
 checkp
In-reply-to: 
 <CAPpHfdurV-j_e0pb=UFENAy3tyzxfF+yHveNDNQk2gM82WBU5A@mail.gmail.com>
References: <E1uSOip-002TUa-0p@gemulon.postgresql.org>
 <CAAKRu_ZCOzQpEumLFgG_+iw3FTa+hJ4SRpxzaQBYxxM_ZAzWcA@mail.gmail.com>
 <2542023.1750456985@sss.pgh.pa.us> <2585993.1750458298@sss.pgh.pa.us>
 <CAPpHfdsedYNmKDnDuueJor2k8kA+hgEJKFJRf4PhDowdP_A4Bg@mail.gmail.com>
 <CAPpHfdtmLNpswWzk2qQ7-nMBLod2GrRWwMvfb0hc7tQfDD--ZQ@mail.gmail.com>
 <2594452.1750462958@sss.pgh.pa.us>
 <CAPpHfdurV-j_e0pb=UFENAy3tyzxfF+yHveNDNQk2gM82WBU5A@mail.gmail.com>
Comments: In-reply-to Alexander Korotkov <aekorotkov@gmail.com>
	message dated "Sun, 22 Jun 2025 03:16:10 +0300"
MIME-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Content-ID: <2852528.1750553810.1@sss.pgh.pa.us>
Content-Transfer-Encoding: 8bit
Date: Sat, 21 Jun 2025 20:56:50 -0400
Message-ID: <2852529.1750553810@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/2852529.1750553810%40sss.pgh.pa.us>
Precedence: bulk

Alexander Korotkov <aekorotkov@gmail.com> writes:
> On Sat, Jun 21, 2025 at 2:42 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> But "Wait for the next page to become available" seems awfully
>> trusting that there will be another page.  Should this be
>> using the no-wait code path?

> Thank you for the help.  It seems to me that problem is deeper.  The
> code seems to only trying to read till the end of given WAL record,
> but can't reach it.  According to the values I've seen in XLogCtl, it
> seems that RedoRecPtr points somewhere inside of that record's body.
> I don't feel confident about to understand what's going on and how to
> fix it.

Hmm.  My theory about what's happening is that we are writing a WAL
record that spans across a page boundary, and the asynchronous
immediate-stop request comes in and kills that operation, so that
the first half of the record is on disk but the second is not.
(This outcome would obviously be very timing-dependent.)  Then
we restart the server, and crash recovery copes with that situation
successfully.  But pg_logical_slot_get_changes() fails to, because
it's coded in such a way that it will wait forever for the second
half of the WAL record to appear.

The main hole in this theory is: if crash recovery thought that the
partial record was invalid, wouldn't it set the end-of-WAL position
to the start of that record?  And shouldn't that stop
pg_logical_slot_get_changes() from trying to process the partial
record?  So I'm sure there's some details not quite right about
this picture.  But maybe it's within hailing distance of the truth.

> The new tests just spotted the existing bug.

Yeah, that's what I think too.  The unintentional omission of a
pre-shutdown delay in the 046 test has exposed some pre-existing
fragility in pg_logical_slot_get_changes().  So I'm not in favor
of changing 046 till we understand this better.

			regards, tom lane