Re: confirmed flush lsn seems to be move backward in certain error cases - Mailing list pgsql-hackers

From vignesh C
Subject Re: confirmed flush lsn seems to be move backward in certain error cases
Date
Msg-id CALDaNm0LLLvBjDK4z71kC8bonpq2xz=JODO6Lrek3Yqvu-KVKQ@mail.gmail.com
Whole thread Raw
In response to Re: confirmed flush lsn seems to be move backward in certain error cases  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: confirmed flush lsn seems to be move backward in certain error cases
List pgsql-hackers
On Mon, 10 Jun 2024 at 16:38, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Feb 20, 2024 at 12:35 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Sat, 17 Feb 2024 at 12:03, Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > >
> > > @@ -1839,7 +1839,8 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
> > >
> > >   SpinLockAcquire(&MyReplicationSlot->mutex);
> > >
> > > - MyReplicationSlot->data.confirmed_flush = lsn;
> > > + if (lsn > MyReplicationSlot->data.confirmed_flush)
> > > + MyReplicationSlot->data.confirmed_flush = lsn;
> > >
> > >   /* if we're past the location required for bumping xmin, do so */
> > >   if (MyReplicationSlot->candidate_xmin_lsn != InvalidXLogRecPtr &&
> > > @@ -1904,7 +1905,8 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
> > >   else
> > >   {
> > >   SpinLockAcquire(&MyReplicationSlot->mutex);
> > > - MyReplicationSlot->data.confirmed_flush = lsn;
> > > + if (lsn > MyReplicationSlot->data.confirmed_flush)
> > > + MyReplicationSlot->data.confirmed_flush = lsn;
> > >
> > > BTW, from which code path does it update the prior value of
> > > confirmed_flush?
> >
> > The confirmed_flush is getting set in the else condition for this scenario.
> >
> > If it is through the else check, then can we see if
> > > it may change the confirm_flush to the prior position via the first
> > > code path? I am asking because in the first code path, we can even
> > > flush the re-treated value of confirm_flush LSN.
> >
> > I was not able to find any scenario to set a prior position with the
> > first code path. I tried various scenarios like adding delay in
> > walsender, add delay in apply worker, restart the instances and with
> > various DML operations. It was always setting it to either to the same
> > value as previous or greater value.
> >
>
> Fair enough. This means that in the prior versions, it was never
> possible to move confirmed_flush LSN in the slot to a backward
> position on the disk. So, moving it backward temporarily (in the
> memory) shouldn't create any problem. I would prefer your
> Assert_confirmed_flush_will_always_not_be_less_than_last_saved_confirmed_flush.patch
> to fix this issue.

I have re-verified  the issue by running the tests in a loop of 150
times and found it to be working fine. Also patch applies neatly,
there was no pgindent issue and all the regression/tap tests run were
successful.

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: Ilia Evdokimov
Date:
Subject: Re: list_free in addRangeTableEntryForJoin
Next
From: Shlok Kyal
Date:
Subject: Re: 001_rep_changes.pl fails due to publisher stuck on shutdown