On Thu, Dec 8, 2022 at 4:42 PM Amit Kapila wrote:
>
> On Thu, Dec 8, 2022 at 12:42 PM Masahiko Sawada wrote:
> >
> > On Wed, Dec 7, 2022 at 10:03 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > >
> > > > +static void
> > > > +ProcessParallelApplyInterrupts(void)
> > > > +{
> > > > +
On Thu, Dec 8, 2022 at 12:42 PM Masahiko Sawada wrote:
>
> On Wed, Dec 7, 2022 at 10:03 PM houzj.f...@fujitsu.com
> wrote:
> >
> >
> > > +static void
> > > +ProcessParallelApplyInterrupts(void)
> > > +{
> > > +CHECK_FOR_INTERRUPTS();
> > > +
> > > +if (ShutdownRequestPending)
> >
On Wed, Dec 7, 2022 at 10:03 PM houzj.f...@fujitsu.com
wrote:
>
> 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
> >
> > > >
> > > >
On Thu, Dec 8, 2022 at 1:52 PM Amit Kapila wrote:
>
> On Wed, Dec 7, 2022 at 6:33 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada
> > wrote:
> > >
> >
> > > ---
> > > When max_parallel_apply_workers_per_subscription is changed to a value
> > >
On Wed, Dec 7, 2022 at 6:33 PM houzj.f...@fujitsu.com
wrote:
>
> On Wednesday, December 7, 2022 7:51 PM Masahiko Sawada
> wrote:
> >
>
> > ---
> > When max_parallel_apply_workers_per_subscription is changed to a value
> > lower than the number of parallel worker running at that time, do we
> >
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 Kapila
>
> > > wrote:
> > > > Some other
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 Kapila
>
> > > wrote:
> > > > Some other
On Wed, Dec 7, 2022 at 4:31 PM Amit Kapila wrote:
>
> On Wed, Dec 7, 2022 at 10:10 AM Masahiko Sawada wrote:
> >
> > On Wed, Dec 7, 2022 at 1:29 PM Amit Kapila wrote:
> > >
> > > Right, but the leader will anyway exit at some point either due to an
> > > ERROR like "lost connection ... to
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 Kapila
> > wrote:
> > > Some other comments:
> > ...
> > Attach the new version patch set which addressed most of the
On Wed, Dec 7, 2022 at 8:28 AM houzj.f...@fujitsu.com
wrote:
>
> Besides, I fixed a bug where there could still be messages left in memory
> queue and the PA has started to apply spooled message.
>
Few comments on the recent changes in the patch:
1. It
On Wed, Dec 7, 2022 at 10:10 AM Masahiko Sawada wrote:
>
> On Wed, Dec 7, 2022 at 1:29 PM Amit Kapila wrote:
> >
> > Right, but the leader will anyway exit at some point either due to an
> > ERROR like "lost connection ... to parallel worker" or with a LOG
> > like: "... will restart because of
On Wed, Dec 7, 2022 at 1:29 PM Amit Kapila wrote:
>
> On Wed, Dec 7, 2022 at 9:00 AM Masahiko Sawada wrote:
> >
> > On Thu, Dec 1, 2022 at 7:17 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > > ---
> > > > if (am_parallel_apply_worker() && on_subinfo_change) {
> > > > /*
> > > > * If
On Wed, Dec 7, 2022 at 9:00 AM Masahiko Sawada wrote:
>
> On Thu, Dec 1, 2022 at 7:17 PM houzj.f...@fujitsu.com
> wrote:
> >
> > > ---
> > > if (am_parallel_apply_worker() && on_subinfo_change) {
> > > /*
> > > * If a parallel apply worker exits due to the subscription
> > > *
On Thu, Dec 1, 2022 at 7:17 PM houzj.f...@fujitsu.com
wrote:
>
> 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 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
On Tue, Dec 6, 2022 at 2:51 PM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 5:27 AM Peter Smith wrote:
> >
> > Here are my review comments for patch v55-0002
> >
> ...
> > 4.
> >
> > /*
> > + * Replay the spooled messages in the parallel apply worker if the leader
> > apply
> > + * worker
On Mon, Dec 5, 2022 at 9:59 AM houzj.f...@fujitsu.com
wrote:
>
> Attach a new version patch set which fixed a testcase failure on CFbot.
>
Few comments:
1.
+ /*
+ * Break the loop if the parallel apply worker has finished applying
+ * the transaction. The parallel apply worker
On Tue, Dec 6, 2022 at 5:27 AM Peter Smith wrote:
>
> Here are my review comments for patch v55-0002
>
...
>
> 3. pa_spooled_messages
>
> Previously I suggested this function name should be changed but that
> was rejected (see [1] #6a)
>
> > 6a.
> > IMO a better name for this function would be
>
Here are my review comments for patch v55-0002
==
.../replication/logical/applyparallelworker.c
1. pa_can_start
@@ -276,9 +278,9 @@ pa_can_start(TransactionId xid)
/*
* Don't start a new parallel worker if user has set skiplsn as it's
* possible that user want to skip the streaming
On Sun, Dec 4, 2022 at 4:48 PM houzj.f...@fujitsu.com
wrote:
>
> On Friday, December 2, 2022 4:59 PM Peter Smith wrote:
> >
>
> > ~~~
> >
> > 12. pa_clean_subtrans
> >
> > +/* Reset the list that maintains subtransactions. */ void
> > +pa_clean_subtrans(void)
> > +{
> > + subxactlist = NIL;
> >
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,
> +
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
(Resending this because somehow my previous post did not appear in the
mail archives)
-- Forwarded message -
From: Peter Smith
Date: Fri, Dec 2, 2022 at 7:59 PM
Subject: Re: Perform streaming logical transactions by background
workers and parallel apply
To: houzj.f...@fujitsu.com
-- Forwarded message -
From: Peter Smith
Date: Sat, Dec 3, 2022 at 8:03 AM
Subject: Re: Perform streaming logical transactions by background
workers and parallel apply
To: Amit Kapila
On Fri, Dec 2, 2022 at 8:57 PM Amit Kapila wrote:
>
> On Fri, Dec 2, 2022 at 2:29 PM
On Fri, Dec 2, 2022 at 4:57 PM Hayato Kuroda (Fujitsu)
wrote:
>
> handle_streamed_transaction()
>
> ```
> + if (apply_action == TRANS_LEADER_SEND_TO_PARALLEL)
> + pa_send_data(winfo, s->len, s->data);
> + else
> +
Dear Hou,
Thanks for making the patch. Followings are my comments for v54-0003 and 0004.
0003
pa_free_worker()
+ /* Unlink any files that were needed to serialize partial changes. */
+ if (winfo->serialize_changes)
+ stream_cleanup_files(MyLogicalRepWorker->subid,
On Fri, Dec 2, 2022 at 2:29 PM Peter Smith wrote:
>
> 3. pa_setup_dsm
>
> +/*
> + * Set up a dynamic shared memory segment.
> + *
> + * We set up a control region that contains a fixed-size worker info
> + * (ParallelApplyWorkerShared), a message queue, and an error queue.
> + *
> + * Returns
On Wed, Nov 30, 2022 at 4:23 PM Amit Kapila wrote:
>
> 2.
> + /*
> + * The stream lock is released when processing changes in a
> + * streaming block, so the leader needs to acquire the lock here
> + * before entering PARTIAL_SERIALIZE mode to ensure that the
> + * parallel apply worker will wait
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 29, 2022 8:34 PM Amit Kapila
> > > > Review
On Thu, Dec 1, 2022 at 11:44 AM Masahiko Sawada wrote:
>
> On Wed, Nov 30, 2022 at 7:54 PM Amit Kapila wrote:
> >
> > On Tue, Nov 29, 2022 at 10:18 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > Attach the new version patch which addressed all comments.
> > >
> >
> > Some comments on
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 29, 2022 8:34 PM Amit Kapila
> > > Review comments on v53-0001*
> >
> > Attach the new version patch set.
>
> Sorry, there were
On Wed, Nov 30, 2022 at 7:54 PM Amit Kapila wrote:
>
> On Tue, Nov 29, 2022 at 10:18 AM houzj.f...@fujitsu.com
> wrote:
> >
> > Attach the new version patch which addressed all comments.
> >
>
> Some comments on v53-0002*
>
> 1. I think testing the scenario where the
Dear hackers,
> 1. I think testing the scenario where the shm_mq buffer is full
> between the leader and parallel apply worker would require a large
> amount of data and then also there is no guarantee. How about having a
> developer GUC [1] force_apply_serialize which allows us to serialize
>
On Tue, Nov 29, 2022 at 10:18 AM houzj.f...@fujitsu.com
wrote:
>
> Attach the new version patch which addressed all comments.
>
Some comments on v53-0002*
1. I think testing the scenario where the shm_mq buffer is full
between the leader and parallel apply worker would
On Tue, Nov 29, 2022 at 6:03 PM Amit Kapila wrote:
>
> 12. Apart from the above, I have made a few changes in the comments
> and some other cosmetic changes in the attached patch.
>
I have made some additional changes in the comments at various places.
Kindly check the attached and let me know
On Tue, Nov 29, 2022 at 10:18 AM houzj.f...@fujitsu.com
wrote:
>
> Attach the new version patch which addressed all comments.
>
Review comments on v53-0001*
==
1.
Subscription *MySubscription = NULL;
-static bool MySubscriptionValid = false;
+bool MySubscriptionValid =
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
On Mon, Nov 28, 2022 at 12:49 PM Peter Smith wrote:
>
...
>
> 17.
> @@ -388,10 +401,9 @@ static inline void cleanup_subxact_info(void);
> /*
> * Serialize and deserialize changes for a toplevel transaction.
> */
> -static void stream_cleanup_files(Oid subid, TransactionId xid);
> static
On Sun, Nov 27, 2022 at 9:43 AM houzj.f...@fujitsu.com
wrote:
>
> Attach the new version patch which addressed all comments so far.
>
Few comments on v52-0001*
1.
pa_free_worker()
{
...
+ /* Free the worker information if the worker exited cleanly. */
+ if
Here are some review comments for patch v51-0002
==
1.
GENERAL - terminology: spool/serialize and data/changes/message
The terminology seems to be used at random. IMO it might be worthwhile
rechecking at least that terms are used consistently in all the
comments. e.g "serialize message
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
>
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
Here are some review comments for v51-0001.
==
.../replication/logical/applyparallelworker.c
1. General - Error messages, get_worker_name()
I previously wrote a comment to ask if the get_worker_name() should be
used in more places but the reply [1, #2b] was:
> 2b.
> Consider if maybe all
On Tue, Nov 22, 2022 at 7:30 AM houzj.f...@fujitsu.com
wrote:
>
Few minor comments and questions:
1.
+static void
+LogicalParallelApplyLoop(shm_mq_handle *mqh)
{
+ for (;;)
+ {
+ void*data;
+ Size len;
+
+ ProcessParallelApplyInterrupts();
...
...
+ if (rc &
On Tue, Nov 22, 2022 at 7:23 PM Hayato Kuroda (Fujitsu)
wrote:
>
>
> 07. proto.c - logicalrep_write_stream_abort()
>
> We may able to add assertions for abort_lsn and abort_time, like xid and
> subxid.
>
If you see logicalrep_write_stream_commit(), we have an assertion for
xid but not for LSN
Dear Hou,
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
some contents may be not. It takes time to understand 0002
Thanks for addressing my review comments on v47-0001.
Here are my review comments for v49-0001.
==
src/backend/replication/logical/applyparallelworker.c
1. GENERAL - NULL checks
There is inconsistent NULL checking in the patch.
Sometimes it is like (!winfo)
Sometimes explicit NULL checks
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
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
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 v47-0001
==
doc/src/sgml/monitoring.
1.
@@ -1851,6 +1851,11 @@ postgres
On Fri, Nov 18, 2022 at 7:56 AM Amit Kapila wrote:
>
> On Wed, Nov 16, 2022 at 1:50 PM houzj.f...@fujitsu.com
> wrote:
> >
> > On Tuesday, November 15, 2022 7:58 PM houzj.f...@fujitsu.com
> > wrote:
> >
> > I noticed that I didn't add CHECK_FOR_INTERRUPTS while retrying send
> > message.
> >
Here are some review comments for v47-0001
(This review is a WIP - I will post more comments for this patch next week)
==
.../replication/logical/applyparallelworker.c
1.
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
On Fri, Nov 18, 2022 at 10:31 AM Masahiko Sawada wrote:
>
> On Fri, Nov 18, 2022 at 1:47 PM Amit Kapila wrote:
> >
> > On Fri, Nov 18, 2022 at 8:01 AM Peter Smith wrote:
> > >
> > > On Fri, Nov 18, 2022 at 11:36 AM Masahiko Sawada
> > > wrote:
> > > >
> > > ...
> > > > ---
> > > > The
On Fri, Nov 18, 2022 at 1:47 PM Amit Kapila wrote:
>
> On Fri, Nov 18, 2022 at 8:01 AM Peter Smith wrote:
> >
> > On Fri, Nov 18, 2022 at 11:36 AM Masahiko Sawada
> > wrote:
> > >
> > ...
> > > ---
> > > The streaming parameter has the new value "parallel" for "streaming"
> > > option to
On Fri, Nov 18, 2022 at 8:01 AM Peter Smith wrote:
>
> On Fri, Nov 18, 2022 at 11:36 AM Masahiko Sawada
> wrote:
> >
> ...
> > ---
> > The streaming parameter has the new value "parallel" for "streaming"
> > option to enable the parallel apply. It fits so far but I think the
> > parallel apply
On Fri, Nov 18, 2022 at 11:36 AM Masahiko Sawada wrote:
>
...
> ---
> The streaming parameter has the new value "parallel" for "streaming"
> option to enable the parallel apply. It fits so far but I think the
> parallel apply feature doesn't necessarily need to be tied up with
> streaming
On Wed, Nov 16, 2022 at 1:50 PM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, November 15, 2022 7:58 PM houzj.f...@fujitsu.com
> wrote:
>
> I noticed that I didn't add CHECK_FOR_INTERRUPTS while retrying send message.
> So, attach the new version which adds that. Also attach the 0004 patch that
On Tue, Nov 15, 2022 at 8:57 PM houzj.f...@fujitsu.com
wrote:
>
> On Saturday, November 12, 2022 7:06 PM Amit Kapila
> >
> > On Fri, Nov 11, 2022 at 2:12 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> >
> > Few comments on v46-0001:
> > ==
> >
>
> Thanks for the comments.
>
>
On Fri, Nov 11, 2022 at 2:12 PM houzj.f...@fujitsu.com
wrote:
>
Few comments on v46-0001:
==
1.
+static void
+apply_handle_stream_abort(StringInfo s)
{
...
+ /* Send STREAM ABORT message to the parallel apply worker. */
+ parallel_apply_send_data(winfo, s->len, s->data);
+
+
On Thu, Nov 10, 2022 at 8:41 PM houzj.f...@fujitsu.com
wrote:
>
> On Monday, November 7, 2022 6:18 PM Masahiko Sawada
> >
> > Here are comments on v42-0001:
>
> Thanks for the comments.
>
> > We have the following three similar name functions regarding to
> > starting a new parallel apply
On Fri, Nov 11, 2022 at 7:57 AM houzj.f...@fujitsu.com
wrote:
>
> On Monday, November 7, 2022 6:18 PM Masahiko Sawada
> wrote:
> >
> > Here are comments on v42-0001:
> >
> > We have the following three similar name functions regarding to
> > starting a new parallel apply worker:
> > ---
> >
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, 2022 at 8:42 PM Masahiko Sawada
> > > wrote:
> > > >
> > > > On
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(StringInfo s) { ...
> > > + if (!first_segment)
> > > + {
> > > +
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;
> +
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
On Mon, Nov 7, 2022 at 1:46 PM Peter Smith wrote:
>
> Here are my review comments for v42-0001
...
...
>
> 8.
>
> + /*
> + * Resend the pending message to parallel apply worker to cleanup the
> + * queue. Note that parallel apply worker will just ignore this message
> + * as it has already
Hi all,
I have tested the patch set in two cases, so I want to share the result.
Case 1. deadlock caused by leader worker, parallel worker, and backend.
Case 2. deadlock caused by non-immutable trigger
===
It has worked well in both cases. PSA reports what I did.
I want to investigate
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(StringInfo s)
> > {
> > ...
> > + if (!first_segment)
> > + {
> > + /*
> > + * Unlock the shared object lock so that parallel apply
> Fair point. I think if the user wants, she can join with
> pg_stat_subscription based on PID and find the corresponding
> subscription. However, if we want to identify everything via pg_locks
> then I think we should also mention classid or database id as field1.
> So, it would look like:
Dear Hou,
The followings are my comments. I want to consider the patch more, but I sent
it once.
===
worker.c
01. typedef enum TransApplyAction
```
/*
* What action to take for the transaction.
*
* TRANS_LEADER_APPLY means that we are in the leader apply worker and changes
* of the
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, 2022 at 8:42 PM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Oct 12, 2022 at 3:04 PM Amit Kapila
> > wrote:
> > > >
> > > > On Tue, Oct
Here are my review comments for v42-0001
==
1. General.
Please take the time to process all new code comments using a
grammar/spelling checker (e.g. simply cut/paste them into MSWord or
Grammarly or any other tool of your choice as a quick double-check)
*before* posting the patches; too
On Mon, Nov 7, 2022 at 10:02 AM Masahiko Sawada wrote:
>
> On Mon, Nov 7, 2022 at 12:58 PM Amit Kapila wrote:
> >
> > > > I agree that it could be better to have a new lock tag. Another point
> > > > is that
> > > > the remote xid and Local xid could be the same in some rare cases, so I
> > >
On Mon, Nov 7, 2022 at 12:58 PM Amit Kapila wrote:
>
> On Mon, Nov 7, 2022 at 8:26 AM Masahiko Sawada wrote:
> >
> > On Sun, Nov 6, 2022 at 3:40 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Saturday, November 5, 2022 1:43 PM Amit Kapila
> > >
> > > >
> > > > On Fri, Nov 4, 2022 at 7:35
On Mon, Nov 7, 2022 at 8:26 AM Masahiko Sawada wrote:
>
> On Sun, Nov 6, 2022 at 3:40 PM houzj.f...@fujitsu.com
> wrote:
> >
> > 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
On Sun, Nov 6, 2022 at 3:40 PM houzj.f...@fujitsu.com
wrote:
>
> 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
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...@fujitsu.com
> > > wrote:
> > > >
> > > > Thanks for the
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...@fujitsu.com
> > wrote:
> > >
> > > Thanks for the analysis and summary !
> > >
> > > I tried to implement the above idea
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.
> >
>
> Few comments on v42-0001
>
On Fri, Nov 4, 2022 at 1:36 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.
> >
>
> Few comments on v42-0001
> ===
> While testing yours, I found that the leader apply worker has been crashed in
> the
> following case.
> I will dig the failure more, but I reported here for records.
I found a reason why the leader apply worker crasehes.
In parallel_apply_free_worker() the leader sends the pending message to
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.
>
Few comments on v42-0001
===
1.
+ /*
+ * Set the xact_state flag in the leader instead of the
+ *
Dear Hou,
Thank you for updating the patch!
While testing yours, I found that the leader apply worker has been crashed in
the following case.
I will dig the failure more, but I reported here for records.
1. Change macros for forcing to write a temporary file.
```
-#define CHANGES_THRESHOLD
On Mon, Oct 24, 2022 at 8:42 PM Masahiko Sawada wrote:
>
> On Wed, Oct 12, 2022 at 3:04 PM Amit Kapila wrote:
> >
> > On Tue, Oct 11, 2022 at 5:52 AM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, Oct 7, 2022 at 2:00 PM Amit Kapila
> > > wrote:
> > > >
> > > > About your point that having
On Fri, Oct 28, 2022 at 3:04 PM shiy.f...@fujitsu.com
wrote:
>
> On Tue, Oct 25, 2022 2:56 PM Wang, Wei/王 威 wrote:
>
> I tried to write a draft patch to force streaming every change instead of
> waiting until logical_decoding_work_mem is exceeded, which could help to test
> streaming parallel.
On Tue, Oct 25, 2022 2:56 PM Wang, Wei/王 威 wrote:
>
> On Tues, Oct 25, 2022 at 14:28 PM Peter Smith
> wrote:
> > FYI - After a recent push, the v40-0001 patch no longer applies on the
> > latest HEAD.
> >
> > [postgres@CentOS7-x64 oss_postgres_misc]$ git apply
> >
On Thu, Oct 27, 2022 at 11:34 AM shiy.f...@fujitsu.com
wrote:
>
> On Wed, Oct 26, 2022 7:19 PM Amit Kapila wrote:
> >
> > On Tue, Oct 25, 2022 at 8:38 AM Masahiko Sawada
> > wrote:
> > >
> > > On Fri, Oct 21, 2022 at 6:32 PM houzj.f...@fujitsu.com
> > > wrote:
> > >
> > > I've started to
On Wed, Oct 26, 2022 7:19 PM Amit Kapila wrote:
>
> On Tue, Oct 25, 2022 at 8:38 AM Masahiko Sawada
> wrote:
> >
> > On Fri, Oct 21, 2022 at 6:32 PM houzj.f...@fujitsu.com
> > wrote:
> >
> > I've started to review this patch. I tested v40-0001 patch and have
> > one question:
> >
> > IIUC even
On Tue, Oct 25, 2022 at 8:38 AM Masahiko Sawada wrote:
>
> On Fri, Oct 21, 2022 at 6:32 PM houzj.f...@fujitsu.com
> wrote:
>
> I've started to review this patch. I tested v40-0001 patch and have
> one question:
>
> IIUC even when most of the changes in the transaction are filtered out
> in
FYI - After a recent push, the v40-0001 patch no longer applies on the
latest HEAD.
[postgres@CentOS7-x64 oss_postgres_misc]$ git apply
../patches_misc/v40-0001-Perform-streaming-logical-transactions-by-parall.patch
error: patch failed: src/backend/replication/logical/launcher.c:54
error:
On Fri, Oct 21, 2022 at 6:32 PM houzj.f...@fujitsu.com
wrote:
>
> On Thursday, October 20, 2022 5:49 PM Amit Kapila
> wrote:
> > On Thu, Oct 20, 2022 at 2:08 PM Peter Smith
> > wrote:
> > >
> > > 7. get_transaction_apply_action
> > >
> > > > 12. get_transaction_apply_action
> > > >
> > > > I
On Mon, Oct 24, 2022 at 11:41 AM Peter Smith wrote:
>
> Here are my review comments for v40-0001.
>
> ==
>
> src/backend/replication/logical/worker.c
>
>
> 1. should_apply_changes_for_rel
>
> + else if (am_parallel_apply_worker())
> + {
> + if (rel->state != SUBREL_STATE_READY)
> +
On Wed, Oct 12, 2022 at 3:04 PM Amit Kapila wrote:
>
> On Tue, Oct 11, 2022 at 5:52 AM Masahiko Sawada wrote:
> >
> > On Fri, Oct 7, 2022 at 2:00 PM Amit Kapila wrote:
> > >
> > > About your point that having different partition structures for
> > > publisher and subscriber, I don't know how
On Fri, Oct 21, 2022 at 3:02 PM houzj.f...@fujitsu.com
wrote:
>
Few comments on the 0001 and 0003 patches:
v40-0001*
==
1.
+ /*
+ * The queue used to transfer messages from the parallel apply worker to
+ * the leader apply worker.
+ */
+ shm_mq_handle *error_mq_handle;
Shall we say
Here are my review comments for v40-0001.
==
src/backend/replication/logical/worker.c
1. should_apply_changes_for_rel
+ else if (am_parallel_apply_worker())
+ {
+ if (rel->state != SUBREL_STATE_READY)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("logical
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
> *
On Thu, Oct 20, 2022 at 2:08 PM Peter Smith wrote:
>
> 7. get_transaction_apply_action
>
> > 12. get_transaction_apply_action
> >
> > I still felt like there should be some tablesync checks/comments in
> > this function, just for sanity, even if it works as-is now.
> >
> > For example, are you
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
Hi, here are my review comments for the patch v39-0001
==
src/backend/libpq/pqmq.c
1. mq_putmessage
+ if (IsParallelWorker())
+ SendProcSignal(pq_mq_parallel_leader_pid,
+PROCSIG_PARALLEL_MESSAGE,
+pq_mq_parallel_leader_backend_id);
+ else
+ {
+
On Tue, Oct 18, 2022 at 6:25 PM Amit Kapila wrote:
> > Interesting case. So I think the root of the problem is the same as
> > what we have for a column is marked unique to the subscriber but not
> > to the publisher. In short, two transactions which are independent of
> > each other on the
201 - 300 of 488 matches
Mail list logo
