Re: Perform streaming logical transactions by background workers and parallel apply

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, "shiy(dot)fnst(at)fujitsu(dot)com" <shiy(dot)fnst(at)fujitsu(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Date: 2022-12-22 12:04:56
Message-ID: CAA4eK1LrADDNXKRVYox4Q+zSFQvtiQu-7piyd_PCmVjg1gsN2w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 21, 2022 at 11:02 AM houzj(dot)fnst(at)fujitsu(dot)com
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> Attach the new patch set which also includes some
> cosmetic comment changes.
>

Few minor comments:
=================
1.
+ <literal>t</literal> = spill the changes of in-progress
transactions to+ disk and apply at once after the transaction is
committed on the+ publisher,

Can we change this description to: "spill the changes of in-progress
transactions to disk and apply at once after the transaction is
committed on the publisher and received by the subscriber,"

2.
table is in progress, there will be additional workers for the tables
- being synchronized.
+ being synchronized. Moreover, if the streaming transaction is applied in
+ parallel, there will be additional workers.

Do we need this change in the first patch? We skip parallel apply
workers from view for the first patch. Am, I missing something?

3.
I think we would need a catversion bump for parallel apply feature
because of below change:
@@ -7913,11 +7913,16 @@ SCRAM-SHA-256$<replaceable>&lt;iteration
count&gt;</replaceable>:<replaceable>&l

<row>
<entry role="catalog_table_entry"><para role="column_definition">
- <structfield>substream</structfield> <type>bool</type>
+ <structfield>substream</structfield> <type>char</type>
</para>

Am, I missing something? If not, then I think we can note that in the
commit message to avoid forgetting it before commit.

4. Kindly change the below comments:
diff --git a/src/backend/replication/logical/applyparallelworker.c
b/src/backend/replication/logical/applyparallelworker.c
index 97f4a3037c..02bb608188 100644
--- a/src/backend/replication/logical/applyparallelworker.c
+++ b/src/backend/replication/logical/applyparallelworker.c
@@ -9,11 +9,10 @@
*
* This file contains the code to launch, set up, and teardown a parallel apply
* worker which receives the changes from the leader worker and
invokes routines
- * to apply those on the subscriber database.
- *
- * This file contains routines that are intended to support setting up, using
- * and tearing down a ParallelApplyWorkerInfo which is required so the leader
- * worker and parallel apply workers can communicate with each other.
+ * to apply those on the subscriber database. Additionally, this file contains
+ * routines that are intended to support setting up, using, and tearing down a
+ * ParallelApplyWorkerInfo which is required so the leader worker and parallel
+ * apply workers can communicate with each other.
*
* The parallel apply workers are assigned (if available) as soon as xact's
* first stream is received for subscriptions that have set their 'streaming'

--
With Regards,
Amit Kapila.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2022-12-22 12:20:37 Re: appendBinaryStringInfo stuff
Previous Message Dag Lem 2022-12-22 12:00:43 Re: daitch_mokotoff module