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

2023-05-09 Thread Amit Kapila
On Tue, May 9, 2023 at 7:50 AM Masahiko Sawada wrote: > > On Mon, May 8, 2023 at 8:09 PM Amit Kapila wrote: > > > > > > I think it is only possible for the leader apply can worker to try to > > receive the error message from an error queue after your 0002 patch. > > Because another place already

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

2023-05-08 Thread Masahiko Sawada
On Mon, May 8, 2023 at 8:09 PM Amit Kapila wrote: > > On Fri, May 5, 2023 at 9:14 AM Zhijie Hou (Fujitsu) > wrote: > > > > On Wednesday, May 3, 2023 3:17 PM Amit Kapila > > wrote: > > > > > > > Attach another patch to fix the problem that pa_shutdown will access invalid > > MyLogicalRepWorker.

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

2023-05-08 Thread Masahiko Sawada
On Mon, May 8, 2023 at 3:34 PM Amit Kapila wrote: > > On Mon, May 8, 2023 at 11:08 AM Masahiko Sawada wrote: > > > > On Mon, May 8, 2023 at 12:52 PM Zhijie Hou (Fujitsu) > > wrote: > > > > > > On Monday, May 8, 2023 11:08 AM Masahiko Sawada > > > > > > Hi, > > > > > > > > > > > On Tue, May 2,

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

2023-05-08 Thread Amit Kapila
On Fri, May 5, 2023 at 9:14 AM Zhijie Hou (Fujitsu) wrote: > > On Wednesday, May 3, 2023 3:17 PM Amit Kapila wrote: > > > > Attach another patch to fix the problem that pa_shutdown will access invalid > MyLogicalRepWorker. I personally want to avoid introducing new static > variable, > so I

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

2023-05-08 Thread Amit Kapila
On Mon, May 8, 2023 at 11:08 AM Masahiko Sawada wrote: > > On Mon, May 8, 2023 at 12:52 PM Zhijie Hou (Fujitsu) > wrote: > > > > On Monday, May 8, 2023 11:08 AM Masahiko Sawada > > > > Hi, > > > > > > > > On Tue, May 2, 2023 at 12:22 PM Amit Kapila > > > wrote: > > > > > > > > On Fri, Apr 28,

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

2023-05-07 Thread Masahiko Sawada
On Mon, May 8, 2023 at 12:52 PM Zhijie Hou (Fujitsu) wrote: > > On Monday, May 8, 2023 11:08 AM Masahiko Sawada > > Hi, > > > > > On Tue, May 2, 2023 at 12:22 PM Amit Kapila > > wrote: > > > > > > On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada > > wrote: > > > > > > > > While investigating

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

2023-05-07 Thread Zhijie Hou (Fujitsu)
On Monday, May 8, 2023 11:08 AM Masahiko Sawada Hi, > > On Tue, May 2, 2023 at 12:22 PM Amit Kapila > wrote: > > > > On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada > wrote: > > > > > > While investigating this issue, I've reviewed the code around > > > callbacks and worker termination etc

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

2023-05-07 Thread Masahiko Sawada
On Tue, May 2, 2023 at 12:22 PM Amit Kapila wrote: > > On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada > wrote: > > > > While investigating this issue, I've reviewed the code around > > callbacks and worker termination etc and I found a problem. > > > > A parallel apply worker calls the

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

2023-05-04 Thread Zhijie Hou (Fujitsu)
On Wednesday, May 3, 2023 3:17 PM Amit Kapila wrote: > > On Tue, May 2, 2023 at 9:46 AM Amit Kapila > wrote: > > > > On Tue, May 2, 2023 at 9:06 AM Zhijie Hou (Fujitsu) > > wrote: > > > > > > On Friday, April 28, 2023 2:18 PM Masahiko Sawada > wrote: > > > > > > > > > > > > > > Alexander,

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

2023-05-03 Thread Amit Kapila
On Tue, May 2, 2023 at 9:46 AM Amit Kapila wrote: > > On Tue, May 2, 2023 at 9:06 AM Zhijie Hou (Fujitsu) > wrote: > > > > On Friday, April 28, 2023 2:18 PM Masahiko Sawada > > wrote: > > > > > > > > > > > Alexander, does the proposed patch fix the problem you are facing? > > > > Sawada-San,

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

2023-05-01 Thread Amit Kapila
On Tue, May 2, 2023 at 9:06 AM Zhijie Hou (Fujitsu) wrote: > > On Friday, April 28, 2023 2:18 PM Masahiko Sawada > wrote: > > > > > > > > Alexander, does the proposed patch fix the problem you are facing? > > > Sawada-San, and others, do you see any better way to fix it than what > > > has been

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

2023-05-01 Thread Zhijie Hou (Fujitsu)
On Friday, April 28, 2023 2:18 PM Masahiko Sawada wrote: > > On Fri, Apr 28, 2023 at 11:51 AM Amit Kapila wrote: > > > > On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) > > wrote: > > > > > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin > wrote: > > > > > > > > IIUC, that assert

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

2023-05-01 Thread Amit Kapila
On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada wrote: > > While investigating this issue, I've reviewed the code around > callbacks and worker termination etc and I found a problem. > > A parallel apply worker calls the before_shmem_exit callbacks in the > following order: > > 1.

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

2023-04-30 Thread Masahiko Sawada
On Fri, Apr 28, 2023 at 6:01 PM Amit Kapila wrote: > > On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada > wrote: > > > > On Fri, Apr 28, 2023 at 11:51 AM Amit Kapila > > wrote: > > > > > > On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) > > > wrote: > > > > > > > > On Wednesday, April

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

2023-04-28 Thread Amit Kapila
On Fri, Apr 28, 2023 at 11:48 AM Masahiko Sawada wrote: > > On Fri, Apr 28, 2023 at 11:51 AM Amit Kapila wrote: > > > > On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) > > wrote: > > > > > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin > > > wrote: > > > > > > > > IIUC, that

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

2023-04-28 Thread Masahiko Sawada
On Fri, Apr 28, 2023 at 11:51 AM Amit Kapila wrote: > > On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) > wrote: > > > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin > > wrote: > > > > > > IIUC, that assert will fail in case of any error raised between > > >

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

2023-04-27 Thread Alexander Lakhin
Hello Amit and Zhijie, 28.04.2023 05:51, Amit Kapila wrote: On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) wrote: I think the problem is that it tried to release locks in logicalrep_worker_onexit() before the initialization of the process is complete because this callback function was

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

2023-04-27 Thread Amit Kapila
On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) wrote: > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin > wrote: > > > > IIUC, that assert will fail in case of any error raised between > > ApplyWorkerMain()->logicalrep_worker_attach()->before_shmem_exit() and > >

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

2023-04-26 Thread Amit Kapila
On Wed, Apr 26, 2023 at 4:11 PM Zhijie Hou (Fujitsu) wrote: > > On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin > wrote: > > Thanks for reporting the issue. > > I think the problem is that it tried to release locks in > logicalrep_worker_onexit() before the initialization of the process

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

2023-04-26 Thread Zhijie Hou (Fujitsu)
On Wednesday, April 26, 2023 5:00 PM Alexander Lakhin wrote: > Please look at a new anomaly that can be observed starting from 216a7848. > > The following script: > echo "CREATE SUBSCRIPTION testsub CONNECTION 'dbname=nodb' > PUBLICATION testpub WITH (connect = false); > ALTER SUBSCRIPTION

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

2023-04-26 Thread Alexander Lakhin
Hello hackers, Please look at a new anomaly that can be observed starting from 216a7848. The following script: echo "CREATE SUBSCRIPTION testsub CONNECTION 'dbname=nodb' PUBLICATION testpub WITH (connect = false); ALTER SUBSCRIPTION testsub ENABLE;" | psql sleep 1 rm

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

2023-04-24 Thread Masahiko Sawada
On Mon, Apr 24, 2023 at 2:24 PM Amit Kapila wrote: > > On Mon, Apr 24, 2023 at 7:26 AM Masahiko Sawada wrote: > > > > While looking at the worker.c, I realized that we have the following > > code in handle_streamed_transaction(): > > > > default: > > Assert(false); > >

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

2023-04-23 Thread Amit Kapila
On Mon, Apr 24, 2023 at 7:26 AM Masahiko Sawada wrote: > > While looking at the worker.c, I realized that we have the following > code in handle_streamed_transaction(): > > default: > Assert(false); > return false; / silence compiler warning / > > I think

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

2023-04-23 Thread Kyotaro Horiguchi
At Mon, 24 Apr 2023 08:59:07 +0530, Amit Kapila wrote in > > Sorry for posting multiple times in a row, but I'm a bit unceratin > > whether we should use FATAL or ERROR for this situation. The stream is > > not provided by user, and the session or process cannot continue. > > > > I think ERROR

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

2023-04-23 Thread Amit Kapila
On Mon, Apr 24, 2023 at 8:40 AM Kyotaro Horiguchi wrote: > > At Mon, 24 Apr 2023 11:50:37 +0900 (JST), Kyotaro Horiguchi > wrote in > > In my opinion, it is fine to replace the Assert with an ERROR. > > Sorry for posting multiple times in a row, but I'm a bit unceratin > whether we should use

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

2023-04-23 Thread Kyotaro Horiguchi
At Mon, 24 Apr 2023 11:50:37 +0900 (JST), Kyotaro Horiguchi wrote in > In my opinion, it is fine to replace the Assert with an ERROR. Sorry for posting multiple times in a row, but I'm a bit unceratin whether we should use FATAL or ERROR for this situation. The stream is not provided by user,

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

2023-04-23 Thread Kyotaro Horiguchi
At Mon, 24 Apr 2023 11:50:37 +0900 (JST), Kyotaro Horiguchi wrote in > I concur that returning false is problematic. > > For assertion builds, Assert typically provides more detailed > information than elog. However, in this case, it wouldn't matter much > since the worker would repeatedly

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

2023-04-23 Thread Kyotaro Horiguchi
At Mon, 24 Apr 2023 10:55:44 +0900, Masahiko Sawada wrote in > While looking at the worker.c, I realized that we have the following > code in handle_streamed_transaction(): > > default: > Assert(false); > return false; / silence compiler warning / > > I

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

2023-04-23 Thread Masahiko Sawada
On Mon, Jan 9, 2023 at 5:51 PM Amit Kapila wrote: > > On Sun, Jan 8, 2023 at 11:32 AM houzj.f...@fujitsu.com > wrote: > > > > On Sunday, January 8, 2023 11:59 AM houzj.f...@fujitsu.com > > wrote: > > > Attach the updated patch set. > > > > Sorry, the commit message of 0001 was accidentally

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

2023-02-15 Thread Peter Smith
LGTM. My only comment is about the commit message. == Commit message d9d7fe6 reuse existing wait event when sending data in apply worker. But we should have invent a new wait state if we are waiting at a new place, so fix this. ~ SUGGESTION d9d7fe6 made use of an existing wait event when

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

2023-02-15 Thread Amit Kapila
On Wed, Feb 15, 2023 at 8:55 AM houzj.f...@fujitsu.com wrote: > > On Wednesday, February 15, 2023 10:34 AM Amit Kapila > wrote: > > > > > > > > > > So names like the below seem correct format: > > > > > > > > a) WAIT_EVENT_LOGICAL_APPLY_SEND_DATA > > > > b) WAIT_EVENT_LOGICAL_LEADER_SEND_DATA >

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

2023-02-14 Thread houzj.f...@fujitsu.com
On Wednesday, February 15, 2023 10:34 AM Amit Kapila wrote: > > On Tue, Feb 14, 2023 at 7:45 PM Masahiko Sawada > wrote: > > > > On Tue, Feb 14, 2023 at 3:58 PM Peter Smith > wrote: > > > > > > On Tue, Feb 14, 2023 at 5:04 PM Amit Kapila > wrote: > > > > > > > > On Fri, Feb 10, 2023 at 8:56

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

2023-02-14 Thread Amit Kapila
On Tue, Feb 14, 2023 at 7:45 PM Masahiko Sawada wrote: > > On Tue, Feb 14, 2023 at 3:58 PM Peter Smith wrote: > > > > On Tue, Feb 14, 2023 at 5:04 PM Amit Kapila wrote: > > > > > > On Fri, Feb 10, 2023 at 8:56 AM Peter Smith wrote: > > > > > > > > My first impression was the > > > >

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

2023-02-14 Thread Masahiko Sawada
On Tue, Feb 14, 2023 at 3:58 PM Peter Smith wrote: > > On Tue, Feb 14, 2023 at 5:04 PM Amit Kapila wrote: > > > > On Fri, Feb 10, 2023 at 8:56 AM Peter Smith wrote: > > > > > > On Fri, Feb 10, 2023 at 1:32 PM houzj.f...@fujitsu.com > > > wrote: > > > > > > > > On Tuesday, February 7, 2023

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

2023-02-13 Thread Peter Smith
On Tue, Feb 14, 2023 at 5:04 PM Amit Kapila wrote: > > On Fri, Feb 10, 2023 at 8:56 AM Peter Smith wrote: > > > > On Fri, Feb 10, 2023 at 1:32 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Tuesday, February 7, 2023 11:17 AM Amit Kapila > > > wrote: > > > > > > > > On Mon, Feb 6, 2023 at

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

2023-02-13 Thread Amit Kapila
On Fri, Feb 10, 2023 at 8:56 AM Peter Smith wrote: > > On Fri, Feb 10, 2023 at 1:32 PM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, February 7, 2023 11:17 AM Amit Kapila > > wrote: > > > > > > On Mon, Feb 6, 2023 at 3:43 PM houzj.f...@fujitsu.com > > > wrote: > > > > > > > > while

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

2023-02-09 Thread Peter Smith
On Fri, Feb 10, 2023 at 1:32 PM houzj.f...@fujitsu.com wrote: > > On Tuesday, February 7, 2023 11:17 AM Amit Kapila > wrote: > > > > On Mon, Feb 6, 2023 at 3:43 PM houzj.f...@fujitsu.com > > wrote: > > > > > > while reading the code, I noticed that in pa_send_data() we set wait > > > event to

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

2023-02-09 Thread houzj.f...@fujitsu.com
On Tuesday, February 7, 2023 11:17 AM Amit Kapila wrote: > > On Mon, Feb 6, 2023 at 3:43 PM houzj.f...@fujitsu.com > wrote: > > > > while reading the code, I noticed that in pa_send_data() we set wait > > event to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while > sending > > the message

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

2023-02-07 Thread wangw.f...@fujitsu.com
On Tue, Feb 7, 2023 15:37 PM Amit Kapila wrote: > On Tue, Feb 7, 2023 at 12:41 PM Masahiko Sawada > wrote: > > > > On Fri, Feb 3, 2023 at 6:44 PM Amit Kapila wrote: > > > > > We need to think of a predictable > > > way to test this path which may not be difficult. But I guess it would > > > be

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

2023-02-06 Thread Amit Kapila
On Tue, Feb 7, 2023 at 12:41 PM Masahiko Sawada wrote: > > On Fri, Feb 3, 2023 at 6:44 PM Amit Kapila wrote: > > > We need to think of a predictable > > way to test this path which may not be difficult. But I guess it would > > be better to wait for some feedback from the field about this

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

2023-02-06 Thread Masahiko Sawada
On Fri, Feb 3, 2023 at 6:44 PM Amit Kapila wrote: > > On Fri, Feb 3, 2023 at 1:28 PM Masahiko Sawada wrote: > > > > On Fri, Feb 3, 2023 at 12:29 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Friday, February 3, 2023 11:04 AM Amit Kapila > > > wrote: > > > > > > > > On Thu, Feb 2, 2023

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

2023-02-06 Thread Amit Kapila
On Mon, Feb 6, 2023 at 3:43 PM houzj.f...@fujitsu.com wrote: > > while reading the code, I noticed that in pa_send_data() we set wait event > to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while sending the > message to the queue. Because this state is used in multiple places, user > might >

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

2023-02-06 Thread Hayato Kuroda (Fujitsu)
Dear Hou, > I think PARALLEL_APPLY_MAIN waits for two kinds of event: 1) wait for new > message from the queue 2) wait for the partial file state to be set. So, I > think introducing a new general event for them is better and it is also > consistent with the WAIT_EVENT_LOGICAL_APPLY_MAIN which is

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

2023-02-06 Thread houzj.f...@fujitsu.com
On Monday, February 6, 2023 6:34 PM Kuroda, Hayato wrote: > > while reading the code, I noticed that in pa_send_data() we set wait > > event to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while > sending > > the message to the queue. Because this state is used in multiple > > places, user

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

2023-02-06 Thread Hayato Kuroda (Fujitsu)
Dear Hou, > while reading the code, I noticed that in pa_send_data() we set wait event > to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while sending > the > message to the queue. Because this state is used in multiple places, user > might > not be able to distinguish what they are waiting

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

2023-02-06 Thread houzj.f...@fujitsu.com
Hi, while reading the code, I noticed that in pa_send_data() we set wait event to WAIT_EVENT_LOGICAL_PARALLEL_APPLY_STATE_CHANGE while sending the message to the queue. Because this state is used in multiple places, user might not be able to distinguish what they are waiting for. So It seems

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

2023-02-03 Thread Amit Kapila
On Fri, Feb 3, 2023 at 1:28 PM Masahiko Sawada wrote: > > On Fri, Feb 3, 2023 at 12:29 PM houzj.f...@fujitsu.com > wrote: > > > > On Friday, February 3, 2023 11:04 AM Amit Kapila > > wrote: > > > > > > On Thu, Feb 2, 2023 at 4:52 AM Peter Smith > > > wrote: > > > > > > > > Some minor review

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

2023-02-02 Thread Masahiko Sawada
On Fri, Feb 3, 2023 at 12:29 PM houzj.f...@fujitsu.com wrote: > > On Friday, February 3, 2023 11:04 AM Amit Kapila > wrote: > > > > On Thu, Feb 2, 2023 at 4:52 AM Peter Smith > > wrote: > > > > > > Some minor review comments for v91-0001 > > > > > > > Pushed this yesterday after addressing

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

2023-02-02 Thread houzj.f...@fujitsu.com
On Friday, February 3, 2023 11:04 AM Amit Kapila wrote: > > On Thu, Feb 2, 2023 at 4:52 AM Peter Smith > wrote: > > > > Some minor review comments for v91-0001 > > > > Pushed this yesterday after addressing your comments! Thanks for pushing. Currently, we have two remaining patches which we

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

2023-02-02 Thread Amit Kapila
On Thu, Feb 2, 2023 at 4:52 AM Peter Smith wrote: > > Some minor review comments for v91-0001 > Pushed this yesterday after addressing your comments! -- With Regards, Amit Kapila.

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

2023-02-01 Thread Peter Smith
Some minor review comments for v91-0001 == doc/src/sgml/config.sgml 1. -Allows streaming or serializing changes immediately in logical decoding. -The allowed values of logical_replication_mode are -buffered and immediate. When set -to immediate,

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

2023-02-01 Thread Amit Kapila
On Tue, Jan 31, 2023 at 9:04 AM houzj.f...@fujitsu.com wrote: > > I think your comment makes sense, thanks. > I updated the patch for the same. > The patch looks mostly good to me. I have made a few changes in the comments and docs, see attached. -- With Regards, Amit Kapila.

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

2023-01-30 Thread Peter Smith
Thanks for the updates to address all of my previous review comments. Patch v90-0001 LGTM. -- Kind Reagrds, Peter Smith. Fujitsu Australia

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

2023-01-30 Thread houzj.f...@fujitsu.com
On Tuesday, January 31, 2023 8:23 AM Peter Smith wrote: > > On Mon, Jan 30, 2023 at 5:23 PM houzj.f...@fujitsu.com > wrote: > > > > On Monday, January 30, 2023 12:13 PM Peter Smith > wrote: > > > > > > Here are my review comments for v88-0002. > > > > Thanks for your comments. > > > > > > > >

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

2023-01-30 Thread houzj.f...@fujitsu.com
On Monday, January 30, 2023 10:20 PM Masahiko Sawada wrote: > > > I have one comment on v89 patch: > > + /* > +* Using 'immediate' mode returns false to cause a switch to > +* PARTIAL_SERIALIZE mode so that the remaining changes will > be serialized. > +*/ > +

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

2023-01-30 Thread Peter Smith
On Mon, Jan 30, 2023 at 5:23 PM houzj.f...@fujitsu.com wrote: > > On Monday, January 30, 2023 12:13 PM Peter Smith > wrote: > > > > Here are my review comments for v88-0002. > > Thanks for your comments. > > > > > == > > General > > > > 1. > > The test cases are checking the log content but

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

2023-01-30 Thread Masahiko Sawada
On Mon, Jan 30, 2023 at 3:23 PM houzj.f...@fujitsu.com wrote: > > On Monday, January 30, 2023 12:13 PM Peter Smith > wrote: > > > > Here are my review comments for v88-0002. > > Thanks for your comments. > > > > > == > > General > > > > 1. > > The test cases are checking the log content but

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

2023-01-30 Thread Hayato Kuroda (Fujitsu)
Dear Hou, Thank you for updating the patch! I checked your replies and new patch, and it seems good. Currently I have no comments Best Regards, Hayato Kuroda FUJITSU LIMITED

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

2023-01-29 Thread houzj.f...@fujitsu.com
On Thursday, January 26, 2023 11:37 AM Kuroda, Hayato/黒田 隼人 wrote: > > Followings are comments. Thanks for the comments. > In this test the rollback-prepared seems not to be executed. This is because > serializations are finished while handling PREPARE message and the final > state of

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

2023-01-29 Thread houzj.f...@fujitsu.com
On Monday, January 30, 2023 12:13 PM Peter Smith wrote: > > Here are my review comments for v88-0002. Thanks for your comments. > > == > General > > 1. > The test cases are checking the log content but they are not checking for > debug logs or untranslated elogs -- they are expecting a

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

2023-01-29 Thread Amit Kapila
On Mon, Jan 30, 2023 at 5:40 AM Peter Smith wrote: > > Patch v88-0001 LGTM. > Pushed. -- With Regards, Amit Kapila.

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

2023-01-29 Thread Peter Smith
Here are my review comments for v88-0002. == General 1. The test cases are checking the log content but they are not checking for debug logs or untranslated elogs -- they are expecting a normal ereport LOG that might be translated. I’m not sure if that is OK, or if it is a potential problem.

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

2023-01-29 Thread Peter Smith
Patch v88-0001 LGTM. Below are just some minor review comments about the commit message. == Commit message 1. We have discussed having this parameter as a subscription option but exposing a parameter that is primarily used for testing/debugging to users didn't seem advisable and there is no

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

2023-01-27 Thread Masahiko Sawada
On Wed, Jan 25, 2023 at 3:27 PM Amit Kapila wrote: > > On Wed, Jan 25, 2023 at 10:05 AM Amit Kapila wrote: > > > > On Wed, Jan 25, 2023 at 3:15 AM Peter Smith wrote: > > > > > > 1. > > > @@ -210,7 +210,7 @@ int logical_decoding_work_mem; > > > static const Size max_changes_in_memory = 4096; /*

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

2023-01-25 Thread Hayato Kuroda (Fujitsu)
Dear Hou, Thank you for updating the patch! Followings are comments. 1. config.sgml ``` +the changes till logical_decoding_work_mem is reached. It can also be ``` I think it should be sandwiched by . 2. config.sgml ``` +On the publisher side, logical_replication_mode allows

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

2023-01-25 Thread houzj.f...@fujitsu.com
On Wednesday, January 25, 2023 7:30 AM Peter Smith wrote: > > Here are my review comments for patch v87-0002. Thanks for your comments. > == > doc/src/sgml/config.sgml > > 1. > > -Allows streaming or serializing changes immediately in > logical decoding. > The

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

2023-01-24 Thread Hayato Kuroda (Fujitsu)
Dear Amit, > > I have updated the patch accordingly and it looks good to me. I'll > push this first patch early next week (Monday) unless there are more > comments. Thanks for updating. I checked v88-0001 and I have no objection. LGTM. Best Regards, Hayato Kuroda FUJITSU LIMITED

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

2023-01-24 Thread Amit Kapila
On Wed, Jan 25, 2023 at 10:05 AM Amit Kapila wrote: > > On Wed, Jan 25, 2023 at 3:15 AM Peter Smith wrote: > > > > 1. > > @@ -210,7 +210,7 @@ int logical_decoding_work_mem; > > static const Size max_changes_in_memory = 4096; /* XXX for restore only */ > > > > /* GUC variable */ > > -int

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

2023-01-24 Thread Amit Kapila
On Wed, Jan 25, 2023 at 3:15 AM Peter Smith wrote: > > 1. > @@ -210,7 +210,7 @@ int logical_decoding_work_mem; > static const Size max_changes_in_memory = 4096; /* XXX for restore only */ > > /* GUC variable */ > -int logical_decoding_mode = LOGICAL_DECODING_MODE_BUFFERED; > +int

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

2023-01-24 Thread Peter Smith
Here are my review comments for patch v87-0002. == doc/src/sgml/config.sgml 1. -Allows streaming or serializing changes immediately in logical decoding. The allowed values of logical_replication_mode are -buffered and immediate. When set -to

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

2023-01-24 Thread Peter Smith
On Tue, Jan 24, 2023 at 11:49 PM houzj.f...@fujitsu.com wrote: > ... > > Sorry, the patch set was somehow attached twice. Here is the correct new > version > patch set which addressed all comments so far. > Here are my review comments for patch v87-0001. ==

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

2023-01-24 Thread Hayato Kuroda (Fujitsu)
Dear Hou, > Sorry, the patch set was somehow attached twice. Here is the correct new > version > patch set which addressed all comments so far. Thank you for updating the patch! I confirmed that All of my comments are addressed. One comment: In this test the rollback-prepared seems not to be

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

2023-01-24 Thread houzj.f...@fujitsu.com
On Tuesday, January 24, 2023 8:47 PM Hou, Zhijie wrote: > > On Tuesday, January 24, 2023 3:19 PM Peter Smith > wrote: > > > > Here are some review comments for v86-0002 > > Sorry, the patch set was somehow attached twice. Here is the correct new version patch set which addressed all comments so

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

2023-01-24 Thread houzj.f...@fujitsu.com
On Monday, January 23, 2023 8:34 PM Kuroda, Hayato wrote: > > Followings are my comments. Thanks for your comments. > > 1. guc_tables.c > > ``` > static const struct config_enum_entry logical_decoding_mode_options[] = { > - {"buffered", LOGICAL_DECODING_MODE_BUFFERED, false}, > -

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

2023-01-24 Thread houzj.f...@fujitsu.com
On Tuesday, January 24, 2023 11:43 AM Peter Smith wrote: > > Here are my review comments for patch v86-0001. Thanks for your comments. > > > == > Commit message > > 2. > Since we may extend the developer option logical_decoding_mode to to test the > parallel apply of large transaction

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

2023-01-24 Thread houzj.f...@fujitsu.com
On Tuesday, January 24, 2023 3:19 PM Peter Smith wrote: > > Here are some review comments for v86-0002 > > == > Commit message > > 1. > Use the use the existing developer option logical_replication_mode to test the > parallel apply of large transaction on subscriber. > > ~ > > Typo “Use

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

2023-01-23 Thread Peter Smith
Here are some review comments for v86-0002 == Commit message 1. Use the use the existing developer option logical_replication_mode to test the parallel apply of large transaction on subscriber. ~ Typo “Use the use the” SUGGESTION (rewritten) Give additional functionality to the existing

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

2023-01-23 Thread Amit Kapila
On Tue, Jan 24, 2023 at 9:13 AM Peter Smith wrote: > > 1. > > IIUC the GUC name was made generic 'logical_replication_mode' so that > multiple developer GUCs are not needed later. > > But IMO those current option values (buffered/immediate) for that GUC > are maybe a bit too generic. Perhaps in

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

2023-01-23 Thread Peter Smith
Here are my review comments for patch v86-0001. == General 1. IIUC the GUC name was made generic 'logical_replication_mode' so that multiple developer GUCs are not needed later. But IMO those current option values (buffered/immediate) for that GUC are maybe a bit too generic. Perhaps in

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

2023-01-23 Thread Hayato Kuroda (Fujitsu)
Dear Hou, Thank you for updating the patch! Followings are my comments. 1. guc_tables.c ``` static const struct config_enum_entry logical_decoding_mode_options[] = { - {"buffered", LOGICAL_DECODING_MODE_BUFFERED, false}, - {"immediate", LOGICAL_DECODING_MODE_IMMEDIATE, false}, +

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

2023-01-23 Thread houzj.f...@fujitsu.com
On Monday, January 23, 2023 11:17 AM Amit Kapila wrote: > > On Fri, Jan 20, 2023 at 11:48 AM Masahiko Sawada > wrote: > > > > > > > > Yet another way is to use the existing parameter logical_decode_mode > > > [1]. If the value of logical_decoding_mode is 'immediate', then we > > > can

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

2023-01-22 Thread Dilip Kumar
On Mon, Jan 23, 2023 at 8:47 AM Amit Kapila wrote: > > On Fri, Jan 20, 2023 at 11:48 AM Masahiko Sawada > wrote: > > > > > > > > Yet another way is to use the existing parameter logical_decode_mode > > > [1]. If the value of logical_decoding_mode is 'immediate', then we can > > > immediately

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

2023-01-22 Thread Amit Kapila
On Fri, Jan 20, 2023 at 11:48 AM Masahiko Sawada wrote: > > > > > Yet another way is to use the existing parameter logical_decode_mode > > [1]. If the value of logical_decoding_mode is 'immediate', then we can > > immediately switch to partial serialize mode. This will eliminate the > >

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

2023-01-19 Thread Masahiko Sawada
On Thu, Jan 19, 2023 at 2:41 PM Amit Kapila wrote: > > On Wed, Jan 18, 2023 at 12:09 PM Amit Kapila wrote: > > > > On Fri, Jan 13, 2023 at 11:50 AM Peter Smith wrote: > > > > > > Here are some review comments for patch v79-0002. > > > > > > > So, this is about the latest

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

2023-01-19 Thread shveta malik
On Thu, Jan 19, 2023 at 3:44 PM shveta malik wrote: > > On Thu, Jan 19, 2023 at 11:11 AM Amit Kapila wrote: > > > > On Wed, Jan 18, 2023 at 12:09 PM Amit Kapila > > wrote: > > > > > > On Fri, Jan 13, 2023 at 11:50 AM Peter Smith > > > wrote: > > > > > > > > Here are some review comments for

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

2023-01-19 Thread shveta malik
On Thu, Jan 19, 2023 at 11:11 AM Amit Kapila wrote: > > On Wed, Jan 18, 2023 at 12:09 PM Amit Kapila wrote: > > > > On Fri, Jan 13, 2023 at 11:50 AM Peter Smith wrote: > > > > > > Here are some review comments for patch v79-0002. > > > > > > > So, this is about the latest

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

2023-01-18 Thread Amit Kapila
On Wed, Jan 18, 2023 at 12:09 PM Amit Kapila wrote: > > On Fri, Jan 13, 2023 at 11:50 AM Peter Smith wrote: > > > > Here are some review comments for patch v79-0002. > > > > So, this is about the latest v84-0001-Stop-extra-worker-if-GUC-was-changed. > > > > > I feel this patch just adds more

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

2023-01-17 Thread Amit Kapila
On Fri, Jan 13, 2023 at 11:50 AM Peter Smith wrote: > > Here are some review comments for patch v79-0002. > So, this is about the latest v84-0001-Stop-extra-worker-if-GUC-was-changed. > == > > General > > 1. > > I saw that earlier in this thread Hou-san [1] and Amit [2] also seemed > to say

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

2023-01-17 Thread wangw.f...@fujitsu.com
On Wed, Jan 18, 2023 12:36 PM Amit Kapila wrote: > On Tue, Jan 17, 2023 at 8:07 PM Masahiko Sawada > wrote: > > > > On Tue, Jan 17, 2023 at 6:14 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada > wrote: > > > > > > > > On Tue, Jan 17, 2023

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

2023-01-17 Thread Amit Kapila
On Tue, Jan 17, 2023 at 8:07 PM Masahiko Sawada wrote: > > On Tue, Jan 17, 2023 at 6:14 PM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada > > wrote: > > > > > > On Tue, Jan 17, 2023 at 12:37 PM houzj.f...@fujitsu.com > > > wrote: > > > I'm

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

2023-01-17 Thread Masahiko Sawada
On Tue, Jan 17, 2023 at 6:14 PM houzj.f...@fujitsu.com wrote: > > On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada > wrote: > > > > On Tue, Jan 17, 2023 at 12:37 PM houzj.f...@fujitsu.com > > wrote: > > > Attach the new version 0001 patch which addressed all other comments. > > > > > > >

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

2023-01-17 Thread houzj.f...@fujitsu.com
On Tuesday, January 17, 2023 12:34 PM shveta malik wrote: > > On Tue, Jan 17, 2023 at 9:07 AM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, January 17, 2023 11:32 AM Peter Smith > wrote: > > > OK. I didn't know there was another header convention that you were > > > following. > > > In

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

2023-01-17 Thread houzj.f...@fujitsu.com
On Tuesday, January 17, 2023 2:46 PM Masahiko Sawada wrote: > > On Tue, Jan 17, 2023 at 12:37 PM houzj.f...@fujitsu.com > wrote: > > Attach the new version 0001 patch which addressed all other comments. > > > > Thank you for updating the patch. Here is one comment: > > @@ -426,14 +427,24 @@

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

2023-01-16 Thread Masahiko Sawada
On Tue, Jan 17, 2023 at 12:37 PM houzj.f...@fujitsu.com wrote: > > On Tuesday, January 17, 2023 11:32 AM Peter Smith > wrote: > > > > On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Tuesday, January 17, 2023 5:43 AM Peter Smith > > wrote: > > > > > > > > On

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

2023-01-16 Thread houzj.f...@fujitsu.com
On Tuesday, January 17, 2023 12:55 PM Amit Kapila wrote: > > On Tue, Jan 17, 2023 at 8:59 AM Amit Kapila wrote: > > > > On Tue, Jan 17, 2023 at 8:35 AM Masahiko Sawada > wrote: > > > > > > On Mon, Jan 16, 2023 at 3:19 PM Amit Kapila > wrote: > > > > > > > > Okay, I have added the comments in

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

2023-01-16 Thread Masahiko Sawada
On Tue, Jan 17, 2023 at 1:55 PM Amit Kapila wrote: > > On Tue, Jan 17, 2023 at 8:59 AM Amit Kapila wrote: > > > > On Tue, Jan 17, 2023 at 8:35 AM Masahiko Sawada > > wrote: > > > > > > On Mon, Jan 16, 2023 at 3:19 PM Amit Kapila > > > wrote: > > > > > > > > Okay, I have added the comments in

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

2023-01-16 Thread Amit Kapila
On Tue, Jan 17, 2023 at 8:59 AM Amit Kapila wrote: > > On Tue, Jan 17, 2023 at 8:35 AM Masahiko Sawada wrote: > > > > On Mon, Jan 16, 2023 at 3:19 PM Amit Kapila wrote: > > > > > > Okay, I have added the comments in get_transaction_apply_action() and > > > updated the comments to refer to the

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

2023-01-16 Thread shveta malik
On Tue, Jan 17, 2023 at 9:07 AM houzj.f...@fujitsu.com wrote: > > On Tuesday, January 17, 2023 11:32 AM Peter Smith > wrote: > > > > On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Tuesday, January 17, 2023 5:43 AM Peter Smith > > wrote: > > > > > > > > On

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

2023-01-16 Thread Peter Smith
On Tue, Jan 17, 2023 at 2:37 PM houzj.f...@fujitsu.com wrote: > > On Tuesday, January 17, 2023 11:32 AM Peter Smith > wrote: > > > > On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com > > wrote: > > > > > > On Tuesday, January 17, 2023 5:43 AM Peter Smith > > wrote: > > > > > > > > On

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

2023-01-16 Thread houzj.f...@fujitsu.com
On Tuesday, January 17, 2023 11:32 AM Peter Smith wrote: > > On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, January 17, 2023 5:43 AM Peter Smith > wrote: > > > > > > On Mon, Jan 16, 2023 at 5:41 PM Amit Kapila > > > > > > wrote: > > > > > > > > On Mon, Jan

  1   2   3   4   5   >