Backward movement of confirmed_flush resulting in data duplication.

Started by shveta malik8 months ago9 messages

shveta.malik@gmail.com

8 months ago

3 attachment(s)

Hi All,

It is a spin-off thread from earlier discussions at [1]/messages/by-id/OS3PR01MB5718BC899AEE1D04755C6E7594832@OS3PR01MB5718.jpnprd01.prod.outlook.com and [2]/messages/by-id/OS0PR01MB57164AB5716AF2E477D53F6F9489A@OS0PR01MB5716.jpnprd01.prod.outlook.com.

While analyzing the slot-sync BF failure as stated in [1]/messages/by-id/OS3PR01MB5718BC899AEE1D04755C6E7594832@OS3PR01MB5718.jpnprd01.prod.outlook.com, 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]/messages/by-id/OS0PR01MB57164AB5716AF2E477D53F6F9489A@OS0PR01MB5716.jpnprd01.prod.outlook.com). 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]: /messages/by-id/OS3PR01MB5718BC899AEE1D04755C6E7594832@OS3PR01MB5718.jpnprd01.prod.outlook.com
[2]: /messages/by-id/OS0PR01MB57164AB5716AF2E477D53F6F9489A@OS0PR01MB5716.jpnprd01.prod.outlook.com

thanks
Shveta

Attachments:

reproduce_data_duplicate_without_twophase.shtext/x-sh; charset=US-ASCII; name=reproduce_data_duplicate_without_twophase.shDownload

v1-0001-Injection-points-to-reproduce-the-confirmed_flush.patchapplication/octet-stream; name=v1-0001-Injection-points-to-reproduce-the-confirmed_flush.patchDownload

From 8e083d7e7f34039fbedc5d306d088eca4744f6df Mon Sep 17 00:00:00 2001
From: Zhijie Hou <houzj.fnst@cn.fujitsu.com>
Date: Thu, 8 May 2025 10:53:53 +0800
Subject: [PATCH v1] Injection points to reproduce the confirmed_flush issue.

---
 src/backend/postmaster/bgwriter.c           |  2 +-
 src/backend/replication/logical/logical.c   | 11 +++++++++++
 src/backend/replication/logical/tablesync.c |  3 +++
 src/backend/replication/logical/worker.c    |  6 ++++++
 src/backend/replication/walsender.c         |  4 ++++
 5 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 72f5acceec7..3bbe8ff6ab2 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -67,7 +67,7 @@ int			BgWriterDelay = 200;
  * Interval in which standby snapshots are logged into the WAL stream, in
  * milliseconds.
  */
-#define LOG_SNAPSHOT_INTERVAL_MS 15000
+#define LOG_SNAPSHOT_INTERVAL_MS 1500000
 
 /*
  * LSN and timestamp at which we last issued a LogStandbySnapshot(), to avoid
diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index a8d2e024d34..ff498c6b539 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -1828,6 +1828,11 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
 
 		SpinLockAcquire(&MyReplicationSlot->mutex);
 
+		if (lsn < MyReplicationSlot->data.confirmed_flush)
+			elog(LOG, "confirmed_flush moved backwards from %X/%X to %X/%X",
+				LSN_FORMAT_ARGS(MyReplicationSlot->data.confirmed_flush),
+				LSN_FORMAT_ARGS(lsn));
+
 		MyReplicationSlot->data.confirmed_flush = lsn;
 
 		/* if we're past the location required for bumping xmin, do so */
@@ -1893,6 +1898,12 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
 	else
 	{
 		SpinLockAcquire(&MyReplicationSlot->mutex);
+
+		if (lsn < MyReplicationSlot->data.confirmed_flush)
+			elog(LOG, "confirmed_flush moved backwards from %X/%X to %X/%X",
+				LSN_FORMAT_ARGS(MyReplicationSlot->data.confirmed_flush),
+				LSN_FORMAT_ARGS(lsn));
+
 		MyReplicationSlot->data.confirmed_flush = lsn;
 		SpinLockRelease(&MyReplicationSlot->mutex);
 	}
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index 8e1e8762f62..f0c2cad40a4 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -122,6 +122,7 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/usercontext.h"
+#include "utils/injection_point.h"
 
 typedef enum
 {
@@ -1551,6 +1552,8 @@ copy_table_done:
 		 "LogicalRepSyncTableStart: '%s' origin_startpos lsn %X/%X",
 		 originname, LSN_FORMAT_ARGS(*origin_startpos));
 
+	if (MyLogicalRepWorker->relid % 2 == 0)
+		INJECTION_POINT("table-sync-wait-2", NULL);
 	/*
 	 * We are done with the initial data synchronization, update the state.
 	 */
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 4151a4b2a96..9fc967b8463 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -192,6 +192,7 @@
 #include "utils/snapmgr.h"
 #include "utils/syscache.h"
 #include "utils/usercontext.h"
+#include "utils/injection_point.h"
 
 #define NAPTIME_PER_CYCLE 1000	/* max sleep time between cycles (1s) */
 
@@ -2451,6 +2452,8 @@ apply_handle_insert(StringInfo s)
 	slot_fill_defaults(rel, estate, remoteslot);
 	MemoryContextSwitchTo(oldctx);
 
+	elog(LOG, "handle remote insert");
+
 	/* For a partitioned table, insert the tuple into a partition. */
 	if (rel->localrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 		apply_handle_tuple_routing(edata,
@@ -3725,6 +3728,9 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
 			 * now.
 			 */
 			AcceptInvalidationMessages();
+
+			if (am_leader_apply_worker())
+				INJECTION_POINT("reread-sub", NULL);
 			maybe_reread_subscription();
 
 			/* Process any table synchronization changes. */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9fa8beb6103..f41fc398f9f 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -95,6 +95,7 @@
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
 #include "utils/timestamp.h"
+#include "utils/injection_point.h"
 
 /* Minimum interval used by walsender for stats flushes, in ms */
 #define WALSENDER_STATS_FLUSH_INTERVAL         1000
@@ -2810,6 +2811,9 @@ WalSndLoop(WalSndSendDataCallback send_data)
 			SyncRepInitConfig();
 		}
 
+		if (send_data == XLogSendLogical)
+			INJECTION_POINT("process-replies", NULL);
+
 		/* Check for input from the client */
 		ProcessRepliesIfAny();
 
-- 
2.34.1

v1-0001-Fix-confirmed_flush-backward-movement-issue.patchapplication/octet-stream; name=v1-0001-Fix-confirmed_flush-backward-movement-issue.patchDownload

From 3df43004cf98e50f1866250d9e877a880e961891 Mon Sep 17 00:00:00 2001
From: Shveta Malik <shveta.malik@gmail.com>
Date: Tue, 13 May 2025 15:16:32 +0530
Subject: [PATCH v1] Fix confirmed_flush backward movement issue.

This patch prevents moving the confirmed_flush backwards, as it
could lead to data duplication issues caused by the replay of already
replicated changes.
---
 src/backend/replication/logical/logical.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c
index a8d2e024d34..242e04b4a15 100644
--- a/src/backend/replication/logical/logical.c
+++ b/src/backend/replication/logical/logical.c
@@ -1828,7 +1828,13 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
 
 		SpinLockAcquire(&MyReplicationSlot->mutex);
 
-		MyReplicationSlot->data.confirmed_flush = lsn;
+		/*
+		 * Prevent moving the confirmed_flush backwards, as this could lead to
+		 * data duplication issues caused by the replay of already replicated
+		 * changes.
+		 */
+		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 &&
@@ -1893,7 +1899,15 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
 	else
 	{
 		SpinLockAcquire(&MyReplicationSlot->mutex);
-		MyReplicationSlot->data.confirmed_flush = lsn;
+
+		/*
+		 * Prevent moving the confirmed_flush backwards, as this could lead to
+		 * data duplication issues caused by the replay of already replicated
+		 * changes.
+		 */
+		if (lsn > MyReplicationSlot->data.confirmed_flush)
+			MyReplicationSlot->data.confirmed_flush = lsn;
+
 		SpinLockRelease(&MyReplicationSlot->mutex);
 	}
 }
-- 
2.34.1

Dilip Kumar

dilipbalaut@gmail.com

8 months ago

In reply to: shveta malik (#1)

Re: Backward movement of confirmed_flush resulting in data duplication.

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

Amit Kapila

amit.kapila16@gmail.com

8 months ago

In reply to: Dilip Kumar (#2)

Re: Backward movement of confirmed_flush resulting in data duplication.

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]/messages/by-id/CAA4eK1+zWQwOe5G8zCYGvErnaXh5+Dbyg_A1Z3uywSf_4=T0UA@mail.gmail.com, 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]/messages/by-id/CAA4eK1+zWQwOe5G8zCYGvErnaXh5+Dbyg_A1Z3uywSf_4=T0UA@mail.gmail.com,
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]: /messages/by-id/CAA4eK1+zWQwOe5G8zCYGvErnaXh5+Dbyg_A1Z3uywSf_4=T0UA@mail.gmail.com

--
With Regards,
Amit Kapila.

Dilip Kumar

dilipbalaut@gmail.com

8 months ago

In reply to: Amit Kapila (#3)

Re: Backward movement of confirmed_flush resulting in data duplication.

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

Alexander Kukushkin

cyberdemn@gmail.com

8 months ago

In reply to: Dilip Kumar (#4)

Re: Backward movement of confirmed_flush resulting in data duplication.

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

Amit Kapila

amit.kapila16@gmail.com

8 months ago

In reply to: Dilip Kumar (#4)

Re: Backward movement of confirmed_flush resulting in data duplication.

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.

Dilip Kumar

dilipbalaut@gmail.com

8 months ago

In reply to: Amit Kapila (#6)

Re: Backward movement of confirmed_flush resulting in data duplication.

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

Nisha Moond

nisha.moond412@gmail.com

8 months ago

In reply to: shveta malik (#1)

2 attachment(s)

Re: Backward movement of confirmed_flush resulting in data duplication.

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

Amit Kapila

amit.kapila16@gmail.com

8 months ago

In reply to: Nisha Moond (#8)

Re: Backward movement of confirmed_flush resulting in data duplication.

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.