Horton

Thread

Commits

  1. Don't retreat slot's confirmed_flush LSN.

  1. Backward movement of confirmed_flush resulting in data duplication.

    shveta malik <shveta.malik@gmail.com> — 2025-05-13T10:17:55Z

    Hi All,

It is a spin-off thread from earlier discussions at [1] and [2].

While analyzing the slot-sync BF failure as stated in [1], it was
observed that there are chances that confirmed_flush_lsn may move
backward depending on the feedback messages received from the
downstream system. It was suspected that the backward movement of
confirmed_flush_lsn may result in data duplication issues. Earlier we
were able to successfully reproduce the issue with two_phase enabled
subscriptions (see[2]). Now on further analysing, it seems possible
that data duplication issues may happen without two-phase as well.

With the attached injection-point patch and test-script (provided by
Hou-San), the data duplication issue is reproduced without using the
twophase option. The problem arises when changes applied by the table
sync worker are duplicated by the apply worker if the
confirmed_flush_lsn moves backward after the table sync is completed.

The error expected on sub after executing the test script:
# ERROR:  conflict detected on relation "public.tab2": conflict=insert_exists
# DETAIL:  Key already exists in unique index "tab2_a_key", modified
in transaction ....
# Key (a)=(2); existing local tuple (2); remote tuple (2).

The general steps followed by attached script are:

1. After adding a new table, tab2, to the publication, refresh the
subscription to initiate a table sync for tab2. Before the state
reaches SYNCDONE, insert some data into tab2. This new insertion will
be replicated by the table sync worker.
2. Disable the subscription and stop the apply worker before changing
the state to READY.
3. Re-enable the subscription and wait for the table sync to finish.
Notice that the origin position should not have progressed since step
1.
4. Disable and re-enable the subscription. Given that the origin
position is less than the slot's confirmed_flush_lsn, control the
walsender to stop when the confirmed_flush_lsn moves backward.
5. Disable and re-enable the subscription once more, causing the slot
to retain the previous confirmed_flush_lsn, and the insertion from
step 2 will be replicated again.

The test script uses 3 injection points to control the race condition
mentioned in above steps. Also the LOG_SNAPSHOT_INTERVAL_MS is
increased to prevent the restart_lsn from increasing beyond the
insert. To fix this issue, we need to  prevent confirmed_flush_lsn
from moving backward. Attached the fix patch for the same.

With the given script, the problem reproduces on Head and PG17. We are
trying to reproduce the issue on PG16 and below where injection points
are not there.

[1]: https://www.postgresql.org/message-id/OS3PR01MB5718BC899AEE1D04755C6E7594832%40OS3PR01MB5718.jpnprd01.prod.outlook.com
[2]: https://www.postgresql.org/message-id/OS0PR01MB57164AB5716AF2E477D53F6F9489A%40OS0PR01MB5716.jpnprd01.prod.outlook.com

thanks
Shveta

  2. Re: Backward movement of confirmed_flush resulting in data duplication.

    Dilip Kumar <dilipbalaut@gmail.com> — 2025-05-13T10:52:39Z

    On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
>
> Hi All,
>
> It is a spin-off thread from earlier discussions at [1] and [2].
>
> While analyzing the slot-sync BF failure as stated in [1], it was
> observed that there are chances that confirmed_flush_lsn may move
> backward depending on the feedback messages received from the
> downstream system. It was suspected that the backward movement of
> confirmed_flush_lsn may result in data duplication issues. Earlier we
> were able to successfully reproduce the issue with two_phase enabled
> subscriptions (see[2]). Now on further analysing, it seems possible
> that data duplication issues may happen without two-phase as well.

Thanks for the detailed explanation. Before we focus on patching the
symptoms, I’d like to explore whether the issue can be addressed on
the subscriber side. Specifically, have we analyzed if there’s a way
to prevent the subscriber from moving the LSN backward in the first
place? That might lead to a cleaner and more robust solution overall.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  3. Re: Backward movement of confirmed_flush resulting in data duplication.

    Amit Kapila <amit.kapila16@gmail.com> — 2025-05-14T03:45:53Z

    On Tue, May 13, 2025 at 4:22 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > Hi All,
> >
> > It is a spin-off thread from earlier discussions at [1] and [2].
> >
> > While analyzing the slot-sync BF failure as stated in [1], it was
> > observed that there are chances that confirmed_flush_lsn may move
> > backward depending on the feedback messages received from the
> > downstream system. It was suspected that the backward movement of
> > confirmed_flush_lsn may result in data duplication issues. Earlier we
> > were able to successfully reproduce the issue with two_phase enabled
> > subscriptions (see[2]). Now on further analysing, it seems possible
> > that data duplication issues may happen without two-phase as well.
>
> Thanks for the detailed explanation. Before we focus on patching the
> symptoms, I’d like to explore whether the issue can be addressed on
> the subscriber side. Specifically, have we analyzed if there’s a way
> to prevent the subscriber from moving the LSN backward in the first
> place? That might lead to a cleaner and more robust solution overall.
>

The subscriber doesn't move the LSN backwards, it only shares the
information with the publisher, which is the latest value of remote
LSN tracked by the origin. Now, as explained in email [1], the
subscriber doesn't persistently store/advance the LSN, for which it
doesn't have to do anything like DDLs, or any other non-published
DMLs. However, subscribers need to send confirmation of such LSNs for
synchronous replication. This is commented in the code as well, see
comments in CreateDecodingContext (It might seem like we should error
out in this case, but it's pretty common for a client to acknowledge a
LSN it doesn't have to do anything for ...). As mentioned in email[1],
persisting the LSN information that the subscriber doesn't have to do
anything with could be a noticeable performance overhead.

I think it is better to deal with this in the publisher by not
allowing it to move confirm_flush LSN backwards, as Shveta proposed.

[1]: https://www.postgresql.org/message-id/CAA4eK1%2BzWQwOe5G8zCYGvErnaXh5%2BDbyg_A1Z3uywSf_4%3DT0UA%40mail.gmail.com

-- 
With Regards,
Amit Kapila.

  4. Re: Backward movement of confirmed_flush resulting in data duplication.

    Dilip Kumar <dilipbalaut@gmail.com> — 2025-05-14T06:29:00Z

    On Wed, May 14, 2025 at 9:16 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, May 13, 2025 at 4:22 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > Hi All,
> > >
> > > It is a spin-off thread from earlier discussions at [1] and [2].
> > >
> > > While analyzing the slot-sync BF failure as stated in [1], it was
> > > observed that there are chances that confirmed_flush_lsn may move
> > > backward depending on the feedback messages received from the
> > > downstream system. It was suspected that the backward movement of
> > > confirmed_flush_lsn may result in data duplication issues. Earlier we
> > > were able to successfully reproduce the issue with two_phase enabled
> > > subscriptions (see[2]). Now on further analysing, it seems possible
> > > that data duplication issues may happen without two-phase as well.
> >
> > Thanks for the detailed explanation. Before we focus on patching the
> > symptoms, I’d like to explore whether the issue can be addressed on
> > the subscriber side. Specifically, have we analyzed if there’s a way
> > to prevent the subscriber from moving the LSN backward in the first
> > place? That might lead to a cleaner and more robust solution overall.
> >
>
> The subscriber doesn't move the LSN backwards, it only shares the
> information with the publisher, which is the latest value of remote
> LSN tracked by the origin. Now, as explained in email [1], the
> subscriber doesn't persistently store/advance the LSN, for which it
> doesn't have to do anything like DDLs, or any other non-published
> DMLs. However, subscribers need to send confirmation of such LSNs for
> synchronous replication. This is commented in the code as well, see
> comments in CreateDecodingContext (It might seem like we should error
> out in this case, but it's pretty common for a client to acknowledge a
> LSN it doesn't have to do anything for ...). As mentioned in email[1],
> persisting the LSN information that the subscriber doesn't have to do
> anything with could be a noticeable performance overhead.

Thanks for your response.

What I meant wasn’t that the subscriber is moving the confirmed LSN
backward, nor was I suggesting we fix it by persisting the LSN on the
subscriber side. My point was: the fact that the subscriber is sending
an LSN older than one it has already sent, does that indicate a bug on
the subscriber side?  And if so, should the logic be fixed there?

I understand this might not be feasible, and it may not even be a bug
on the subscriber side, it could be an intentional part of the design.
But my question was whether we’ve already considered and ruled out
that possibility.

That said, I’m planning to dig deeper into the full sequence of steps
to understand exactly how this behavior is occurring. Hopefully, from
there, I might get a better idea of why the subscriber is doing that.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  5. Re: Backward movement of confirmed_flush resulting in data duplication.

    Alexander Kukushkin <cyberdemn@gmail.com> — 2025-05-14T06:42:42Z

    Hi Dilip,

On Wed, 14 May 2025 at 08:29, Dilip Kumar <dilipbalaut@gmail.com> wrote:

> What I meant wasn’t that the subscriber is moving the confirmed LSN
> backward, nor was I suggesting we fix it by persisting the LSN on the
> subscriber side. My point was: the fact that the subscriber is sending
> an LSN older than one it has already sent, does that indicate a bug on
> the subscriber side?  And if so, should the logic be fixed there?
>

In my experience, client applications do a lot of surprisingly not smart
things.
However, it doesn't mean that the server should be blindly accepting
whatever LSN client sends.
I tend to agree with Amit, we shouldn't allow confirmed_flush_lsn to move
backwards.

-- 
Regards,
--
Alexander Kukushkin

  6. Re: Backward movement of confirmed_flush resulting in data duplication.

    Amit Kapila <amit.kapila16@gmail.com> — 2025-05-14T06:45:07Z

    On Wed, May 14, 2025 at 11:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Wed, May 14, 2025 at 9:16 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Tue, May 13, 2025 at 4:22 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > >
> > > On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
> > > >
> > > > Hi All,
> > > >
> > > > It is a spin-off thread from earlier discussions at [1] and [2].
> > > >
> > > > While analyzing the slot-sync BF failure as stated in [1], it was
> > > > observed that there are chances that confirmed_flush_lsn may move
> > > > backward depending on the feedback messages received from the
> > > > downstream system. It was suspected that the backward movement of
> > > > confirmed_flush_lsn may result in data duplication issues. Earlier we
> > > > were able to successfully reproduce the issue with two_phase enabled
> > > > subscriptions (see[2]). Now on further analysing, it seems possible
> > > > that data duplication issues may happen without two-phase as well.
> > >
> > > Thanks for the detailed explanation. Before we focus on patching the
> > > symptoms, I’d like to explore whether the issue can be addressed on
> > > the subscriber side. Specifically, have we analyzed if there’s a way
> > > to prevent the subscriber from moving the LSN backward in the first
> > > place? That might lead to a cleaner and more robust solution overall.
> > >
> >
> > The subscriber doesn't move the LSN backwards, it only shares the
> > information with the publisher, which is the latest value of remote
> > LSN tracked by the origin. Now, as explained in email [1], the
> > subscriber doesn't persistently store/advance the LSN, for which it
> > doesn't have to do anything like DDLs, or any other non-published
> > DMLs. However, subscribers need to send confirmation of such LSNs for
> > synchronous replication. This is commented in the code as well, see
> > comments in CreateDecodingContext (It might seem like we should error
> > out in this case, but it's pretty common for a client to acknowledge a
> > LSN it doesn't have to do anything for ...). As mentioned in email[1],
> > persisting the LSN information that the subscriber doesn't have to do
> > anything with could be a noticeable performance overhead.
>
> Thanks for your response.
>
> What I meant wasn’t that the subscriber is moving the confirmed LSN
> backward, nor was I suggesting we fix it by persisting the LSN on the
> subscriber side. My point was: the fact that the subscriber is sending
> an LSN older than one it has already sent, does that indicate a bug on
> the subscriber side?  And if so, should the logic be fixed there?
>
> I understand this might not be feasible, and it may not even be a bug
> on the subscriber side, it could be an intentional part of the design.
>

Right, it is how currently the subscriber/publisher communication is designed.

> But my question was whether we’ve already considered and ruled out
> that possibility.
>

That is what I explained in my previous response. Basically, to
achieve what you are saying, we need to persist the remote LSN values
by advancing the origin for cases, even when the subscriber doesn't
need to apply such changes like DDLs.

-- 
With Regards,
Amit Kapila.

  7. Re: Backward movement of confirmed_flush resulting in data duplication.

    Dilip Kumar <dilipbalaut@gmail.com> — 2025-05-14T09:15:53Z

    On Wed, May 14, 2025 at 12:15 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, May 14, 2025 at 11:59 AM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, May 14, 2025 at 9:16 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> > >
> > > On Tue, May 13, 2025 at 4:22 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> > > >
> > > > On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
> > > > >
> > > > > Hi All,
> > > > >
> > > > > It is a spin-off thread from earlier discussions at [1] and [2].
> > > > >
> > > > > While analyzing the slot-sync BF failure as stated in [1], it was
> > > > > observed that there are chances that confirmed_flush_lsn may move
> > > > > backward depending on the feedback messages received from the
> > > > > downstream system. It was suspected that the backward movement of
> > > > > confirmed_flush_lsn may result in data duplication issues. Earlier we
> > > > > were able to successfully reproduce the issue with two_phase enabled
> > > > > subscriptions (see[2]). Now on further analysing, it seems possible
> > > > > that data duplication issues may happen without two-phase as well.
> > > >
> > > > Thanks for the detailed explanation. Before we focus on patching the
> > > > symptoms, I’d like to explore whether the issue can be addressed on
> > > > the subscriber side. Specifically, have we analyzed if there’s a way
> > > > to prevent the subscriber from moving the LSN backward in the first
> > > > place? That might lead to a cleaner and more robust solution overall.
> > > >
> > >
> > > The subscriber doesn't move the LSN backwards, it only shares the
> > > information with the publisher, which is the latest value of remote
> > > LSN tracked by the origin. Now, as explained in email [1], the
> > > subscriber doesn't persistently store/advance the LSN, for which it
> > > doesn't have to do anything like DDLs, or any other non-published
> > > DMLs. However, subscribers need to send confirmation of such LSNs for
> > > synchronous replication. This is commented in the code as well, see
> > > comments in CreateDecodingContext (It might seem like we should error
> > > out in this case, but it's pretty common for a client to acknowledge a
> > > LSN it doesn't have to do anything for ...). As mentioned in email[1],
> > > persisting the LSN information that the subscriber doesn't have to do
> > > anything with could be a noticeable performance overhead.
> >
> > Thanks for your response.
> >
> > What I meant wasn’t that the subscriber is moving the confirmed LSN
> > backward, nor was I suggesting we fix it by persisting the LSN on the
> > subscriber side. My point was: the fact that the subscriber is sending
> > an LSN older than one it has already sent, does that indicate a bug on
> > the subscriber side?  And if so, should the logic be fixed there?
> >
> > I understand this might not be feasible, and it may not even be a bug
> > on the subscriber side, it could be an intentional part of the design.
> >
>
> Right, it is how currently the subscriber/publisher communication is designed.
>
> > But my question was whether we’ve already considered and ruled out
> > that possibility.
> >
>
> That is what I explained in my previous response. Basically, to
> achieve what you are saying, we need to persist the remote LSN values
> by advancing the origin for cases, even when the subscriber doesn't
> need to apply such changes like DDLs.

Understood, yeah, it makes sense to fix the way Shveta has fixed.
Sorry for the noise.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

  8. Re: Backward movement of confirmed_flush resulting in data duplication.

    Nisha Moond <nisha.moond412@gmail.com> — 2025-05-16T12:08:46Z

    Hi,

On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
>
>
> With the given script, the problem reproduces on Head and PG17. We are
> trying to reproduce the issue on PG16 and below where injection points
> are not there.
>

The issue can also be reproduced on PostgreSQL versions 13 through 16.

The same steps shared earlier in the
'reproduce_data_duplicate_without_twophase.sh' script can be used to
reproduce the issue on versions PG14 to PG16.

Since back branches do not support injection points, you can add infinite
loops at the locations where the patch
'v1-0001-Injection-points-to-reproduce-the-confirmed_flush.patch introduces
injection points'. These loops allow holding and releasing processes using
a debugger when needed.

Attached are detailed documents describing the reproduction steps:
 1) Use 'reproduce_steps_for_pg14_to_16.txt' for PG14 to PG16.
 2) Use 'reproduce_steps_for_pg13.txt' for PG13.

Note: PG13 uses temporary replication slots for tablesync workers, unlike
later versions that use permanent slots. Because of this difference, some
debugger-related steps differ slightly in PG13, which is why a separate
document is provided for it.

--
Thanks,
Nisha

  9. Re: Backward movement of confirmed_flush resulting in data duplication.

    Amit Kapila <amit.kapila16@gmail.com> — 2025-05-19T09:43:19Z

    On Fri, May 16, 2025 at 5:39 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> Hi,
>
> On Tue, May 13, 2025 at 3:48 PM shveta malik <shveta.malik@gmail.com> wrote:
> >
> >
> > With the given script, the problem reproduces on Head and PG17. We are
> > trying to reproduce the issue on PG16 and below where injection points
> > are not there.
> >
>
> The issue can also be reproduced on PostgreSQL versions 13 through 16.
>

Thanks. I have pushed the fix.

-- 
With Regards,
Amit Kapila.