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 16, 2023 at 10:24 AM Peter Smith
> > wrote:
> > > >
> > > > 2.
> > > >
> > > > /*
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 enum TransApplyAction where all
> > the actions are explained.
>
>
On Mon, Jan 16, 2023 at 3:19 PM Amit Kapila wrote:
>
> On Sun, Jan 15, 2023 at 10:39 PM Tomas Vondra
> wrote:
> >
> > I think there's a bug in how get_transaction_apply_action() interacts
> > with handle_streamed_transaction() to decide whether the transaction is
> > streamed or not. Originally,
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
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
> > of a
> > + * parallel apply worker, otherwise return InvalidPid.
> > + */
> >
Hi Amit,
Thanks for the patch, the changes seem reasonable to me and it does fix
the issue in the sequence decoding patch.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
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 of a
> + * parallel apply worker, otherwise return InvalidPid.
> + */
> +pid_t
> +GetLeaderApplyWorkerPid(pid_t pid)
> +{
> + int leader_pid = InvalidPid;
On Sun, Jan 15, 2023 at 10:39 PM Tomas Vondra
wrote:
>
> I think there's a bug in how get_transaction_apply_action() interacts
> with handle_streamed_transaction() to decide whether the transaction is
> streamed or not. Originally, the code was simply:
>
> /* not in streaming mode */
> if
Here are some review comments for v81-0001.
==
Commit Message
1.
Additionally, update the leader_pid column in pg_stat_activity as well to
display the PID of the leader apply worker for parallel apply workers.
~
Probably it should not say both "Additionally" and "as well" in the
same
At Tue, 10 Jan 2023 12:01:43 +0530, Amit Kapila wrote
in
> On Tue, Jan 10, 2023 at 11:16 AM Kyotaro Horiguchi
> wrote:
> > Although I don't see a technical difference between the two, all the
> > other occurances including the just above (except test_shm_mq) use
> > "could not". A faint memory
Hi,
I think there's a bug in how get_transaction_apply_action() interacts
with handle_streamed_transaction() to decide whether the transaction is
streamed or not. Originally, the code was simply:
/* not in streaming mode */
if (!in_streamed_transaction)
return false;
But now
On Fri, Jan 13, 2023 at 3:44 PM houzj.f...@fujitsu.com
wrote:
>
> 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:
In GetLogicalLeaderApplyWorker(), we can use shared lock instead
exclusive as we are just
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
> wrote:
> > > >
> > > > On Thu, Jan 12,
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.
> > > >
> > > >
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.
>
>
Here are some review comments for patch v79-0002.
==
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.
So I wanted to +1 that same opinion.
I feel this patch just adds more complexity for almost no gain:
-
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 wrote:
> > >
> > > On Thu, Jan 12, 2023 at 10:34 AM Amit Kapila
> > wrote:
> > > >
> > > > On Thu, Jan 12, 2023 at
On Fri, Jan 13, 2023 at 2:37 PM Amit Kapila wrote:
>
> > 3.
> >
> >
> > + leader_pid integer
> > +
> > +
> > + Process ID of the leader apply worker if this process is a parallel
> > + apply worker; NULL if this process is a leader apply worker or does
> >
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.
> > >
> > >
> > > + leader_pid integer
> > > +
> > > +
> > > + Process ID
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.
> >
> >
> > + leader_pid integer
> > +
> > +
> > + Process ID of the leader apply worker if this process is a parallel
> > + apply
On Fri, Jan 13, 2023 at 7:56 AM Peter Smith wrote:
>
> Here are my review comments for v79-0001.
>
> ==
>
> General
>
> 1.
>
> When Amit suggested [1] changing the name just to "leader_pid" instead
> of "leader_apply_pid" I thought he was only referring to changing the
> view column name, not
On Thu, Jan 12, 2023 at 4:37 PM Amit Kapila wrote:
>
>
> But then do you suggest that tomorrow if we allow parallel sync
> workers then we have a separate column leader_sync_pid? I think that
> doesn't sound like a good idea and moreover one can refer to docs for
> clarification.
>
> --
okay,
Here are my review comments for v79-0001.
==
General
1.
When Amit suggested [1] changing the name just to "leader_pid" instead
of "leader_apply_pid" I thought he was only referring to changing the
view column name, not also the internal member names of the worker
structure. Maybe it is OK
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
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
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
> > >
> > > 5. pg_stat_subscription
> > >
> > > @@ -3198,11 +3198,22 @@ SELECT
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
> >
> > 5. pg_stat_subscription
> >
> > @@ -3198,11 +3198,22 @@ SELECT pid, wait_event_type, wait_event FROM
> > pg_stat_activity WHERE wait_event i
On Thu, Jan 12, 2023 at 9:54 AM Peter Smith wrote:
>
>
> doc/src/sgml/monitoring.sgml
>
> 5. pg_stat_subscription
>
> @@ -3198,11 +3198,22 @@ SELECT pid, wait_event_type, wait_event FROM
> pg_stat_activity WHERE wait_event i
>
>
>
> + apply_leader_pid integer
> +
> +
Hi, here are some review comments for patch v78-0001.
==
General
1. (terminology)
AFAIK everywhere until now we’ve been referring everywhere
(docs/comments/code) to the parent apply worker as the "leader apply
worker". Not the "main apply worker". Not the "apply leader worker".
Not any
On Wed, Jan 11, 2023 at 9:34 AM houzj.f...@fujitsu.com
wrote:
>
> > I was looking into 0001, IMHO the pid should continue to represent the main
> > apply worker. So the pid will always show the main apply worker which is
> > actually receiving all the changes for the subscription (in short
On Wed, Jan 11, 2023 at 9:34 AM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, January 10, 2023 7:48 PM Dilip Kumar
> wrote:
> >
> > I was looking into 0001, IMHO the pid should continue to represent the main
> > apply worker. So the pid will always show the main apply worker which is
> >
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:32 AM houzj.f...@fujitsu.com
> > > wrote:
> > > >
> > > > On
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:32 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Sunday, January 8, 2023 11:59 AM houzj.f...@fujitsu.com
> > wrote:
> > > > Attach
On Tue, Jan 10, 2023 at 11:16 AM Kyotaro Horiguchi
wrote:
>
> At Mon, 9 Jan 2023 14:21:03 +0530, Amit Kapila
> wrote in
> > Pushed the first (0001) patch.
>
> It added the following error message.
>
> + seg = dsm_attach(handle);
> + if (!seg)
> + ereport(ERROR,
> +
Hello.
At Mon, 9 Jan 2023 14:21:03 +0530, Amit Kapila wrote
in
> Pushed the first (0001) patch.
It added the following error message.
+ seg = dsm_attach(handle);
+ if (!seg)
+ ereport(ERROR,
+
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, the commit message of 0001 was accidentally
bject: RE: Perform streaming logical transactions by background workers and
parallel apply
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 Log
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
: Perform streaming logical transactions by background workers and
parallel apply
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 c
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 deleted, just attach
> the same patch set again with commit message.
>
On Sat, Jan 7, 2023 at 2:25 PM Dilip Kumar wrote:
>
Today, I was analyzing this patch w.r.t recent commit c6e1f62e2c and
found that pa_set_xact_state() should set the latch (wake up) for the
leader worker as the leader could be waiting in
pa_wait_for_xact_state(). What do you think? But
On Sat, Jan 7, 2023 at 11:13 AM houzj.f...@fujitsu.com
wrote:
>
> On Saturday, January 7, 2023 12:50 PM Dilip Kumar
> >
> > On Fri, Jan 6, 2023 at 3:38 PM houzj.f...@fujitsu.com
> >
> > wrote:
> > >
> >
> > Looks good, but I feel in pa_process_spooled_messages_if_required()
> > function after
On Fri, Jan 6, 2023 at 3:38 PM houzj.f...@fujitsu.com
wrote:
>
Looks good, but I feel in pa_process_spooled_messages_if_required()
function after getting the filestate the first check should be if
(filestate== FS_EMPTY) return false. I mean why to process through
all the states if it is empty
On Fri, Jan 6, 2023 at 12:59 PM Dilip Kumar wrote:
>
> > > 3.
> > > +else if (shmq_res == SHM_MQ_WOULD_BLOCK)
> > > +{
> > > +/* Replay the changes from the file, if any. */
> > > +if (pa_has_spooled_message_pending())
> > > +{
> > > +
On Fri, Jan 6, 2023 at 12:05 PM Amit Kapila wrote:
>
>
> Yeah, I also don't think sending extra eight bytes with stream_start
> message is worth it. But it is fine to mention the same in the
> comments.
Right.
> > 2.
> >
> > + * XXX Additionally, we also stop the worker if the leader apply
On Fri, Jan 6, 2023 at 11:24 AM Dilip Kumar wrote:
>
> On Fri, Jan 6, 2023 at 9:37 AM houzj.f...@fujitsu.com
> wrote:
> >
> > On Thursday, January 5, 2023 7:54 PM Dilip Kumar
> > wrote:
> > Thanks, I have started another thread[1]
> >
> > Attach the parallel apply patch set here again. I
On Fri, Jan 6, 2023 at 9:37 AM houzj.f...@fujitsu.com
wrote:
>
> On Thursday, January 5, 2023 7:54 PM Dilip Kumar
> wrote:
> Thanks, I have started another thread[1]
>
> Attach the parallel apply patch set here again. I didn't change the patch set,
> attach it here just to let the CFbot keep
On Thu, Jan 5, 2023 at 5:03 PM houzj.f...@fujitsu.com
wrote:
>
> On Thursday, January 5, 2023 4:22 PM Dilip Kumar
> wrote:
> >
> Thanks for reporting the problem.
>
> After analyzing the behavior, I think it's a bug on publisher side which
> is not directly related to parallel apply.
>
> I
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.
> >
> > Thanks for the comments.
> > Attach the new version
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.
>
> Thanks for the comments.
> Attach the new version patch set which changed the comments as suggested.
Thanks for the updated patch,
On Wed, Jan 4, 2023 at 6:40 PM Amit Kapila wrote:
>
> On Wed, Jan 4, 2023 at 4:52 PM Dilip Kumar wrote:
> >
> > 2.
> > + * Since the database structure (schema of subscription tables,
> > constraints,
> > + * etc.) of the publisher and subscriber could be different, applying
> > + *
On Wed, Jan 4, 2023 at 4:52 PM Dilip Kumar wrote:
>
> 2.
> + * Since the database structure (schema of subscription tables, constraints,
> + * etc.) of the publisher and subscriber could be different, applying
> + * transactions in parallel mode on the subscriber side can cause some
> + *
On Wed, Jan 4, 2023 at 4:25 PM houzj.f...@fujitsu.com
wrote:
>
> Attach the new patch set.
> Apart from addressing Sawada-San's comments, I also did some other minor
> changes in the patch:
I have done a high-level review of 0001, and later I will do a
detailed review of this while reading
On Wed, Jan 4, 2023 at 2:31 PM Masahiko Sawada wrote:
>
> On Tue, Jan 3, 2023 at 2:40 PM wangw.f...@fujitsu.com
> wrote:
> >
> > On Mon, Jan 2, 2023 at 18:54 PM Amit Kapila wrote:
> > > On Fri, Dec 30, 2022 at 3:55 PM wangw.f...@fujitsu.com
> > > wrote:
> > > >
> > > > I've checked it and it
On Tue, Jan 3, 2023 at 2:40 PM wangw.f...@fujitsu.com
wrote:
>
> On Mon, Jan 2, 2023 at 18:54 PM Amit Kapila wrote:
> > On Fri, Dec 30, 2022 at 3:55 PM wangw.f...@fujitsu.com
> > wrote:
> > >
> > > I've checked it and it looks good to me.
> > > Rebased the other patches and ran the pgident for
On Tue, Jan 3, 2023 at 11:10 AM wangw.f...@fujitsu.com
wrote:
>
> On Mon, Jan 2, 2023 at 18:54 PM Amit Kapila wrote:
> > On Fri, Dec 30, 2022 at 3:55 PM wangw.f...@fujitsu.com
> > wrote:
> > >
> > > I've checked it and it looks good to me.
> > > Rebased the other patches and ran the pgident for
On Tue, Dec 27, 2022 at 10:24 AM wangw.f...@fujitsu.com
wrote:
>
> Attach the new version patch which addressed all above comments and part of
> comments from [1] except one comment that are being discussed.
>
1.
+# Test that the deadlock is detected among leader and parallel apply workers.
+
On Tue, Dec 27, 2022 at 11:28 AM Masahiko Sawada wrote:
>
> On Mon, Dec 26, 2022 at 10:29 PM Amit Kapila wrote:
> >
> > On Mon, Dec 26, 2022 at 6:33 PM Masahiko Sawada
> > wrote:
> > >
> > > ---
> > > +if (!pa_can_start(xid))
> > > +return;
> > > +
> > > +/*
On Mon, Dec 26, 2022 at 10:29 PM Amit Kapila wrote:
>
> On Mon, Dec 26, 2022 at 6:33 PM Masahiko Sawada wrote:
> >
> > ---
> > +if (!pa_can_start(xid))
> > +return;
> > +
> > +/* First time through, initialize parallel apply worker state
> > hashtable. */
> > +
On Tue, Dec 27, 2022 at 10:36 AM Dilip Kumar wrote:
>
> On Tue, Dec 27, 2022 at 9:15 AM Amit Kapila wrote:
> >
> > On Mon, Dec 26, 2022 at 7:35 PM Dilip Kumar wrote:
> > >
> > > In the commit message, there is a statement like this
> > >
> > > "However, if the leader apply worker times out
On Tue, Dec 27, 2022 at 9:15 AM Amit Kapila wrote:
>
> On Mon, Dec 26, 2022 at 7:35 PM Dilip Kumar wrote:
> >
> > In the commit message, there is a statement like this
> >
> > "However, if the leader apply worker times out while attempting to
> > send a message to the
> > parallel apply worker,
On Mon, Dec 26, 2022 21:02 PM Masahiko Sawada wrote:
> Thank you for updating the patches. Here are some comments for 0001
> and 0002 patches:
Thanks for your comments.
> I think it'd be better to write logs when the leader enters the
> serialization mode. It would be helpful for investigating
On Mon, Dec 26, 2022 at 7:35 PM Dilip Kumar wrote:
>
> In the commit message, there is a statement like this
>
> "However, if the leader apply worker times out while attempting to
> send a message to the
> parallel apply worker, it will switch to "partial serialize" mode - in this
> mode the
On Mon, Dec 26, 2022 at 6:59 PM Amit Kapila wrote:
>
In the commit message, there is a statement like this
"However, if the leader apply worker times out while attempting to
send a message to the
parallel apply worker, it will switch to "partial serialize" mode - in this
mode the leader
On Mon, Dec 26, 2022 at 6:33 PM Masahiko Sawada wrote:
>
> ---
> +if (!pa_can_start(xid))
> +return;
> +
> +/* First time through, initialize parallel apply worker state
> hashtable. */
> +if (!ParallelApplyTxnHash)
> +{
> +HASHCTL
On Mon, Dec 26, 2022 at 1:22 PM houzj.f...@fujitsu.com
wrote:
>
> On Friday, December 23, 2022 5:20 PM houzj.f...@fujitsu.com
> wrote:
> >
> > I noticed a CFbot failure in one of the new testcases in 015_stream.pl which
> > comes from old 032_xx.pl. It's because I slightly adjusted the change
On Mon, Dec 26, 2022 at 9:52 AM houzj.f...@fujitsu.com
wrote:
>
> On Friday, December 23, 2022 5:20 PM houzj.f...@fujitsu.com
> wrote:
>
> Since the GUC used to force stream changes has been committed, I removed that
> patch from the patch set here and rebased the testcases based on that
On Fri, Dec 23, 2022 at 12:21 PM Amit Kapila wrote:
>
> On Thu, Dec 22, 2022 at 6:18 PM Masahiko Sawada wrote:
> >
> > On Thu, Dec 22, 2022 at 7:04 PM Amit Kapila wrote:
> > >
> > > On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > Thank you for updating the patch.
On Thu, Dec 22, 2022 at 6:18 PM Masahiko Sawada wrote:
>
> On Thu, Dec 22, 2022 at 7:04 PM Amit Kapila wrote:
> >
> > On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada
> > wrote:
> > >
> > > Thank you for updating the patch. Here are some comments on v64 patches:
> > >
> > > While testing the
On Thu, Dec 22, 2022 at 7:04 PM Amit Kapila wrote:
>
> On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada
> wrote:
> >
> > Thank you for updating the patch. Here are some comments on v64 patches:
> >
> > While testing the patch, I realized that if all streamed transactions
> > are handled by
On Wed, Dec 21, 2022 at 11:02 AM houzj.f...@fujitsu.com
wrote:
>
> Attach the new patch set which also includes some
> cosmetic comment changes.
>
Few minor comments:
=
1.
+ t = spill the changes of in-progress
transactions to+ disk and apply at once after the
On Thu, Dec 22, 2022 at 11:39 AM Masahiko Sawada wrote:
>
> Thank you for updating the patch. Here are some comments on v64 patches:
>
> While testing the patch, I realized that if all streamed transactions
> are handled by parallel workers, there is no chance for the leader to
> call
On Wed, Dec 21, 2022 at 2:32 PM houzj.f...@fujitsu.com
wrote:
>
> On Wed, Dec 21, 2022 9:07 AM Peter Smith wrote:
> > FYI - applying v63-0001 using the latest master does not work.
> >
> > git apply
> > ../patches_misc/v63-0001-Perform-streaming-logical-transactions-by-
> > parall.patch
> >
On Wed, Dec 21, 2022 at 11:02 AM houzj.f...@fujitsu.com
wrote:
>
>
> Attach the new patch set which also includes some
> cosmetic comment changes.
>
I noticed one problem with the recent change in the patch.
+ * The fileset state should become FS_SERIALIZE_DONE once we have waited
+ * for a
On Tue, Dec 20, 2022 at 5:22 PM Peter Smith wrote:
>
> On Tue, Dec 20, 2022 at 2:20 PM Amit Kapila wrote:
> >
> > On Tue, Dec 20, 2022 at 8:17 AM Peter Smith wrote:
> > >
> > > Summary
> > > ---
> > >
> > > In summary, everything I have tested so far appeared to be working
> > > properly.
FYI - applying v63-0001 using the latest master does not work.
git apply
../patches_misc/v63-0001-Perform-streaming-logical-transactions-by-parall.patch
error: patch failed: src/backend/replication/logical/meson.build:1
error: src/backend/replication/logical/meson.build: patch does not apply
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.
> >
>
> Few comments:
>
> 1.
> + if
On Mon, Dec 19, 2022 at 6:17 PM Amit Kapila wrote:
>
> 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.
> >
>
> Few comments:
>
>
Few more minor
On Tue, Dec 20, 2022 at 2:20 PM Amit Kapila wrote:
>
> On Tue, Dec 20, 2022 at 8:17 AM Peter Smith wrote:
> >
> > Summary
> > ---
> >
> > In summary, everything I have tested so far appeared to be working
> > properly. In other words, for overlapping streamed transactions of
> > different
On Tue, Dec 20, 2022 at 8:17 AM Peter Smith wrote:
>
> Summary
> ---
>
> In summary, everything I have tested so far appeared to be working
> properly. In other words, for overlapping streamed transactions of
> different kinds, and regardless of whether zero/some/all of those
> transactions
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.
>
Few comments:
1.
+ if (fileset_state == FS_SERIALIZE_IN_PROGRESS)
+ {
+
On Fri, Dec 16, 2022 at 4:34 PM Amit Kapila wrote:
>
> On Fri, Dec 16, 2022 at 2:47 PM houzj.f...@fujitsu.com
> wrote:
> >
> > > ---
> > > + active_workers = list_copy(ParallelApplyWorkerPool);
> > > +
> > > + foreach(lc, active_workers)
> > > + {
> > > + int
On Fri, Dec 16, 2022 at 2:47 PM houzj.f...@fujitsu.com
wrote:
>
> > ---
> > + active_workers = list_copy(ParallelApplyWorkerPool);
> > +
> > + foreach(lc, active_workers)
> > + {
> > + int slot_no;
> > + uint16 generation;
On Thu, Dec 15, 2022 at 6:29 PM Amit Kapila wrote:
>
I have noticed that the origin information of the rollback is not
restored after restart of the server. So, the apply worker will send
the old origin information in that case. It seems we need the below
change in XactLogAbortRecord(). What do
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
On Thu, Dec 15, 2022 at 12:28 PM houzj.f...@fujitsu.com
wrote:
>
> On Wednesday, December 14, 2022 2:49 PM Amit Kapila
> wrote:
>
> >
> > On Wed, Dec 14, 2022 at 9:50 AM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Tuesday, December 13, 2022 11:25 PM Masahiko Sawada
> > wrote:
> > > >
> >
On Thu, Dec 15, 2022 at 8:58 AM houzj.f...@fujitsu.com
wrote:
>
Few minor comments:
=
1.
+ for (i = list_length(subxactlist) - 1; i >= 0; i--)
+ {
+ TransactionId xid_tmp = lfirst_xid(list_nth_cell(subxactlist, i));
+
+ if (xid_tmp == subxid)
+ {
+ RollbackToSavepoint(spname);
+
On Wed, Dec 14, 2022 at 3:48 PM Amit Kapila wrote:
>
> On Wed, Dec 14, 2022 at 9:50 AM houzj.f...@fujitsu.com
> wrote:
> >
> > On Tuesday, December 13, 2022 11:25 PM Masahiko Sawada
> > wrote:
> > >
> > > Here are comments on v59 0001, 0002 patches:
> >
> > Thanks for the comments!
> >
> > >
On Wed, Dec 14, 2022 at 1:20 PM houzj.f...@fujitsu.com
wrote:
>
> 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
On Wed, Dec 14, 2022 at 9:50 AM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, December 13, 2022 11:25 PM Masahiko Sawada
> wrote:
> >
> > Here are comments on v59 0001, 0002 patches:
>
> Thanks for the comments!
>
> > +void
> > +pa_increment_stream_block(ParallelApplyWorkerShared *wshared) {
>
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:37 PM houzj.f...@fujitsu.com
> > > 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:37 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> >
> > Review comments
>
> Thanks for the comments!
>
> > ==
> > 1. Currently,
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
On Tue, Dec 13, 2022 at 4:36 AM Peter Smith wrote:
>
> ~~~
>
> 3. pa_set_stream_apply_worker
>
> +/*
> + * Set the worker that required to apply the current streaming transaction.
> + */
> +void
> +pa_set_stream_apply_worker(ParallelApplyWorkerInfo *winfo)
> +{
> + stream_apply_worker = winfo;
>
Some minor review comments for v58-0001
==
.../replication/logical/applyparallelworker.c
1. pa_can_start
+ /*
+ * Don't start a new parallel worker if user has set skiplsn as it's
+ * possible that user want to skip the streaming transaction. For streaming
+ * transaction, we need to
FYI - a rebase is needed.
This patch is currently failing in cfbot [1], probably due to recent
logical replication documentation updates [2].
--
[1] cfbot failing for v59 - http://cfbot.cputube.org/patch_41_3621.log
[2] PGDOCS updated -
On Fri, Dec 9, 2022 at 3:05 PM Amit Kapila wrote:
>
> On Fri, Dec 9, 2022 at 7:45 AM Peter Smith wrote:
> >
> > On Thu, Dec 8, 2022 at 7:43 PM Masahiko Sawada
> > wrote:
> > >
> > > On Thu, Dec 8, 2022 at 4:42 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Thu, Dec 8, 2022 at 12:42 PM
On Thu, Dec 8, 2022 at 12:37 PM houzj.f...@fujitsu.com
wrote:
>
Review comments
==
1. Currently, we don't release the stream lock in LA (leade apply
worker) for "rollback to savepoint" and the reason is mentioned in
comments of apply_handle_stream_abort() in the patch. But, today,
On Fri, Dec 9, 2022 at 7:45 AM Peter Smith wrote:
>
> On Thu, Dec 8, 2022 at 7:43 PM Masahiko Sawada wrote:
> >
> > 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
On Thu, Dec 8, 2022 at 7:43 PM Masahiko Sawada wrote:
>
> 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
101 - 200 of 488 matches
Mail list logo
