public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: Hayato Kuroda (Fujitsu) <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Kyotaro Horiguchi <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Takamichi Osumi (Fujitsu) <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
Date: Tue, 28 Feb 2023 14:05:52 +0530
Message-ID: <CAA4eK1JeBycTHw6rG_Db_9xNWEuK1kCpEAG-2UoMGeaU6TTqaw@mail.gmail.com> (raw)
In-Reply-To: <TYCPR01MB5870CCDE096B4A41F53087A8F5AF9@TYCPR01MB5870.jpnprd01.prod.outlook.com>
References: <TYAPR01MB5866A9A39100EDA85E612B3EF5A39@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB5866A2D600A86F743DDF8FE5F5A09@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<[email protected]>
	<TYAPR01MB5866F00191375D0193320A4DF5A19@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CAA4eK1K4uPbudrNdH+=_vN-Hpe9wYh=3vBS5Ww9dHn-LOWMV0g@mail.gmail.com>
	<TYAPR01MB5866504C73ACCF98E0E09B11F5A49@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CAHut+Pu6Y+BkYKg6MYGi2zGnx6imeK4QzxBVhpQoPMeDr1npnQ@mail.gmail.com>
	<TYAPR01MB5866C6BCA4D9386D9C486033F5A59@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CAA4eK1++pGobxhw6WiAGpuiQMr_cQ2NiHvMq1inn-YV54Cx6fg@mail.gmail.com>
	<TYAPR01MB5866982AD09BD79D2536675EF5AA9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<OSZPR01MB631003E2729310B412199EB3FDAB9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<TYAPR01MB5866F85511EE35513A899AC3F5AB9@TYAPR01MB5866.jpnprd01.prod.outlook.com>
	<CAD21AoCpf184dPD_n7F6FCn99siCNsiK3Viiu2K6gUzig+wu7Q@mail.gmail.com>
	<TYCPR01MB5870CCDE096B4A41F53087A8F5AF9@TYCPR01MB5870.jpnprd01.prod.outlook.com>

On Mon, Feb 27, 2023 at 2:21 PM Hayato Kuroda (Fujitsu)
<[email protected]> wrote:
>

Few comments:
1.
+ /*
+ * If we've requested to shut down, exit the process.
+ *
+ * Note that WalSndDone() cannot be used here because the delaying
+ * changes will be sent in the function.
+ */
+ if (got_STOPPING)
+ {
+ QueryCompletion qc;
+
+ /* Inform the standby that XLOG streaming is done */
+ SetQueryCompletion(&qc, CMDTAG_COPY, 0);
+ EndCommand(&qc, DestRemote, false);
+ pq_flush();

Do we really need to do anything except for breaking the loop and let
the exit handling happen in the main loop when 'got_STOPPING' is set?
AFAICS, this is what we are doing in some other palces (See
WalSndWaitForWal). Won't that work? It seems that will help us sending
all the pending WAL.

2.
+ /* Try to flush pending output to the client */
+ if (pq_flush_if_writable() != 0)
+ WalSndShutdown();

Is there a reason to try flushing here?

Apart from the above, I have made a few changes in the comments in the
attached diff patch. If you agree with those then please include them
in the next version.

-- 
With Regards,
Amit Kapila.


Attachments:

  [application/octet-stream] changes_amit_1.patch (3.5K, ../CAA4eK1JeBycTHw6rG_Db_9xNWEuK1kCpEAG-2UoMGeaU6TTqaw@mail.gmail.com/2-changes_amit_1.patch)
  download | inline diff:
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index e68902ae34..3cde6d37fa 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -4623,9 +4623,11 @@ ApplyWorkerMain(Datum main_arg)
 	if (!am_tablesync_worker())
 	{
 		/*
-		 * Time-delayed logical replication does not support tablesync
-		 * workers, so only the leader apply worker can request walsenders to
-		 * delay on the publisher side.
+		 * We support time-delayed logical replication only for the apply
+		 * worker. This is because if we support delay during the initial sync
+		 * then once we reach the limit of tablesync workers it would impose a
+		 * delay for each subsequent worker. That would cause initial table
+		 * synchronization completion to take a long time.
 		 */
 		if (server_version >= 160000 && MySubscription->minsenddelay > 0)
 			options.proto.logical.min_send_delay = MySubscription->minsenddelay;
diff --git a/src/backend/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
index 61faf2d685..90bbea941b 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -529,7 +529,10 @@ pgoutput_startup(LogicalDecodingContext *ctx, OutputPluginOptions *opt,
 		else
 			ctx->twophase_opt_given = true;
 
-		/* Copy given time period to decoding context */
+		/*
+		 * Remember the delay time period to be used later before sending the
+		 * changes.
+		 */
 		ctx->min_send_delay = data->min_send_delay;
 
 		/* Init publication state. */
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 9f3968928f..ad3515821e 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -3854,13 +3854,15 @@ LagTrackerRead(int head, XLogRecPtr lsn, TimestampTz now)
 /*
  * LogicalDecodingContext 'delay' callback.
  *
- * Wait long enough to make sure a transaction is applied at least that
- * period behind the publisher.
+ * Wait long enough to make sure a transaction is applied at least
+ * min_send_delay time period after it is performed at the publisher.
+ *
+ * delay_start is the transaction end time.
  */
 static void
 WalSndDelay(LogicalDecodingContext *ctx, TransactionId xid, TimestampTz delay_start)
 {
-	/* Wait till delayUntil by the latch mechanism */
+	/* Apply the delay by the latch mechanism */
 	while (true)
 	{
 		TimestampTz now;
diff --git a/src/test/subscription/t/001_rep_changes.pl b/src/test/subscription/t/001_rep_changes.pl
index ad0831bb1a..a273e7df68 100644
--- a/src/test/subscription/t/001_rep_changes.pl
+++ b/src/test/subscription/t/001_rep_changes.pl
@@ -518,10 +518,10 @@ $node_publisher->poll_query_until('postgres',
 # Test time-delayed logical replication
 #
 # If the subscription sets min_send_delay parameter, the walsender will delay
-# the transaction send for min_send_delay milliseconds. We verify this by
-# looking at the time difference between a) when tuples are inserted on the
-# publisher, and b) when those changes are replicated on the subscriber. Even
-# on slow machines, this strategy will give predictable behavior.
+# the transaction for min_send_delay milliseconds. We verify this by looking
+# at the time difference between a) when tuples are inserted on the publisher,
+# and b) when those changes are replicated on the subscriber. Even on slow
+# machines, this strategy will give predictable behavior.
 
 # Set min_send_delay parameter to 3 seconds
 my $delay = 3;


view thread (16+ 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], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Time delayed LR (WAS Re: logical replication restrictions)
  In-Reply-To: <CAA4eK1JeBycTHw6rG_Db_9xNWEuK1kCpEAG-2UoMGeaU6TTqaw@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