On Wed, 10 Jan 2024 at 15:04, Amit Kapila wrote:
>
> On Wed, Jan 10, 2024 at 2:59 PM Shlok Kyal wrote:
> >
> > This patch is not applying on the HEAD. Please rebase and share the
> > updated patch.
> >
>
> IIRC, there were some regressions observed with this patch. So, one
> needs to analyze thos
On Wed, Jan 10, 2024 at 2:59 PM Shlok Kyal wrote:
>
> This patch is not applying on the HEAD. Please rebase and share the
> updated patch.
>
IIRC, there were some regressions observed with this patch. So, one
needs to analyze those as well. I think we should mark it as "Returned
with feedback".
Hi,
This patch is not applying on the HEAD. Please rebase and share the
updated patch.
Thanks and Regards
Shlok Kyal
On Wed, 10 Jan 2024 at 14:55, Peter Smith wrote:
>
> Oops - now with attachments
>
> On Mon, Aug 21, 2023 at 5:56 PM Peter Smith wrote:
>>
>> Hi Melih,
>>
>> Last week we revisi
Oops - now with attachments
On Mon, Aug 21, 2023 at 5:56 PM Peter Smith wrote:
> Hi Melih,
>
> Last week we revisited your implementation of design#2. Vignesh rebased
> it, and then made a few other changes.
>
> PSA v28*
>
> The patch changes include:
> * changed the logic slightly by setting re
Hi Melih,
Last week we revisited your implementation of design#2. Vignesh rebased it,
and then made a few other changes.
PSA v28*
The patch changes include:
* changed the logic slightly by setting recv_immediately(new variable), if
this variable is set the main apply worker loop will not wait in
On Fri, Aug 11, 2023 at 11:45 PM Melih Mutlu wrote:
>
> Again, I couldn't reproduce the cases where you saw significantly degraded
> performance. I wonder if I'm missing something. Did you do anything not
> included in the test scripts you shared? Do you think v26-0001 will perform
> 84% worse
Here is another review comment about patch v26-0001.
The tablesync worker processes include the 'relid' as part of their
name. See launcher.c:
snprintf(bgw.bgw_name, BGW_MAXLEN,
"logical replication tablesync worker for subscription %u sync %u",
subid,
relid);
~~
And if that worker
On Thu, 10 Aug 2023 at 10:16, Amit Kapila wrote:
>
> On Wed, Aug 9, 2023 at 8:28 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Thursday, August 3, 2023 7:30 PM Melih Mutlu
> > wrote:
> >
> > > Right. I attached the v26 as you asked.
> >
> > Thanks for posting the patches.
> >
> > While reviewing
On Thu, Aug 10, 2023 at 10:15 AM Amit Kapila wrote:
>
> On Wed, Aug 9, 2023 at 8:28 AM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Thursday, August 3, 2023 7:30 PM Melih Mutlu
> > wrote:
> >
> > > Right. I attached the v26 as you asked.
> >
> > Thanks for posting the patches.
> >
> > While review
On Fri, Aug 11, 2023 at 7:15 PM Melih Mutlu wrote:
>
> Peter Smith , 11 Ağu 2023 Cum, 01:26 tarihinde şunu
> yazdı:
>>
>> No, I meant what I wrote there. When I ran the tests the HEAD included
>> the v25-0001 refactoring patch, but v26 did not yet exist.
>>
>> For now, we are only performance tes
Hi Peter,
Peter Smith , 11 Ağu 2023 Cum, 01:26 tarihinde şunu
yazdı:
> No, I meant what I wrote there. When I ran the tests the HEAD included
> the v25-0001 refactoring patch, but v26 did not yet exist.
>
> For now, we are only performance testing the first
> "Reuse-Tablesyc-Workers" patch, but n
On Fri, 11 Aug 2023 at 16:26, vignesh C wrote:
>
> On Wed, 9 Aug 2023 at 09:51, vignesh C wrote:
> >
> > Hi Melih,
> >
> > Here is a patch to help in getting the execution at various phases
> > like: a) replication slot creation time, b) Wal reading c) Number of
> > WAL records read d) subscripti
On Wed, 9 Aug 2023 at 09:51, vignesh C wrote:
>
> Hi Melih,
>
> Here is a patch to help in getting the execution at various phases
> like: a) replication slot creation time, b) Wal reading c) Number of
> WAL records read d) subscription relation state change etc
> Couple of observation while we te
On Fri, Aug 11, 2023 at 12:54 AM Melih Mutlu wrote:
>
> Hi Peter and Vignesh,
>
> Peter Smith , 7 Ağu 2023 Pzt, 09:25 tarihinde şunu
> yazdı:
>>
>> Hi Melih.
>>
>> Now that the design#1 ERRORs have been fixed, we returned to doing
>> performance measuring of the design#1 patch versus HEAD.
>
>
>
Hi Peter and Vignesh,
Peter Smith , 7 Ağu 2023 Pzt, 09:25 tarihinde şunu
yazdı:
> Hi Melih.
>
> Now that the design#1 ERRORs have been fixed, we returned to doing
> performance measuring of the design#1 patch versus HEAD.
Thanks a lot for taking the time to benchmark the patch. It's really
help
Hi Melih,
FYI -- The same testing was repeated but this time PG was configured
to say synchronous_commit=on. Other factors and scripts were the same
as before --- busy apply, 5 runs, 4 workers, 1000 inserts/tx, 100
empty tables, etc.
There are still more xlog records seen for the v26 patch, but n
On Wed, Aug 9, 2023 at 8:28 AM Zhijie Hou (Fujitsu)
wrote:
>
> On Thursday, August 3, 2023 7:30 PM Melih Mutlu
> wrote:
>
> > Right. I attached the v26 as you asked.
>
> Thanks for posting the patches.
>
> While reviewing the patch, I noticed one rare case that it's possible that
> there
> are
Hi Melih,
Here is a patch to help in getting the execution at various phases
like: a) replication slot creation time, b) Wal reading c) Number of
WAL records read d) subscription relation state change etc
Couple of observation while we tested with this patch:
1) We noticed that the patch takes mor
On Thursday, August 3, 2023 7:30 PM Melih Mutlu wrote:
> Right. I attached the v26 as you asked.
Thanks for posting the patches.
While reviewing the patch, I noticed one rare case that it's possible that there
are two table sync worker for the same table in the same time.
The patch relies o
Hi Melih.
Now that the design#1 ERRORs have been fixed, we returned to doing
performance measuring of the design#1 patch versus HEAD.
Unfortunately, we observed that under some particular conditions
(large transactions of 1000 inserts/tx for a busy apply worker, 100
empty tables to be synced) the
FWIW, I confirmed that my review comments for v22* have all been
addressed in the latest v26* patches.
Thanks!
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Hi,
Amit Kapila , 3 Ağu 2023 Per, 09:22 tarihinde şunu
yazdı:
> On Thu, Aug 3, 2023 at 9:35 AM Peter Smith wrote:
> > I checked the latest patch v25-0001.
> >
> > LGTM.
> >
>
> Thanks, I have pushed 0001. Let's focus on the remaining patches.
>
Thanks!
Peter Smith , 3 Ağu 2023 Per, 12:06 tarih
Just to clarify my previous post, I meant we will need new v26* patches
v24-0001 -> not needed because v25-0001 pushed
v24-0002 -> v26-0001
v24-0003 -> v26-0002
On Thu, Aug 3, 2023 at 6:19 PM Peter Smith wrote:
>
> Hi Melih,
>
> Now that v25-0001 has been pushed, can you please rebase the remain
Hi Melih,
Now that v25-0001 has been pushed, can you please rebase the remaining patches?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Aug 3, 2023 at 9:35 AM Peter Smith wrote:
>
> On Wed, Aug 2, 2023 at 11:19 PM Amit Kapila wrote:
> >
> > On Wed, Aug 2, 2023 at 4:09 PM Melih Mutlu wrote:
> > >
> > > PFA an updated version with some of the earlier reviews addressed.
> > > Forgot to include them in the previous email.
>
On Wed, Aug 2, 2023 at 11:19 PM Amit Kapila wrote:
>
> On Wed, Aug 2, 2023 at 4:09 PM Melih Mutlu wrote:
> >
> > PFA an updated version with some of the earlier reviews addressed.
> > Forgot to include them in the previous email.
> >
>
> It is always better to explicitly tell which reviews are ad
On Wed, Aug 2, 2023 at 4:09 PM Melih Mutlu wrote:
>
> PFA an updated version with some of the earlier reviews addressed.
> Forgot to include them in the previous email.
>
It is always better to explicitly tell which reviews are addressed but
anyway, I have done some minor cleanup in the 0001 patc
Hi,
>
PFA an updated version with some of the earlier reviews addressed.
Forgot to include them in the previous email.
Thanks,
--
Melih Mutlu
Microsoft
v24-0003-Reuse-connection-when-tablesync-workers-change-t.patch
Description: Binary data
v24-0002-Reuse-Tablesync-Workers.patch
Description:
Hi,
Amit Kapila , 2 Ağu 2023 Çar, 12:01 tarihinde şunu
yazdı:
> I think we are getting the error (ERROR: could not find logical
> decoding starting point) because we wouldn't have waited for WAL to
> become available before reading it. It could happen due to the
> following code:
> WalSndWaitFor
On Tue, Aug 1, 2023 at 9:44 AM Peter Smith wrote:
>
>
> FYI, here is some more information about ERRORs seen.
>
> The patches were re-tested -- applied in stages (and also against the
> different scripts) to identify where the problem was introduced. Below
> are the observations:
>
> ~~~
>
> Using
On Tue, 1 Aug 2023 at 09:44, Peter Smith wrote:
>
> On Fri, Jul 28, 2023 at 5:22 PM Peter Smith wrote:
> >
> > Hi Melih,
> >
> > BACKGROUND
> > --
> >
> > We wanted to compare performance for the 2 different reuse-worker
> > designs, when the apply worker is already busy handling other
>
On Fri, Jul 28, 2023 at 5:22 PM Peter Smith wrote:
>
> Hi Melih,
>
> BACKGROUND
> --
>
> We wanted to compare performance for the 2 different reuse-worker
> designs, when the apply worker is already busy handling other
> replications, and then simultaneously the test table tablesyncs are
>
On Thu, Jul 27, 2023 at 11:30 PM Melih Mutlu wrote:
>
> Hi Peter,
>
> Peter Smith , 26 Tem 2023 Çar, 07:40 tarihinde şunu
> yazdı:
>>
>> Here are some comments for patch v22-0001.
>>
>> ==
>> 1. General -- naming conventions
>>
>> There is quite a lot of inconsistency with variable/parameter
Hi Peter,
Peter Smith , 26 Tem 2023 Çar, 07:40 tarihinde şunu
yazdı:
> Here are some comments for patch v22-0001.
>
> ==
> 1. General -- naming conventions
>
> There is quite a lot of inconsistency with variable/parameter naming
> styles in this patch. I understand in most cases the names are
On Thu, Jul 27, 2023 at 6:46 AM Peter Smith wrote:
>
> Here are some review comments for v22-0003
>
> ==
>
> 1. ApplicationNameForTablesync
> +/*
> + * Determine the application_name for tablesync workers.
> + *
> + * Previously, the replication slot name was used as application_name. Since
>
Here are some review comments for v22-0003
==
1. ApplicationNameForTablesync
+/*
+ * Determine the application_name for tablesync workers.
+ *
+ * Previously, the replication slot name was used as application_name. Since
+ * it's possible to reuse tablesync workers now, a tablesync worker can
Here are some review comments for v22-0002
==
1. General - errmsg
AFAIK, the errmsg part does not need to be enclosed by extra parentheses.
e.g.
BEFORE
ereport(LOG,
(errmsg("logical replication table synchronization worker for
subscription \"%s\" has finished",
MySubscription->name)));
AFTER
On Wed, Jul 26, 2023 at 10:10 AM Peter Smith wrote:
>
> Here are some comments for patch v22-0001.
>
> ==
> 1. General -- naming conventions
>
> There is quite a lot of inconsistency with variable/parameter naming
> styles in this patch. I understand in most cases the names are copied
> unchan
Here are some comments for patch v22-0001.
==
1. General -- naming conventions
There is quite a lot of inconsistency with variable/parameter naming
styles in this patch. I understand in most cases the names are copied
unchanged from the original functions. Still, since this is a big
refactor
Hi,
Melih Mutlu , 21 Tem 2023 Cum, 12:47 tarihinde şunu
yazdı:
> I did not realize the order is the same with .c files. Good to know. I'll
> fix it along with other comments.
>
Addressed the recent reviews and attached the updated patches.
Thanks,
--
Melih Mutlu
Microsoft
v22-0001-Refactor-t
Peter Smith , 21 Tem 2023 Cum, 12:48 tarihinde şunu
yazdı:
> On Fri, Jul 21, 2023 at 5:24 PM Amit Kapila
> wrote:
> >
> > On Fri, Jul 21, 2023 at 12:05 PM Peter Smith
> wrote:
> > >
> > > On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila
> wrote:
> > > >
>
> > > > The other thing I noticed is that we
On Fri, Jul 21, 2023 at 5:24 PM Amit Kapila wrote:
>
> On Fri, Jul 21, 2023 at 12:05 PM Peter Smith wrote:
> >
> > On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila wrote:
> > >
> > > The other thing I noticed is that we
> > > don't seem to be consistent in naming functions in these files. For
> > >
Amit Kapila , 21 Tem 2023 Cum, 08:39 tarihinde
şunu yazdı:
> On Fri, Jul 21, 2023 at 7:30 AM Peter Smith wrote:
> How about SetupLogRepWorker? The other thing I noticed is that we
> don't seem to be consistent in naming functions in these files. For
> example, shall we make all exposed functions
On Fri, Jul 21, 2023 at 12:05 PM Peter Smith wrote:
>
> On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila wrote:
> >
> > On Fri, Jul 21, 2023 at 7:30 AM Peter Smith wrote:
> > >
> > > ~~~
> > >
> > > 2. StartLogRepWorker
> > >
> > > /* Common function to start the leader apply or tablesync worker. */
On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila wrote:
>
> On Fri, Jul 21, 2023 at 7:30 AM Peter Smith wrote:
> >
> > ~~~
> >
> > 2. StartLogRepWorker
> >
> > /* Common function to start the leader apply or tablesync worker. */
> > void
> > StartLogRepWorker(int worker_slot)
> > {
> > /* Attach to sl
On Fri, Jul 21, 2023 at 7:30 AM Peter Smith wrote:
>
> ~~~
>
> 2. StartLogRepWorker
>
> /* Common function to start the leader apply or tablesync worker. */
> void
> StartLogRepWorker(int worker_slot)
> {
> /* Attach to slot */
> logicalrep_worker_attach(worker_slot);
>
> /* Setup signal handling
Some review comments for v21-0002.
On Thu, Jul 20, 2023 at 11:41 PM Melih Mutlu wrote:
>
> Hi,
>
> Attached the updated patches with recent reviews addressed.
>
> See below for my comments:
>
> Peter Smith , 19 Tem 2023 Çar, 06:08 tarihinde şunu
> yazdı:
>>
>> 5.
>> + /* Found a table for next i
Some review comments for v21-0001
==
src/backend/replication/logical/worker.c
1. InitializeLogRepWorker
if (am_tablesync_worker())
ereport(LOG,
- (errmsg("logical replication table synchronization worker for
subscription \"%s\", table \"%s\" has started",
+ (errmsg("logical replication w
On Thu, Jul 20, 2023 at 11:41 PM Melih Mutlu wrote:
>
> Hi,
>
> Attached the updated patches with recent reviews addressed.
>
> See below for my comments:
>
> Peter Smith , 19 Tem 2023 Çar, 06:08 tarihinde şunu
> yazdı:
>>
>> Some review comments for v19-0001
>>
>> 2. LogicalRepSyncTableStart
>>
Hi,
Attached the updated patches with recent reviews addressed.
See below for my comments:
Peter Smith , 19 Tem 2023 Çar, 06:08 tarihinde şunu
yazdı:
> Some review comments for v19-0001
>
> 2. LogicalRepSyncTableStart
>
> /*
> * Finally, wait until the leader apply worker tells us to catch up a
On Thu, Jul 20, 2023 at 5:12 PM Melih Mutlu wrote:
>
> Peter Smith , 20 Tem 2023 Per, 05:41 tarihinde şunu
> yazdı:
>>
>> 7. InitializeLogRepWorker
>>
>> if (am_tablesync_worker())
>> ereport(LOG,
>> - (errmsg("logical replication worker for subscription \"%s\", table
>> \"%s\" has started",
Hi,
Peter Smith , 20 Tem 2023 Per, 05:41 tarihinde şunu
yazdı:
> 7. InitializeLogRepWorker
>
> if (am_tablesync_worker())
> ereport(LOG,
> - (errmsg("logical replication worker for subscription \"%s\", table
> \"%s\" has started",
> + (errmsg("logical replication worker for subscription \"%s\
Hi Peter,
Peter Smith , 20 Tem 2023 Per, 07:10 tarihinde şunu
yazdı:
> Hi, I had a look at the latest 3 patch (v20-0003).
>
> Although this patch was recently modified, the updates are mostly only
> to make it compatible with the updated v20-0002 patch. Specifically,
> the v20-0003 updates di
Hi, I had a look at the latest 3 patch (v20-0003).
Although this patch was recently modified, the updates are mostly only
to make it compatible with the updated v20-0002 patch. Specifically,
the v20-0003 updates did not yet address my review comments from
v17-0003 [1].
Anyway, this post is ju
On Thu, Jul 20, 2023 at 8:02 AM Peter Smith wrote:
>
> On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
> >
> > Hi,
> >
> > PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> > reviews for 0001 and 0002 with some small comments below.
> >
> > Peter Smith , 10 Tem 2023 Pz
Some review comments for patch v20-0002
==
src/backend/replication/logical/tablesync.c
1. finish_sync_worker
/*
* Exit routine for synchronization worker.
*
* If reuse_worker is false, the worker will not be reused and exit.
*/
~
IMO the "will not be reused" part doesn't need saying --
On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
>
> Hi,
>
> PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> reviews for 0001 and 0002 with some small comments below.
>
> Peter Smith , 10 Tem 2023 Pzt, 10:09 tarihinde şunu
> yazdı:
>>
>> 6. LogicalRepApplyLoop
>>
>> +
On Wed, Jul 19, 2023 at 8:38 AM Peter Smith wrote:
>
> Some review comments for v19-0001
>
...
> ==
> src/backend/replication/logical/worker.c
>
> 3. set_stream_options
>
> +/*
> + * Sets streaming options including replication slot name and origin start
> + * position. Workers need these opti
Some review comments for v19-0001
==
src/backend/replication/logical/tablesync.c
1. run_tablesync_worker
+run_tablesync_worker(WalRcvStreamOptions *options,
+ char *slotname,
+ char *originname,
+ int originname_size,
+ XLogRecPtr *origin_startpos)
+{
+ /* Start table synchronization. */
+ st
On Tue, 11 Jul 2023 at 08:30, Peter Smith wrote:
>
> On Tue, Jul 11, 2023 at 12:31 AM Melih Mutlu wrote:
> >
> > Hi,
> >
> > Hayato Kuroda (Fujitsu) , 6 Tem 2023 Per,
> > 12:47 tarihinde şunu yazdı:
> > >
> > > Dear Melih,
> > >
> > > > Thanks for the 0003 patch. But it did not work for me. Can y
On Tue, Jul 18, 2023 at 2:33 PM Melih Mutlu wrote:
>
> Attached the fixed patchset.
>
Few comments on 0001
1.
+ logicalrep_worker_attach(worker_slot);
+
+ /* Setup signal handling */
+ pqsignal(SIGHUP, SignalHandlerForConfigReload);
+ pqsignal(SIGTERM, die);
+ BackgroundWorke
On Tue, Jul 18, 2023 at 11:25 AM Peter Smith wrote:
>
> On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
> >
> > Hi,
> >
> > PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> > reviews for 0001 and 0002 with some small comments below.
> >
>
> Thanks, I will take another
On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
>
> Hi,
>
> PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> reviews for 0001 and 0002 with some small comments below.
>
Thanks, I will take another look at these soon. FYI, the 0001 patch
does not apply cleanly. It need
On Fri, Jul 14, 2023 at 3:07 PM Melih Mutlu wrote:
>
> Amit Kapila , 14 Tem 2023 Cum, 11:11 tarihinde şunu
> yazdı:
>>
>> Yeah, it is quite surprising that Design#2 is worse than master. I
>> suspect there is something wrong going on with your Design#2 patch.
>> One area to check is whether apply
Hi,
Amit Kapila , 14 Tem 2023 Cum, 11:11 tarihinde
şunu yazdı:
> Yeah, it is quite surprising that Design#2 is worse than master. I
> suspect there is something wrong going on with your Design#2 patch.
> One area to check is whether apply worker is able to quickly assign
> the new relations to ta
Hi Kuroda-san.
Here are some review comments for the v17-0003 patch. They are all minor.
==
Commit message
1.
Previously tablesync workers establish new connections when it changes
the syncing
table, but this might have additional overhead. This patch allows to
reuse connections
instead.
~
On Fri, Jul 14, 2023 at 1:58 AM Melih Mutlu wrote:
>
> Here are some quick numbers with 100 empty tables.
>
> +--++++
> | | 2 sync workers | 4 sync workers | 8 sync workers |
> +--++---
Hi Peter,
Peter Smith , 11 Tem 2023 Sal, 05:59 tarihinde şunu
yazdı:
> Even if patches 0003 and 0002 are to be combined, I think that should
> not happen until after the "reuse" design is confirmed which way is
> best.
>
> e.g. IMO it might be easier to compare the different PoC designs for
> patc
Dear Melih,
> > > Thanks for the 0003 patch. But it did not work for me. Can you create
> > > a subscription successfully with patch 0003 applied?
> > > I get the following error: " ERROR: table copy could not start
> > > transaction on publisher: another command is already in progress".
> >
> >
On Mon, Jul 10, 2023 at 8:01 PM Melih Mutlu wrote:
>
> Hayato Kuroda (Fujitsu) , 6 Tem 2023 Per,
> 12:47 tarihinde şunu yazdı:
> >
> > Dear Melih,
> >
> > > Thanks for the 0003 patch. But it did not work for me. Can you create
> > > a subscription successfully with patch 0003 applied?
> > > I get
On Tue, Jul 11, 2023 at 12:31 AM Melih Mutlu wrote:
>
> Hi,
>
> Hayato Kuroda (Fujitsu) , 6 Tem 2023 Per,
> 12:47 tarihinde şunu yazdı:
> >
> > Dear Melih,
> >
> > > Thanks for the 0003 patch. But it did not work for me. Can you create
> > > a subscription successfully with patch 0003 applied?
> >
Here are some review comments for patch v16-3
==
1. Commit Message.
The patch description is missing.
==
2. General.
+LogicalRepSyncTableStart(XLogRecPtr *origin_startpos, int worker_slot)
and
+start_table_sync(XLogRecPtr *origin_startpos,
+ char **myslotname,
+ int worker_slot)
Hi,
Hayato Kuroda (Fujitsu) , 6 Tem 2023 Per,
12:47 tarihinde şunu yazdı:
>
> Dear Melih,
>
> > Thanks for the 0003 patch. But it did not work for me. Can you create
> > a subscription successfully with patch 0003 applied?
> > I get the following error: " ERROR: table copy could not start
> > tra
Hi,
Amit Kapila , 6 Tem 2023 Per, 06:56 tarihinde
şunu yazdı:
>
> On Wed, Jul 5, 2023 at 1:48 AM Melih Mutlu wrote:
> >
> > Hayato Kuroda (Fujitsu) , 4 Tem 2023 Sal,
> > 08:42 tarihinde şunu yazdı:
> > > > > But in the later patch the tablesync worker tries to reuse the slot
> > > > > during the
Hi, here are some review comments for patch v16-0002.
==
Commit message
1.
This commit allows reusing tablesync workers for syncing more than one
table sequentially during their lifetime, instead of exiting after
only syncing one table.
Before this commit, tablesync workers were capable of s
Dear hackers,
Hi, I did a performance testing for v16 patch set.
Results show that patches significantly improves the performance in most cases.
# Method
Following tests were done 10 times per condition, and compared by median.
do_one_test.sh was used for the testing.
1. Create tables on p
Hi. Here are some review comments for the patch v16-0001
==
Commit message.
1.
Also; most of the code shared by both worker types are already combined
in LogicalRepApplyLoop(). There is no need to combine the rest in
ApplyWorkerMain() anymore.
~
/are already/is already/
/Also;/Also,/
~~~
On Wed, Jul 5, 2023 at 1:48 AM Melih Mutlu wrote:
>
> Hayato Kuroda (Fujitsu) , 4 Tem 2023 Sal,
> 08:42 tarihinde şunu yazdı:
> > > > But in the later patch the tablesync worker tries to reuse the slot
> > > > during the
> > > > synchronization, so in this case the application_name should be same
Hayato Kuroda (Fujitsu) , 4 Tem 2023 Sal,
08:42 tarihinde şunu yazdı:
> > > But in the later patch the tablesync worker tries to reuse the slot
> > > during the
> > > synchronization, so in this case the application_name should be same as
> > slotname.
> > >
> >
> > Fair enough. I am slightly afra
On Wed, 28 Jun 2023 at 12:02, Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Amit,
>
> > > > This actually makes sense. I quickly try to do that without adding any
> > > > new replication message. As you would expect, it did not work.
> > > > I don't really know what's needed to make a connection to last
On Mon, Jul 3, 2023 at 9:42 AM Amit Kapila wrote:
>
> On Wed, Jun 28, 2023 at 12:02 PM Hayato Kuroda (Fujitsu)
> wrote:
>
> > But in the later patch the tablesync worker tries to reuse the slot during
> > the
> > synchronization, so in this case the application_name should be same as
> > slotna
On Wed, Jun 28, 2023 at 12:02 PM Hayato Kuroda (Fujitsu)
wrote:
>
> > > I have analyzed how we handle this. Please see attached the patch (0003)
> > > which
> > > allows reusing connection.
> > >
> >
> > Why did you change the application name during the connection?
>
> It was because the lifetim
On Tue, Jun 27, 2023 at 1:12 PM Hayato Kuroda (Fujitsu)
wrote:
>
> > This actually makes sense. I quickly try to do that without adding any
> > new replication message. As you would expect, it did not work.
> > I don't really know what's needed to make a connection to last for
> > more than one it
Dear Melih,
Thanks for updating the patch. Followings are my comments.
Note that some lines exceeds 80 characters and some other lines seem too short.
And comments about coding conventions were skipped.
0001
01. logicalrep_worker_launch()
```
if (is_parallel_apply_worker)
+ {
On Fri, Jun 23, 2023 at 7:03 PM Melih Mutlu wrote:
>
> You can find the updated patchset attached.
> I worked to address the reviews and made some additional changes.
>
> Let me first explain the new patchset.
> 0001: Refactors the logical replication code, mostly worker.c and
> tablesync.c. Altho
On Fri, Jun 23, 2023 at 11:50 PM Melih Mutlu wrote:
>
> > src/backend/replication/logical/worker.c
> >
> > 10. General -- run_tablesync_worker, TablesyncWorkerMain
> >
> > IMO these functions would be more appropriately reside in the
> > tablesync.c instead of the (common) worker.c. Was there some
Hi Peter,
Thanks for your reviews. I tried to apply most of them. I just have
some comments below for some of them.
Peter Smith , 14 Haz 2023 Çar, 08:45 tarihinde
şunu yazdı:
>
> 9. process_syncing_tables_for_sync
>
> @@ -378,7 +387,13 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn)
>
Hi,
Thanks for your reviews.
Hayato Kuroda (Fujitsu) , 13 Haz 2023 Sal,
13:06 tarihinde şunu yazdı:
> 01. general
>
> Why do tablesync workers have to disconnect from publisher for every
> iterations?
> I think connection initiation overhead cannot be negligible in the postgres's
> basic
> arch
Here are some review comments for the patch v2-0001.
==
Commit message
1. General
Better to use consistent terms in this message. Either "relations" or
"tables" -- not a mixture of both.
~~~
2.
Before this commit, tablesync workers were capable of syncing only one
relation. For each table,
Dear Melih,
Thank you for making the patch!
I'm also interested in the patchset. Here are the comments for 0001.
Some codes are not suit for our coding conventions, but followings do not
contain them
because patches seems in the early statge.
Moreover, 0003 needs rebase.
01. general
Why do tab
On Thu, Jun 1, 2023 6:54 PM Melih Mutlu wrote:
>
> Hi,
>
> I rebased the patch and addressed the following reviews.
>
Thanks for updating the patch. Here are some comments on 0001 patch.
1.
- ereport(LOG,
- (errmsg("logical replication table synchronization
worker
On Thu, Jun 1, 2023 at 7:22 AM Melih Mutlu wrote:
>
> Hi Peter,
>
> Peter Smith , 26 May 2023 Cum, 10:30 tarihinde
> şunu yazdı:
> >
> > On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote:
> > Yes, I was mostly referring to the same as point 1 below about patch
> > 0001. I guess I just found the c
Hi Peter,
Peter Smith , 26 May 2023 Cum, 10:30 tarihinde
şunu yazdı:
>
> On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote:
> Yes, I was mostly referring to the same as point 1 below about patch
> 0001. I guess I just found the concept of mixing A) launching TSW (via
> apply worker) with B) reass
On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote:
>
> Hi,
>
>
> Peter Smith , 24 May 2023 Çar, 05:59 tarihinde şunu
> yazdı:
>>
>> Hi, and thanks for the patch! It is an interesting idea.
>>
>> I have not yet fully read this thread, so below are only my first
>> impressions after looking at patc
Hi,
Peter Smith , 24 May 2023 Çar, 05:59 tarihinde şunu
yazdı:
> Hi, and thanks for the patch! It is an interesting idea.
>
> I have not yet fully read this thread, so below are only my first
> impressions after looking at patch 0001. Sorry if some of these were
> already discussed earlier.
>
>
Hi, and thanks for the patch! It is an interesting idea.
I have not yet fully read this thread, so below are only my first
impressions after looking at patch 0001. Sorry if some of these were
already discussed earlier.
TBH the patch "reuse-workers" logic seemed more complicated than I had
imagine
On Sun, 26 Feb 2023 at 19:11, Melanie Plageman
wrote:
>
> This is cool! Thanks for working on this.
> I had a chance to review your patchset and I had some thoughts and
> questions.
It looks like this patch got a pretty solid review from Melanie
Plageman in February just before the CF started. It
On Wed, Feb 22, 2023 at 8:04 AM Melih Mutlu wrote:
>
> Hi Wang,
>
> Thanks for reviewing.
> Please see updated patches. [1]
This is cool! Thanks for working on this.
I had a chance to review your patchset and I had some thoughts and
questions.
I notice that you've added a new user-facing option
Hi Wang,
Thanks for reviewing.
Please see updated patches. [1]
wangw.f...@fujitsu.com , 7 Şub 2023 Sal, 10:28
tarihinde şunu yazdı:
> 1. In the function ApplyWorkerMain.
> I think we need to keep the worker name as "leader apply worker" in the
> comment
> like the current HEAD.
>
Done.
> I th
Hi Shveta,
Thanks for reviewing.
Please see attached patches.
shveta malik , 2 Şub 2023 Per, 14:31 tarihinde şunu
yazdı:
> On Wed, Feb 1, 2023 at 5:37 PM Melih Mutlu wrote:
> for (int64 i = 1; i <= lastusedid; i++)
> {
> charoriginname_to_drop[NAMEDAT
1 - 100 of 153 matches
Mail list logo
