Re: [PATCH] Reuse Workers and Replication Slots during Logical Replication
Peter Smith <smithpb2250@gmail.com>
From: Peter Smith <smithpb2250@gmail.com>
To: Melih Mutlu <m.melihmutlu@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>,
Melanie Plageman <melanieplageman@gmail.com>, "Wei Wang (Fujitsu)" <wangw.fnst@fujitsu.com>,
"Yu Shi (Fujitsu)" <shiy.fnst@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>,
shveta malik <shveta.malik@gmail.com>
Date: 2023-07-21T01:48:48Z
Lists: pgsql-hackers
On Thu, Jul 20, 2023 at 11:41 PM Melih Mutlu <m.melihmutlu@gmail.com> wrote: > > Hi, > > Attached the updated patches with recent reviews addressed. > > See below for my comments: > > Peter Smith <smithpb2250@gmail.com>, 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 and >> * then return to let LogicalRepApplyLoop do it. >> */ >> wait_for_worker_state_change(SUBREL_STATE_CATCHUP); >> >> ~ >> >> Should LogicalRepApplyLoop still be mentioned here, since that is >> static in worker.c? Maybe it is better to refer instead to the common >> 'start_apply' wrapper? (see also #5a below) > > > Isn't' LogicalRepApplyLoop static on HEAD and also mentioned in the exact comment in tablesync.c while the common "start_apply" function also exists? I'm not sure how such a change would be related to this patch. > Fair enough. I thought it was questionable for one module to refer to another module's static functions, but you are correct - it is not really related to your patch. Sorry for the noise. ------ Kind Regards, Peter Smith. Fujitsu Australia
Commits
-
Refactor to split Apply and Tablesync Workers code.
- 02c1b64fb15c 17.0 landed
-
Fix possible logical replication crash.
- e7e7da2f8d57 16.0 cited