public inbox for [email protected]  
help / color / mirror / Atom feed
From: Alexander Korotkov <[email protected]>
To: Amit Kapila <[email protected]>
Cc: Vitaly Davydov <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Alexander Lakhin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly
Date: Fri, 20 Jun 2025 03:18:20 +0300
Message-ID: <CAPpHfdvk5RxdKZuFDFgDet6ZAzVW0ojxP-pjjqZPFZUW2N5gEA@mail.gmail.com> (raw)
In-Reply-To: <CAA4eK1+qL9QFOD2Q5kcq0Ff=7OcBLE34QuDBdPcdznUzQwv+eg@mail.gmail.com>
References: <1d12d2-67235980-35-19a406a0@63439497>
	<CAPpHfdtp61pSt72KxxOnnV_Oj4OTAiKYCSTek0Pm87hVDod4Bg@mail.gmail.com>
	<CAA4eK1Lmd-stXWrNqX9iXugjY=ahhe7zRfyq+-vN512xvF4G2w@mail.gmail.com>
	<CAPpHfdsTiYQPywr4yeqVhUtA+eFdqisqcAOiTb+kiYhi9EVEvw@mail.gmail.com>
	<CAPpHfdvk-nfu5xzJyPArxWctofWPR+1L9uJ+yyWFA34n6b5rGQ@mail.gmail.com>
	<CAA4eK1Ladnm40UO-YC+c7eBYqjyFQSwg0dn1w_CiaMoBTsJUDA@mail.gmail.com>
	<CAPpHfduX--KKX853UwaJ8Cjo5bxbh19V_+McKQ3p9aS6b8T1Wg@mail.gmail.com>
	<CAA4eK1+VbBxYY-rG4B7Jq6TsYvdjt3d2nbUrs7p9-1sP5mH+sw@mail.gmail.com>
	<CAPpHfdvxqjE+RZdoOODQZAmQip6NpvSH40DuvVFWuf_00ek-yQ@mail.gmail.com>
	<1d97ec-6841b600-3-b7b4780@40747246>
	<CAA4eK1LvssAcp61qg957xRF0=K4xF0hfJQjQ6c9V7EGHSqraPg@mail.gmail.com>
	<28c8bf-68470780-3-51b29480@89454035>
	<CAPpHfdt77k3BqD=anh4b7UiRevfPOu2Hcf3rvSZXntxH+4G=Ug@mail.gmail.com>
	<[email protected]>
	<CAPpHfdvPpg5VUTNOqfKSNx66ij3ZJNZbf4kn6SDcPkdq9FTvng@mail.gmail.com>
	<[email protected]>
	<[email protected]>
	<CAPpHfdvJMe4i0_wp1tq2ZGSDKqUHKPwkfD_Y6KyJ3XQRFvc0cQ@mail.gmail.com>
	<OSCPR01MB1496667BEA93302F8903EE512F570A@OSCPR01MB14966.jpnprd01.prod.outlook.com>
	<CAPpHfdsdJ4YPTzH=cnany5_LjCNb_Z9e+Oucv53qYQHaHzJhag@mail.gmail.com>
	<CALDaNm3s-jpQTe1MshsvQ8GO=TLj233JCdkQ7uZ6pwqRVpxAdw@mail.gmail.com>
	<3f242e-68518380-11-78125f00@134027639>
	<CAPpHfds0ScFbExsT+pnY7K0JGH6pFPHMOSfCOknC-1s7ahmRFA@mail.gmail.com>
	<214ba-6852df80-9-7e0df800@134698799>
	<21b44-6852e000-3-a556820@87831171>
	<CAPpHfdv3UEUBjsLhB_CwJT0xX9LmN6U+__myYopq4KcgvCSbTg@mail.gmail.com>
	<CAA4eK1+qL9QFOD2Q5kcq0Ff=7OcBLE34QuDBdPcdznUzQwv+eg@mail.gmail.com>

On Thu, Jun 19, 2025 at 1:29 PM Amit Kapila <[email protected]> wrote:
> On Wed, Jun 18, 2025 at 10:17 PM Alexander Korotkov
> <[email protected]> wrote:
> >
> > On Wed, Jun 18, 2025 at 6:50 PM Vitaly Davydov <[email protected]> wrote:
> > > > I think, it is a good idea. Once we do not use the generated data, it is ok
> > > > just to generate WAL segments using the proposed function. I've tested this
> > > > function. The tests worked as expected with and without the fix. The attached
> > > > patch does the change.
> > >
> > > Sorry, forgot to attach the patch. It is created on the current master branch.
> > > It may conflict with your corrections. I hope, it could be useful.
> >
> > Thank you.  I've integrated this into a patch to improve these tests.
> >
> > Regarding assertion failure, I've found that assert in
> > PhysicalConfirmReceivedLocation() conflicts with restart_lsn
> > previously set by ReplicationSlotReserveWal().  As I can see,
> > ReplicationSlotReserveWal() just picks fresh XLogCtl->RedoRecPtr lsn.
> > So, it doesn't seems there is a guarantee that restart_lsn never goes
> > backward.  The commit in ReplicationSlotReserveWal() even states there
> > is a "chance that we have to retry".
> >
>
> I don't see how this theory can lead to a restart_lsn of a slot going
> backwards. The retry mentioned there is just a retry to reserve the
> slot's position again if the required WAL is already removed. Such a
> retry can only get the position later than the previous restart_lsn.

Yes, if retry is needed, then the new position must be later for sure.
What I mean is that ReplicationSlotReserveWal() can reserve something
later than what standby is going to read (and correspondingly report
with PhysicalConfirmReceivedLocation()).

> >  Thus, I propose to remove the
> > assertion introduced by ca307d5cec90.
> >
>
> If what I said above is correct, then the following part of the commit
> message will be incorrect:
> "As stated in the ReplicationSlotReserveWal() comment, this is not
> always true. Additionally, this issue has been spotted by some
> buildfarm
> members."

I agree, this comment needs improvement in terms of clarity.

Meanwhile I've pushed the patch for TAP tests, which I think didn't
get any objections.

------
Regards,
Alexander Korotkov
Supabase





view thread (73+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Slot's restart_lsn may point to removed WAL segment after hard restart unexpectedly
  In-Reply-To: <CAPpHfdvk5RxdKZuFDFgDet6ZAzVW0ojxP-pjjqZPFZUW2N5gEA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox