Horton
Menu

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

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, Dilip Kumar <dilipbalaut@gmail.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-12-23T05:52:00Z
Lists: pgsql-hackers

Attachments

On Thursday, December 22, 2022 8:05 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Wed, Dec 21, 2022 at 11:02 AM houzj.fnst@fujitsu.com
> <houzj.fnst@fujitsu.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,"

Changed.

> 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?

No, I moved this to 0007 which include parallel apply workers in the view.

> 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.

Added.

> 
> 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'

Merged.

Besides, I also did the following changes:
1. Added maybe_reread_subscription_info in leader before assigning the
   transaction to parallel apply worker (Sawada-san's comments[1])
2. Removed the "out of parallel apply workers" LOG ( Sawada-san's comments[1])
3. Improved a elog message (Sawada-san's comments[1]).
4. Moved the testcases from 032_xx into existing 015_stream.pl which can save
the initialization time. Since we introduced quite a few testcases in this
patch set, so I did this to try to reduce the testing time that increased after
applying these patches.

[1] https://www.postgresql.org/message-id/CAD21AoDWd2pXau%2BpkYWOi87VGYrDD%3DOxakEDgOyUS%2BqV9XuAGA%40mail.gmail.com

Best regards,
Hou zj

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix invalid memory access during the shutdown of the parallel apply worker.

  2. Fix assertion failure in apply worker.

  3. Use elog to report unexpected action in handle_streamed_transaction().

  4. Use appropriate wait event when sending data in the apply worker.

  5. Allow the logical_replication_mode to be used on the subscriber.

  6. Rename GUC logical_decoding_mode to logical_replication_mode.

  7. Display the leader apply worker's PID for parallel apply workers.

  8. Improve the code to decide and process the apply action.

  9. Document the newly added wait events added by commit 216a784829.

  10. Perform apply of large transactions by parallel workers.

  11. Wake up a subscription's replication worker processes after DDL.

  12. Add copyright notices to meson files

  13. Better document logical replication parameters

  14. Add a common function to generate the origin name.

  15. Harmonize parameter names in storage and AM code.

  16. Avoid using list_length() to test for empty list.

  17. Improve two comments related to a boolean DefElem's value

  18. Fix partition table's REPLICA IDENTITY checking on the subscriber.

  19. Fix data inconsistency between publisher and subscriber.

  20. Fix cache look-up failures while applying changes in logical replication.