RE: Perform streaming logical transactions by background workers and parallel apply - Mailing list pgsql-hackers

From wangw.fnst@fujitsu.com
Subject RE: Perform streaming logical transactions by background workers and parallel apply
Date
Msg-id OS3PR01MB6275EA6DA19CB8204FC90DE09E499@OS3PR01MB6275.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: Perform streaming logical transactions by background workers and parallel apply  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
On Wed, Sep 13, 2022 at 18:26 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>

Thanks for your comments.

> On Fri, Sep 9, 2022 at 12:32 PM Peter Smith <smithpb2250@gmail.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

pgsql-hackers by date:

Previous
From: "wangw.fnst@fujitsu.com"
Date:
Subject: RE: Perform streaming logical transactions by background workers and parallel apply
Next
From: "wangw.fnst@fujitsu.com"
Date:
Subject: RE: Perform streaming logical transactions by background workers and parallel apply