RE: Support logical replication of DDLs

2023-04-03 Thread houzj.f...@fujitsu.com
On Friday, March 31, 2023 6:31 AM Peter Smith wrote: Hi, > > It seems that lately, the patch attachments are lacking version numbers. It > causes unnecessary confusion. For example, I sometimes fetch patches from > this thread locally to "diff" them with previous patches to get a rough >

RE: Non-superuser subscription owners

2023-03-31 Thread houzj.f...@fujitsu.com
On Saturday, April 1, 2023 4:00 AM Robert Haas Hi, > > On Thu, Mar 30, 2023 at 9:49 PM houzj.f...@fujitsu.com > wrote: > > It looks like the super user check is out of a transaction, I haven't > > checked why it only failed on one BF animal, but it seems we

RE: Non-superuser subscription owners

2023-03-30 Thread houzj.f...@fujitsu.com
On Friday, March 31, 2023 12:05 AM Robert Haas wrote: Hi, > > On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis wrote: > > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote: > > > The other patch you posted seems like it makes a lot of progress in > > > that direction, and I think that should go

RE: Simplify some codes in pgoutput

2023-03-29 Thread houzj.f...@fujitsu.com
On Thursday, March 30, 2023 9:15 AM Peter Smith wrote: > > Hi Hou-san, > > I tried to compare the logic of patch v3-0001 versus the original HEAD code. > > IMO this patch logic is not exactly the same as before -- there are > some subtle differences. I am not sure if these differences

RE: Support logical replication of DDLs

2023-03-28 Thread houzj.f...@fujitsu.com
On Tuesday, March 28, 2023 1:41 PM Amit Kapila wrote: > > On Mon, Mar 27, 2023 at 5:37 PM Amit Kapila > wrote: > > > > On Mon, Mar 27, 2023 at 12:07 PM Amit Kapila > wrote: > > > > > > On Mon, Mar 27, 2023 at 2:52 AM Tom Lane wrote: > > > > > > > > > > > I suggest taking a couple of steps

RE: Support logical replication of DDLs

2023-03-27 Thread houzj.f...@fujitsu.com
On Monday, March 27, 2023 8:08 PM Amit Kapila wrote: > On Mon, Mar 27, 2023 at 12:07 PM Amit Kapila > wrote: > > > > On Mon, Mar 27, 2023 at 2:52 AM Tom Lane wrote: > > > > > > > > I suggest taking a couple of steps back from the minutiae of the > > > patch, and spending some hard effort

RE: Initial Schema Sync for Logical Replication

2023-03-24 Thread houzj.f...@fujitsu.com
On Friday, March 24, 2023 11:01 PM Euler Taveira wrote: Hi, > On Fri, Mar 24, 2023, at 8:57 AM, mailto:houzj.f...@fujitsu.com wrote: > > First, I think the current publisher doesn't know the version number of > > client(subscriber) so we need to check the feasibility of same. Also, having > >

RE: Initial Schema Sync for Logical Replication

2023-03-24 Thread houzj.f...@fujitsu.com
On Friday, March 24, 2023 12:02 AM Euler Taveira wrote: > > On Thu, Mar 23, 2023, at 8:44 AM, Amit Kapila wrote: > > On Thu, Mar 23, 2023 at 2:48 AM Euler Taveira > > wrote: > > > > > > On Tue, Mar 21, 2023, at 8:18 AM, Amit Kapila wrote: > > > > > > Now, how do we

RE: Simplify some codes in pgoutput

2023-03-22 Thread houzj.f...@fujitsu.com
On Monday, March 20, 2023 5:20 pmhouzj.f...@fujitsu.com wrote: > > On Thursday, March 16, 2023 12:30 PM Amit Kapila > wrote: > > > > > On Wed, Mar 15, 2023 at 2:00 PM houzj.f...@fujitsu.com > > wrote: > > > > > > I noticed that there are some du

RE: Initial Schema Sync for Logical Replication

2023-03-22 Thread houzj.f...@fujitsu.com
On Wednesday, March 22, 2023 1:16 PM Amit Kapila wrote: > > On Wed, Mar 22, 2023 at 8:29 AM Masahiko Sawada > wrote: > > > > On Tue, Mar 21, 2023 at 8:18 PM Amit Kapila > wrote: > > > > > > On Tue, Mar 21, 2023 at 7:32 AM Euler Taveira wrote: > > > > > > > You should > > > > exclude them

RE: Simplify some codes in pgoutput

2023-03-20 Thread houzj.f...@fujitsu.com
On Friday, March 17, 2023 11:49 AM Peter Smith wrote: > > On Wed, Mar 15, 2023 at 7:30 PM houzj.f...@fujitsu.com > wrote: > > > > Hi, > > > > I noticed that there are some duplicated codes in pgoutput_change() > > function which can be simplified, and her

RE: Simplify some codes in pgoutput

2023-03-20 Thread houzj.f...@fujitsu.com
On Thursday, March 16, 2023 12:30 PM Amit Kapila wrote: > > On Wed, Mar 15, 2023 at 2:00 PM houzj.f...@fujitsu.com > wrote: > > > > I noticed that there are some duplicated codes in pgoutput_change() > function > > which can be simplified

RE: Allow logical replication to copy tables in binary format

2023-03-15 Thread houzj.f...@fujitsu.com
Hi, Thanks for updating the patch, I think it is a useful feature. I looked at the v15 patch and the patch looks mostly good to me. Here are few comments: 1. + { + appendStringInfo(, " WITH (FORMAT binary)"); We could use appendStringInfoString here. 2. +# It should fail

Simplify some codes in pgoutput

2023-03-15 Thread houzj.f...@fujitsu.com
Hi, I noticed that there are some duplicated codes in pgoutput_change() function which can be simplified, and here is an attempt to do that. Best Regards, Hou Zhijie 0001-simplify-the-code-in-pgoutput_change.patch Description: 0001-simplify-the-code-in-pgoutput_change.patch

RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

2023-03-13 Thread houzj.f...@fujitsu.com
On Monday, March 13, 2023 2:23 PM Önder Kalacı wrote: Hi, > > > > > > > > > Reading [1], I think I can follow what you suggest. So, basically, > > > if the leftmost column is not filtered, we have the following: > > > > > >> but the entire index would have to be scanned, so in most cases the

RE: Support logical replication of DDLs

2023-03-09 Thread houzj.f...@fujitsu.com
On Thur, Mar 9, 2023 10:27 AM Wang, Wei/王 威 > On Mon, Mar 6, 2023 18:17 PM Wang, Wei/王 威 > wrote: > > On Mon, Mar 6, 2023 14:34 AM Ajin Cherian wrote: > > > Changes are in patch 1 and patch 2 > > > > Thanks for updating the patch set. > > > > Here are some comments: > > Here are some more

RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

2023-03-07 Thread houzj.f...@fujitsu.com
On Wednesday, March 8, 2023 2:51 PM houzj.f...@fujitsu.com wrote: > > On Tuesday, March 7, 2023 9:47 PM Önder Kalacı > wrote: > > Hi, > > > > > Let me give an example to demonstrate why I thought something is fishy > here: > > > > > > &

RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

2023-03-07 Thread houzj.f...@fujitsu.com
On Tuesday, March 7, 2023 9:47 PM Önder Kalacı wrote: Hi, > > > Let me give an example to demonstrate why I thought something is fishy > > > here: > > > > > > Imagine rel has a (non-default) REPLICA IDENTITY with Oid=. > > > Imagine the same rel has a PRIMARY KEY with Oid=. > > > > >

RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher

2023-03-02 Thread houzj.f...@fujitsu.com
On Thursday, March 2, 2023 11:23 PM Önder Kalacı wrote: > Both the patches are numbered 0001. It would be better to number them > as 0001 and 0002. > > Alright, attached v27_0001_use_index_on_subs_when_pub_rep_ident_full.patch > and > v27_0002_use_index_on_subs_when_pub_rep_ident_full.patch.

RE: Rework LogicalOutputPluginWriterUpdateProgress

2023-03-02 Thread houzj.f...@fujitsu.com
On Friday, March 3, 2023 8:18 AM Peter Smith wrote: > On Wed, Mar 1, 2023 at 9:16 PM wangw.f...@fujitsu.com > wrote: > > > > On Tues, Feb 28, 2023 at 9:12 AM Peter Smith > wrote: > > > Here are some comments for the v2-0001 patch. > > > > > > (I haven't looked at the v3 that was posted

RE: Support logical replication of DDLs

2023-02-19 Thread houzj.f...@fujitsu.com
On Wed, Feb 15, 2023 at 13:57 PM Amit Kapila wrote: > On Fri, Feb 10, 2023 at 8:23 PM Masahiko Sawada > > wrote: > > > > On Thu, Feb 9, 2023 at 6:55 PM Ajin Cherian wrote: > > > > > (v67) > > > > I have some questions about adding the infrastructure for DDL deparsing. > > > > Apart from the

RE: Support logical replication of DDLs

2023-02-16 Thread houzj.f...@fujitsu.com
On Wednesday, February 15, 2023 5:51 PM Amit Kapila wrote: > > On Wed, Feb 15, 2023 at 2:02 PM Alvaro Herrera > wrote: > > > > On 2023-Feb-15, Peter Smith wrote: > > > > > On Thu, Feb 9, 2023 at 8:55 PM Ajin Cherian wrote: > > > > > > > > On Fri, Feb 3, 2023 at 11:41 AM Peter Smith > wrote:

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: Support logical replication of DDLs

2023-02-14 Thread houzj.f...@fujitsu.com
On Tuesday, February 14, 2023 9:44 AM Peter Smith wrote: > > FYI - the latest patch cannot be applied. > Thanks for reporting. I will post a rebased patch after fixing some of the comments raised so far(in a day or so). Best regards, Hou zj

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 whil

RE: Deadlock between logrep apply worker and tablesync worker

2023-02-06 Thread houzj.f...@fujitsu.com
On Tuesday, February 7, 2023 12:12 PM Peter Smith wrote: > On Fri, Feb 3, 2023 at 6:58 PM houzj.f...@fujitsu.com > wrote: > > > ... > > > Right, I think that case could be addressed by Tom's patch to some > > > extent but I am thinking we should also try to a

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 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: Deadlock between logrep apply worker and tablesync worker

2023-02-02 Thread houzj.f...@fujitsu.com
On Thursday, February 2, 2023 7:21 PM Amit Kapila wrote: > > On Thu, Feb 2, 2023 at 12:05 PM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, January 31, 2023 1:07 AM vignesh C > wrote: > > > On Mon, 30 Jan 2023 at 17:30, vignesh C wrote: > >

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: Deadlock between logrep apply worker and tablesync worker

2023-02-01 Thread houzj.f...@fujitsu.com
On Tuesday, January 31, 2023 1:07 AM vignesh C wrote: > On Mon, 30 Jan 2023 at 17:30, vignesh C wrote: > > > > On Mon, 30 Jan 2023 at 13:00, houzj.f...@fujitsu.com > > wrote: > > > > > > On Monday, January 30, 2023 2:32 PM Amit Kapila > wrote: >

RE: pub/sub - specifying optional parameters without values.

2023-01-31 Thread houzj.f...@fujitsu.com
On Tuesday, January 31, 2023 10:49 PM Tom Lane wrote: Hi, > Amit Kapila writes: > > On Tue, Jan 31, 2023 at 4:25 AM Tom Lane wrote: > >> Hmph. I generally think that options defined like this (it's a > >> boolean, except it isn't) are a bad idea, and would prefer to see > >> that API

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

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: Deadlock between logrep apply worker and tablesync worker

2023-01-29 Thread houzj.f...@fujitsu.com
On Monday, January 30, 2023 2:32 PM Amit Kapila wrote: > > On Mon, Jan 30, 2023 at 9:20 AM vignesh C wrote: > > > > On Sat, 28 Jan 2023 at 11:26, Amit Kapila wrote: > > > > > > One thing that looks a bit odd is that we will anyway have a similar > > > check in replorigin_drop_guts() which is a

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: Deadlock between logrep apply worker and tablesync worker

2023-01-27 Thread houzj.f...@fujitsu.com
On Friday, January 27, 2023 8:16 PM Amit Kapila > > On Fri, Jan 27, 2023 at 3:45 PM vignesh C wrote: > > > > On Mon, 23 Jan 2023 at 10:52, Amit Kapila > wrote: > > > > > > IIRC, this is done to prevent concurrent drops of origin drop say by > > > exposed API pg_replication_origin_drop(). See

RE: Logical replication timeout problem

2023-01-27 Thread houzj.f...@fujitsu.com
On Wednesday, January 25, 2023 7:26 PM Amit Kapila > > On Tue, Jan 24, 2023 at 8:15 AM wangw.f...@fujitsu.com > wrote: > > > > Attach the new patch. > > > > I think the patch missed to handle the case of non-transactional messages > which > was previously getting handled. I have tried to

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 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: wake up logical workers after ALTER SUBSCRIPTION

2023-01-23 Thread houzj.f...@fujitsu.com
On Monday, January 23, 2023 3:13 AM Tom Lane wrote: Hi, > > Nathan Bossart writes: > > On Tue, Jan 10, 2023 at 10:59:14AM +0530, Amit Kapila wrote: > >> I haven't looked in detail but isn't it better to explain somewhere > >> in the comments that it achieves to rate limit the restart of

RE: Logical replication timeout problem

2023-01-23 Thread houzj.f...@fujitsu.com
On Monday, January 23, 2023 8:51 AM Peter Smith wrote: > > Here are my review comments for patch v4-0001 > == > Commit message > > 2. > > The problem is when there is a DDL in a transaction that generates lots of > temporary data due to rewrite rules, these temporary data will not be >

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

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 comme

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

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

2023-01-16 Thread houzj.f...@fujitsu.com
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 16, 2023 at 10:24 AM Peter Smith > wrote: > > > > > > 2. > > > > > > /* > > > + * Return the pid of the leader apply worker if the given pid is > > > +the pid

RE: logrep stuck with 'ERROR: int2vector has too many elements'

2023-01-15 Thread houzj.f...@fujitsu.com
On Sunday, January 15, 2023 5:35 PM Erik Rijkers wrote: > > I can't find the exact circumstances that cause it but it has something to do > with > many columns (or adding many columns) in combination with perhaps > generated columns. > > This replication test, in a slightly different form,

BF animal malleefowl reported an failure in 001_password.pl

2023-01-13 Thread houzj.f...@fujitsu.com
Hi, I noticed one BF failure[1] when monitoring the BF for some other commit. # Failed test 'authentication success for method password, connstr user=scram_role: log matches' # at t/001_password.pl line 120. # '2023-01-13 07:33:46.741 EST [243628:5] LOG: received SIGHUP,

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

2023-01-13 Thread houzj.f...@fujitsu.com
On Friday, January 13, 2023 1:43 PM Masahiko Sawada wrote: > On Thu, Jan 12, 2023 at 9:34 PM houzj.f...@fujitsu.com > wrote: > > > > On Thursday, January 12, 2023 7:08 PM Amit Kapila > wrote: > > > > > > On Thu, Jan 12, 2023 at 4:21 PM shveta malik

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

2023-01-13 Thread houzj.f...@fujitsu.com
On Friday, January 13, 2023 1:02 PM Masahiko Sawada wrote: > > On Fri, Jan 13, 2023 at 1:28 PM Amit Kapila wrote: > > > > On Fri, Jan 13, 2023 at 9:06 AM Amit Kapila > wrote: > > > > > > On Fri, Jan 13, 2023 at 7:56 AM Peter Smith > wrote: > > > > > > > > > > > > > > > 3. > > > > > > > >

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

2023-01-13 Thread houzj.f...@fujitsu.com
On Friday, January 13, 2023 2:20 PM Peter Smith wrote: > > Here are some review comments for patch v79-0002. Thanks for your comments. > == > > General > > 1. > > I saw that earlier in this thread Hou-san [1] and Amit [2] also seemed to say > there is not much point for this patch. > >

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

2023-01-12 Thread houzj.f...@fujitsu.com
On Thursday, January 12, 2023 12:24 PM Peter Smith wrote: > > Hi, here are some review comments for patch v78-0001. Thanks for your comments. > == > > General > > 1. (terminology) > > AFAIK everywhere until now we’ve been referring everywhere > (docs/comments/code) to the parent apply

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

2023-01-12 Thread houzj.f...@fujitsu.com
On Thursday, January 12, 2023 7:08 PM Amit Kapila wrote: > > On Thu, Jan 12, 2023 at 4:21 PM shveta malik wrote: > > > > On Thu, Jan 12, 2023 at 10:34 AM Amit Kapila > wrote: > > > > > > On Thu, Jan 12, 2023 at 9:54 AM Peter Smith > wrote: > > > > > > > > > > > > doc/src/sgml/monitoring.sgml

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

2023-01-10 Thread houzj.f...@fujitsu.com
On Tuesday, January 10, 2023 7:48 PM Dilip Kumar wrote: > > On Tue, Jan 10, 2023 at 10:26 AM houzj.f...@fujitsu.com > wrote: > > > > On Monday, January 9, 2023 4:51 PM Amit Kapila > wrote: > > > > > > On Sun, Jan 8, 2023 at 11

RE: releasing ParallelApplyTxnHash when pa_launch_parallel_worker returns NULL

2023-01-10 Thread houzj.f...@fujitsu.com
On Wednesday, January 11, 2023 10:21 AM Ted Yu wrote: > /* First time through, initialize parallel apply worker state > hashtable. */ > if (!ParallelApplyTxnHash) > > I think it would be better if `ParallelApplyTxnHash` is created by the first > successful parallel apply worker.

RE: releasing ParallelApplyTxnHash when pa_launch_parallel_worker returns NULL

2023-01-10 Thread houzj.f...@fujitsu.com
On Wednesday, January 11, 2023 1:25 AM Ted Yu wrote: > I was reading src/backend/replication/logical/applyparallelworker.c . > In `pa_allocate_worker`, when pa_launch_parallel_worker returns NULL, I think > the `ParallelApplyTxnHash` should be released. Thanks for reporting.

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

2023-01-09 Thread houzj.f...@fujitsu.com
On Monday, January 9, 2023 4: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

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

2023-01-09 Thread houzj.f...@fujitsu.com
On Monday, January 9, 2023 5:32 PM Shinoda, Noriyoshi (PN Japan FSIP) wrote: > > Hi, Thanks for the great new feature. > > Applied patches include adding wait events LogicalParallelApplyMain, > LogicalParallelApplyStateChange. > However, it seems that monitoring.sgml only contains descriptions

RE: Notify downstream to discard the streamed transaction which was aborted due to crash.

2023-01-05 Thread houzj.f...@fujitsu.com
On Friday, January 6, 2023 1:15 PM Amit Kapila wrote: > > On Fri, Jan 6, 2023 at 9:25 AM houzj.f...@fujitsu.com > wrote: > > > > > > To fix it, One idea is to send a stream abort message when we are > > cleaning up crashed transaction on publisher(e.g. in >

Notify downstream to discard the streamed transaction which was aborted due to crash.

2023-01-05 Thread houzj.f...@fujitsu.com
Hi, When developing another feature, I find an existing bug which was reported from Dilip[1]. Currently, it's possible that we only send a streaming block without sending a end of stream message(stream abort) when decoding and streaming a transaction that was aborted due to crash because we

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

2023-01-05 Thread houzj.f...@fujitsu.com
On Thursday, January 5, 2023 4:22 PM Dilip Kumar wrote: > > On Thu, Jan 5, 2023 at 9:07 AM houzj.f...@fujitsu.com > wrote: > > > > On Wednesday, January 4, 2023 9:29 PM Dilip Kumar > wrote: > > > > I think this looks good to me. > > > > Tha

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

2022-12-20 Thread houzj.f...@fujitsu.com
On Monday, December 19, 2022 8:47 PMs Amit Kapila : > > On Sat, Dec 17, 2022 at 7:34 PM houzj.f...@fujitsu.com > wrote: > > > > Agreed. I have addressed all the comments and did some cosmetic changes. > > Attach the new version patch set. > > >

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

2022-12-16 Thread houzj.f...@fujitsu.com
On Friday, December 16, 2022 3:08 PM Masahiko Sawada wrote: > > >Here are some minor comments: Thanks for the comments! > --- > +pa_has_spooled_message_pending() > +{ > + PartialFileSetState fileset_state; > + > + fileset_state = pa_get_fileset_state(); > + > + if

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

2022-12-13 Thread houzj.f...@fujitsu.com
On Tuesday, December 13, 2022 11:25 PM Masahiko Sawada wrote: > > On Sun, Dec 11, 2022 at 8:45 PM houzj.f...@fujitsu.com > wrote: > > > > On Friday, December 9, 2022 3:14 PM Amit Kapila > wrote: > > > > > > On Thu, Dec 8, 2022 at 12

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

2022-12-13 Thread houzj.f...@fujitsu.com
On Tue, Dec 13, 2022 7:06 AM Peter Smith wrote: > Some minor review comments for v58-0001 Thanks for your comments. > == > > .../replication/logical/applyparallelworker.c > > 1. pa_can_start > > + /* > + * Don't start a new parallel worker if user has set skiplsn as it's > + * possible

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

2022-12-07 Thread houzj.f...@fujitsu.com
On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada wrote: > > On Mon, Dec 5, 2022 at 1:29 PM houzj.f...@fujitsu.com > wrote: > > > > On Sunday, December 4, 2022 7:17 PM houzj.f...@fujitsu.com > > > > > > > Thursday, December 1, 2022 8:40 PM Amit

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

2022-12-07 Thread houzj.f...@fujitsu.com
On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada wrote: > > On Mon, Dec 5, 2022 at 1:29 PM houzj.f...@fujitsu.com > wrote: > > > > On Sunday, December 4, 2022 7:17 PM houzj.f...@fujitsu.com > > > > > > > Thursday, December 1, 2022 8:40 PM Amit

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

2022-12-06 Thread houzj.f...@fujitsu.com
On Tue, Dec 6, 2022 7:57 AM Peter Smith wrote: > Here are my review comments for patch v55-0002 Thansk for your comments. > == > > .../replication/logical/applyparallelworker.c > > 1. pa_can_start > > @@ -276,9 +278,9 @@ pa_can_start(TransactionId xid) > /* > * Don't start a new

RE: Avoid streaming the transaction which are skipped (in corner cases)

2022-12-04 Thread houzj.f...@fujitsu.com
On Saturday, December 3, 2022 7:37 PM Amit Kapila wrote: > > On Tue, Nov 29, 2022 at 12:23 PM houzj.f...@fujitsu.com > wrote: > > > > On Tuesday, November 29, 2022 12:08 PM Dilip Kumar > wrote: > > > > I have few comments about the patch. > > >

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

2022-12-04 Thread houzj.f...@fujitsu.com
On Friday, December 2, 2022 4:59 PM Peter Smith wrote: > > Here are my review comments for patch v54-0001. Thanks for the comments! > == > > FILE: .../replication/logical/applyparallelworker.c > > 1b. > > + * > + * This file contains routines that are intended to support setting up, > +

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

2022-12-04 Thread houzj.f...@fujitsu.com
On Friday, December 2, 2022 7:27 PM Kuroda, Hayato/黒田 隼人 wroteL > > Dear Hou, > > Thanks for making the patch. Followings are my comments for v54-0003 and > 0004. Thanks for the comments! > > 0003 > > pa_free_worker() > > + /* Unlink any files that were needed to serialize partial

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

2022-12-01 Thread houzj.f...@fujitsu.com
On Thursday, December 1, 2022 3:58 PM Masahiko Sawada wrote: > > On Wed, Nov 30, 2022 at 10:51 PM houzj.f...@fujitsu.com > wrote: > > > > On Wednesday, November 30, 2022 9:41 PM houzj.f...@fujitsu.com > wrote: > > > > > > On Tuesday, November 2

RE: Avoid streaming the transaction which are skipped (in corner cases)

2022-11-28 Thread houzj.f...@fujitsu.com
On Tuesday, November 29, 2022 12:08 PM Dilip Kumar wrote: Hi, > > On Mon, Nov 28, 2022 at 3:19 PM Dilip Kumar wrote: > > > > On Mon, Nov 28, 2022 at 1:46 PM shiy.f...@fujitsu.com > > wrote: > > > > > > Thanks for your patch. > > > > > > I saw that the patch added a check when selecting

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

2022-11-28 Thread houzj.f...@fujitsu.com
On Mon, November 28, 2022 15:19 PM Peter Smith wrote: > Here are some review comments for patch v51-0002 Thanks for your comments! > == > > 1. > > GENERAL - terminology: spool/serialize and data/changes/message > > The terminology seems to be used at random. IMO it might be worthwhile

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

2022-11-26 Thread houzj.f...@fujitsu.com
On Tuesday, November 22, 2022 9:53 PM Kuroda, Hayato wroteL > > Thanks for updating the patch! > I tested the case whether the deadlock caused by foreign key constraint could > be detected, and it worked well. > > Followings are my review comments. They are basically related with 0001, but >

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

2022-11-26 Thread houzj.f...@fujitsu.com
On Friday, November 25, 2022 10:54 AM Peter Smith wrote: > > Here are some review comments for v51-0001. Thanks for the comments! > == > > .../replication/logical/applyparallelworker.c > > 1. General - Error messages, get_worker_name() > > I previously wrote a comment to ask if the

Avoid distributing new catalog snapshot again for the transaction being decoded.

2022-11-25 Thread houzj.f...@fujitsu.com
Hi, When doing some other related work, I noticed that when decoding the COMMIT record via SnapBuildCommitTxn()-> SnapBuildDistributeNewCatalogSnapshot() we will add a new snapshot to all transactions including the one being decoded(just committed one). But since we've already done required

RE: wake up logical workers after ALTER SUBSCRIPTION

2022-11-21 Thread houzj.f...@fujitsu.com
On Tuesday, November 22, 2022 2:49 PM Hayato Kuroda (Fujitsu) > > Dear Nathan, > > > I think you are correct. I did it this way in v2. I've also moved > > the bulk of the logic to logical/worker.c. > > Thanks for updating! It becomes better. Further comments: > > 01. AlterSubscription() >

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

2022-11-21 Thread houzj.f...@fujitsu.com
On Monday, November 21, 2022 2:26 PM Peter Smith wrote: > On Fri, Nov 18, 2022 at 6:03 PM Peter Smith > wrote: > > > > Here are some review comments for v47-0001 > > > > (This review is a WIP - I will post more comments for this patch next > > week) > > > > Here are the rest of my comments for

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

2022-11-21 Thread houzj.f...@fujitsu.com
On Friday, November 18, 2022 8:36 AM Masahiko Sawada wrote: > > Here are review comments on v47-0001 and v47-0002 patches: Thanks for the comments! > When the parallel apply worker exited, I got the following server log. > I think this log is not appropriate since the worker was not

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

2022-11-10 Thread houzj.f...@fujitsu.com
On Monday, November 7, 2022 6:18 PM Masahiko Sawada wrote: > > On Thu, Nov 3, 2022 at 10:06 PM houzj.f...@fujitsu.com > wrote: > > > > On Wednesday, November 2, 2022 10:50 AM Masahiko Sawada > wrote: > > > > > > On Mon, Oct 24, 20

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

2022-11-10 Thread houzj.f...@fujitsu.com
On Tuesday, November 8, 2022 7:50 PM Amit Kapila wrote: > On Mon, Nov 7, 2022 at 6:49 PM houzj.f...@fujitsu.com > wrote: > > > > On Friday, November 4, 2022 7:45 PM Amit Kapila > wrote: > > > 3. > > > apply_handle_stream_start(StringI

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

2022-11-10 Thread houzj.f...@fujitsu.com
On Monday, November 7, 2022 4:17 PM Peter Smith > > Here are my review comments for v42-0001 Thanks for the comments. > == > > 28. handle_streamed_transaction > > static bool > handle_streamed_transaction(LogicalRepMsgType action, StringInfo s) { > - TransactionId xid; > +

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

2022-11-10 Thread houzj.f...@fujitsu.com
On Monday, November 7, 2022 7:43 PM Kuroda, Hayato/黒田 隼人 wrote: > > Dear Hou, > > The followings are my comments. I want to consider the patch more, but I sent > it once. Thanks for the comments. > > === > worker.c > > 01. typedef enum TransApplyAction > > ``` > /* > * What action to

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

2022-11-06 Thread houzj.f...@fujitsu.com
On Saturday, November 5, 2022 1:43 PM Amit Kapila > > On Fri, Nov 4, 2022 at 7:35 PM houzj.f...@fujitsu.com > wrote: > > > > On Friday, November 4, 2022 4:07 PM Amit Kapila > wrote: > > > > > > On Thu, Nov 3, 2022 at 6:36 PM houzj.f...@fujits

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

2022-11-04 Thread houzj.f...@fujitsu.com
On Friday, November 4, 2022 4:07 PM Amit Kapila wrote: > > On Thu, Nov 3, 2022 at 6:36 PM houzj.f...@fujitsu.com > wrote: > > > > Thanks for the analysis and summary ! > > > > I tried to implement the above idea and here is the patch set.

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

2022-10-21 Thread houzj.f...@fujitsu.com
On Wednesday, October 19, 2022 8:50 PM Kuroda, Hayato/黒田 隼人 wrote: > > === > 01. applyparallelworker.c - SIZE_STATS_MESSAGE > > ``` > /* > * There are three fields in each message received by the parallel apply > * worker: start_lsn, end_lsn and send_time. Because we have updated these > *

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

2022-10-20 Thread houzj.f...@fujitsu.com
On Wednesday, October 19, 2022 8:50 PM Kuroda, Hayato/黒田 隼人 wrote: Thanks for the comments. > 03. applyparallelwprker.c - LogicalParallelApplyLoop > > ``` > case SHM_MQ_WOULD_BLOCK: > { > int

RE: list of TransactionIds

2022-10-20 Thread houzj.f...@fujitsu.com
On Monday, July 4, 2022 9:27 PM Alvaro Herrera wrote: Hi, > > Pushed now, to master only. Thanks for introducing these APIs! While trying to use the newly introduced list_member_xid(), I noticed that it internally use lfirst_oid instead of lfirst_xid. It works ok for now. Just in case we

RE: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

2022-10-18 Thread houzj.f...@fujitsu.com
On Tuesday, October 18, 2022 5:50 PM Alvaro Herrera wrote: > > On 2022-Oct-18, Japin Li wrote: > > > > > On Tue, 18 Oct 2022 at 12:00, houzj.f...@fujitsu.com > wrote: > > > > > Agreed. Here is new version patch which changed the error code and > >

RE: CF Bot failure in wait_for_subscription_sync()

2022-10-18 Thread houzj.f...@fujitsu.com
On Tuesday, October 18, 2022 2:16 PM Bharath Rupireddy wrote: > > Hi, > > I have seen 2 patches registered in CF failing on Linux - Debian Bullseye in > wait_for_subscription_sync(). It seems like the tables aren't being synced. I > have not done any further analysis. I'm not sure if this

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

2022-10-17 Thread houzj.f...@fujitsu.com
On Tuesday, October 18, 2022 10:36 AM Peter Smith wrote: > > Hi, here are my review comments for patch v38-0001. Thanks for the comments. > ~~~ > > 12. get_transaction_apply_action > > I still felt like there should be some tablesync checks/comments in > this function, just for sanity, even

RE: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

2022-10-17 Thread houzj.f...@fujitsu.com
On Monday, October 17, 2022 6:14 PM Amit Kapila wrote: > > On Mon, Oct 17, 2022 at 2:41 PM Alvaro Herrera > wrote: > > > > On 2022-Oct-17, Peter Smith wrote: > > > > > On Mon, Oct 17, 2022 at 6:43 PM Alvaro Herrera > wrote: > > > > > > I'm not sure that ERRCODE_SYNTAX_ERROR is the right thing

Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

2022-10-16 Thread houzj.f...@fujitsu.com
Hi, While working on some logical replication related features. I found the HINT message could be improved when I tried to add a publication to a subscription which was disabled. alter subscription sub add publication pub2; -- ERROR: ALTER SUBSCRIPTION with refresh is not allowed for disabled

  1   2   3   4   5   >