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

From: "wangw(dot)fnst(at)fujitsu(dot)com" <wangw(dot)fnst(at)fujitsu(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(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-09-15 05:22:45
Message-ID: OS3PR01MB6275EA6DA19CB8204FC90DE09E499@OS3PR01MB6275.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Sep 13, 2022 at 18:26 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>

Thanks for your comments.

> On Fri, Sep 9, 2022 at 12:32 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > 29. src/backend/replication/logical/worker.c - TransactionApplyAction
> >
> > /*
> > * What action to take for the transaction.
> > *
> > * TA_APPLY_IN_LEADER_WORKER means that we are in the leader apply
> worker and
> > * changes of the transaction are applied directly in the worker.
> > *
> > * TA_SERIALIZE_TO_FILE means that we are in leader apply worker and
> changes
> > * are written to temporary files and then applied when the final commit
> > * arrives.
> > *
> > * TA_APPLY_IN_PARALLEL_WORKER means that we are in the parallel apply
> worker
> > * and changes of the transaction are applied directly in the worker.
> > *
> > * TA_SEND_TO_PARALLEL_WORKER means that we are in the leader apply
> worker and
> > * need to send the changes to the parallel apply worker.
> > */
> > typedef enum
> > {
> > /* The action for non-streaming transactions. */
> > TA_APPLY_IN_LEADER_WORKER,
> >
> > /* Actions for streaming transactions. */
> > TA_SERIALIZE_TO_FILE,
> > TA_APPLY_IN_PARALLEL_WORKER,
> > TA_SEND_TO_PARALLEL_WORKER
> > } TransactionApplyAction;
> >
> > ~
> >
> > 29a.
> > I think if you change all those enum names slightly (e.g. like below)
> > then they can be more self-explanatory:
> >
> > TA_NOT_STREAMING_LEADER_APPLY
> > TA_STREAMING_LEADER_SERIALIZE
> > TA_STREAMING_LEADER_SEND_TO_PARALLEL
> > TA_STREAMING_PARALLEL_APPLY
> >
> > ~
> >
>
> I also think we can improve naming but adding streaming in the names
> makes them slightly difficult to read. As you have suggested, it will
> be better to add comments for streaming and non-streaming cases. How
> about naming them as below:
>
> typedef enum
> {
> TRANS_LEADER_APPLY
> TRANS_LEADER_SERIALIZE
> TRANS_LEADER_SEND_TO_PARALLEL
> TRANS_PARALLEL_APPLY
> } TransApplyAction;

I think your suggestion looks good.
Improved as suggested.

The new patches were attached in [1].

[1] - https://www.postgresql.org/message-id/OS3PR01MB6275F145878B4A44586C46CE9E499%40OS3PR01MB6275.jpnprd01.prod.outlook.com

Regards,
Wang wei

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message wangw.fnst@fujitsu.com 2022-09-15 05:23:34 RE: Perform streaming logical transactions by background workers and parallel apply
Previous Message wangw.fnst@fujitsu.com 2022-09-15 05:20:54 RE: Perform streaming logical transactions by background workers and parallel apply