RE: Synchronizing slots from primary to standby

2024-02-22 Thread Zhijie Hou (Fujitsu)
Hi, Since the slotsync worker patch has been committed, I rebased the remaining patches. And here is the V95 patch set. Also, I fixed a bug in the current 0001 patch where the member of the standby slot names list pointed to the freed memory after calling ProcessConfigFile(). Now, we will

RE: Synchronizing slots from primary to standby

2024-02-22 Thread Zhijie Hou (Fujitsu)
On Thursday, February 22, 2024 8:41 PM Amit Kapila wrote: > > On Thu, Feb 22, 2024 at 5:23 PM Amit Kapila > wrote: > > > > On Thu, Feb 22, 2024 at 4:35 PM Bertrand Drouvot > > wrote: > > > > > > On Thu, Feb 22, 2024 at 04:01:34PM +0530, shveta malik wrote: > > > > On Thu, Feb 22, 2024 at 3:44 

Re: Synchronizing slots from primary to standby

2024-02-22 Thread Amit Kapila
On Thu, Feb 22, 2024 at 5:23 PM Amit Kapila wrote: > > On Thu, Feb 22, 2024 at 4:35 PM Bertrand Drouvot > wrote: > > > > On Thu, Feb 22, 2024 at 04:01:34PM +0530, shveta malik wrote: > > > On Thu, Feb 22, 2024 at 3:44 PM Bertrand Drouvot > > > wrote: > > > > > > > > Hi, > > > > > > > > Thanks!

Re: Synchronizing slots from primary to standby

2024-02-22 Thread Amit Kapila
On Thu, Feb 22, 2024 at 4:35 PM Bertrand Drouvot wrote: > > On Thu, Feb 22, 2024 at 04:01:34PM +0530, shveta malik wrote: > > On Thu, Feb 22, 2024 at 3:44 PM Bertrand Drouvot > > wrote: > > > > > > Hi, > > > > > > Thanks! > > > > > > Some random comments about v92_001 (Sorry if it has already

Re: Synchronizing slots from primary to standby

2024-02-22 Thread Bertrand Drouvot
Hi, On Thu, Feb 22, 2024 at 04:01:34PM +0530, shveta malik wrote: > On Thu, Feb 22, 2024 at 3:44 PM Bertrand Drouvot > wrote: > > > > Hi, > > > > Thanks! > > > > Some random comments about v92_001 (Sorry if it has already been discussed > > up-thread): > > Thanks for the feedback. The patch is

Re: Synchronizing slots from primary to standby

2024-02-22 Thread shveta malik
On Thu, Feb 22, 2024 at 3:44 PM Bertrand Drouvot wrote: > > Hi, > > Thanks! > > Some random comments about v92_001 (Sorry if it has already been discussed > up-thread): Thanks for the feedback. The patch is pushed 15 minutes back. I will prepare a top-up patch for your comments. > 1 === > > +

Re: Synchronizing slots from primary to standby

2024-02-22 Thread Bertrand Drouvot
Hi, On Thu, Feb 22, 2024 at 12:16:34PM +0530, shveta malik wrote: > On Thu, Feb 22, 2024 at 10:31 AM shveta malik wrote: > There was a recent commit 801792e to improve error messaging in > slotsync.c which resulted in conflict. Thus rebased the patch. There > is no new change in the patch

Re: Synchronizing slots from primary to standby

2024-02-21 Thread shveta malik
On Thu, Feb 22, 2024 at 10:31 AM shveta malik wrote: > > Please find patch001 attached. There is a CFBot failure in patch002. > The test added there needs some adjustment. We will rebase and post > rest of the patches once we fix that issue. > There was a recent commit 801792e to improve error

Re: Synchronizing slots from primary to standby

2024-02-21 Thread shveta malik
On Wed, Feb 21, 2024 at 5:19 PM Amit Kapila wrote: > > A few minor comments: Thanks for the feedback. > = > 1. > +/* > + * Is stopSignaled set in SlotSyncCtx? > + */ > +bool > +IsStopSignaledSet(void) > +{ > + bool signaled; > + > + SpinLockAcquire(>mutex); > + signaled =

Re: Synchronizing slots from primary to standby

2024-02-21 Thread Amit Kapila
On Wed, Feb 21, 2024 at 12:15 PM shveta malik wrote: > > > Thanks for the feedback. I have addressed it in v93. > A few minor comments: = 1. +/* + * Is stopSignaled set in SlotSyncCtx? + */ +bool +IsStopSignaledSet(void) +{ + bool signaled; + + SpinLockAcquire(>mutex); + signaled

Re: Synchronizing slots from primary to standby

2024-02-20 Thread shveta malik
On Tue, Feb 20, 2024 at 6:19 PM Masahiko Sawada wrote: > > On Tue, Feb 20, 2024 at 12:33 PM shveta malik wrote: > Thank you for the explanation. It makes sense to me to move the check. > > > 2. If the wal_level is not logical, the server will need to restart > anyway to change the wal_level and

Re: Synchronizing slots from primary to standby

2024-02-20 Thread Amit Kapila
On Tue, Feb 20, 2024 at 6:19 PM Masahiko Sawada wrote: > > Thank you for the explanation. It makes sense to me to move the check. > > As for ValidateSlotSyncParams() called by SlotSyncWorkerAllowed(), I > have two comments: > > 1. The error messages are not very descriptive and seem not to match

Re: Synchronizing slots from primary to standby

2024-02-20 Thread Masahiko Sawada
On Tue, Feb 20, 2024 at 12:44 PM Amit Kapila wrote: > > On Tue, Feb 20, 2024 at 8:25 AM Masahiko Sawada wrote: > > > > Some comments not related to the patch but to the existing code: > > > > --- > > It might have already been discussed but is the > > src/backend/replication/logical the right

Re: Synchronizing slots from primary to standby

2024-02-20 Thread Masahiko Sawada
On Tue, Feb 20, 2024 at 12:33 PM shveta malik wrote: > > On Tue, Feb 20, 2024 at 8:25 AM Masahiko Sawada wrote: > > > > > > I've reviewed the v91 patch. Here are random comments: > > Thanks for the comments. > > > --- > > /* > > * Checks the remote server info. > > * > > - * We ensure that

Re: Synchronizing slots from primary to standby

2024-02-19 Thread Amit Kapila
On Tue, Feb 20, 2024 at 8:25 AM Masahiko Sawada wrote: > > Some comments not related to the patch but to the existing code: > > --- > It might have already been discussed but is the > src/backend/replication/logical the right place for the slocsync.c? If > it's independent of logical

Re: Synchronizing slots from primary to standby

2024-02-19 Thread shveta malik
On Tue, Feb 20, 2024 at 8:25 AM Masahiko Sawada wrote: > > > I've reviewed the v91 patch. Here are random comments: Thanks for the comments. > --- > /* > * Checks the remote server info. > * > - * We ensure that the 'primary_slot_name' exists on the remote server and the > - * remote

Re: Synchronizing slots from primary to standby

2024-02-19 Thread Masahiko Sawada
On Mon, Feb 19, 2024 at 9:59 PM shveta malik wrote: > > On Mon, Feb 19, 2024 at 5:32 PM Amit Kapila wrote: > > > > Few comments on 0001 > > Thanks for the feedback. > > > > > 1. I think it is better to error out when the valid GUC or option is > > not set in

Re: Synchronizing slots from primary to standby

2024-02-19 Thread shveta malik
On Mon, Feb 19, 2024 at 5:32 PM Amit Kapila wrote: > > Few comments on 0001 Thanks for the feedback. > > 1. I think it is better to error out when the valid GUC or option is > not set in ensure_valid_slotsync_params() and > ensure_valid_remote_info() instead of waiting. And

Re: Synchronizing slots from primary to standby

2024-02-19 Thread Amit Kapila
On Mon, Feb 19, 2024 at 9:46 AM shveta malik wrote: > > Okay I see. Thanks for pointing it out. Here are the patches > addressing your comments. Changes are in patch001, rest are rebased. > Few comments on 0001 1. I think it is better to error out when the valid GUC or

Re: Synchronizing slots from primary to standby

2024-02-18 Thread Bertrand Drouvot
Hi, On Sat, Feb 17, 2024 at 10:10:18AM +0530, Amit Kapila wrote: > On Fri, Feb 16, 2024 at 4:10 PM shveta malik wrote: > > > > On Thu, Feb 15, 2024 at 10:48 PM Bertrand Drouvot > > wrote: > > > > > 5 === > > > > > > + if (SlotSyncWorker->syncing) > > > { > > > -

RE: Synchronizing slots from primary to standby

2024-02-18 Thread Zhijie Hou (Fujitsu)
On Monday, February 19, 2024 11:39 AM shveta malik wrote: > > On Sun, Feb 18, 2024 at 7:40 PM Zhijie Hou (Fujitsu) > wrote: > > > > On Friday, February 16, 2024 6:41 PM shveta malik > wrote: > > Thanks for the patch. Here are few comments: > > Thanks for the comments. > > > > > 2. > > > >

Re: Synchronizing slots from primary to standby

2024-02-18 Thread shveta malik
On Sun, Feb 18, 2024 at 7:40 PM Zhijie Hou (Fujitsu) wrote: > > On Friday, February 16, 2024 6:41 PM shveta malik > wrote: > Thanks for the patch. Here are few comments: Thanks for the comments. > > 2. > > +static bool > +validate_remote_info(WalReceiverConn *wrconn, int elevel) > ... > + > +

RE: Synchronizing slots from primary to standby

2024-02-18 Thread Zhijie Hou (Fujitsu)
On Friday, February 16, 2024 6:41 PM shveta malik wrote: > > On Thu, Feb 15, 2024 at 10:48 PM Bertrand Drouvot > wrote: > > > > Looking at v88_0001, random comments: > > Thanks for the feedback. > > > > > 1 === > > > > Commit message "Be enabling slot synchronization" > > > > Typo? s:Be/By >

Re: Synchronizing slots from primary to standby

2024-02-16 Thread Amit Kapila
On Fri, Feb 16, 2024 at 4:10 PM shveta malik wrote: > > On Thu, Feb 15, 2024 at 10:48 PM Bertrand Drouvot > wrote: > > > 5 === > > > > + if (SlotSyncWorker->syncing) > > { > > - SpinLockRelease(>mutex); > > + SpinLockRelease(>mutex); > >

RE: Synchronizing slots from primary to standby

2024-02-16 Thread Zhijie Hou (Fujitsu)
On Friday, February 16, 2024 3:43 PM Amit Kapila wrote: > > On Fri, Feb 16, 2024 at 11:43 AM Amit Kapila wrote: > > > > Thanks for noticing this. I have pushed all your debug patches. Let's > > hope if there is a BF failure next time, we can gather enough > > information to know the reason of

Re: Synchronizing slots from primary to standby

2024-02-16 Thread Bertrand Drouvot
Hi, On Fri, Feb 16, 2024 at 01:12:31PM +0530, Amit Kapila wrote: > On Fri, Feb 16, 2024 at 11:43 AM Amit Kapila wrote: > > > > Thanks for noticing this. I have pushed all your debug patches. Let's > > hope if there is a BF failure next time, we can gather enough > > information to know the

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Fri, Feb 16, 2024 at 11:43 AM Amit Kapila wrote: > > Thanks for noticing this. I have pushed all your debug patches. Let's > hope if there is a BF failure next time, we can gather enough > information to know the reason of the same. > There is a new BF failure [1] after adding these LOGs and

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Bertrand Drouvot
Hi, On Fri, Feb 16, 2024 at 12:32:45AM +, Zhijie Hou (Fujitsu) wrote: > Agreed. Here is new patch set as suggested. I used debug2 in the 040 as it > could provide more information about communication between primary and > standby. > This also doesn't increase noticeable testing time on my

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Fri, Feb 16, 2024 at 11:12 AM Zhijie Hou (Fujitsu) wrote: > > On Friday, February 16, 2024 8:33 AM Zhijie Hou (Fujitsu) > wrote: > > > > > > Yeah, we can consider outputting some information via this function > > > like how many slots are synced and persisted but not sure what would > > > be

RE: Synchronizing slots from primary to standby

2024-02-15 Thread Zhijie Hou (Fujitsu)
On Friday, February 16, 2024 8:33 AM Zhijie Hou (Fujitsu) wrote: > On Thursday, February 15, 2024 8:29 PM Amit Kapila > wrote: > > > > > On Thu, Feb 15, 2024 at 5:46 PM Bertrand Drouvot > > wrote: > > > > > > On Thu, Feb 15, 2024 at 05:00:18PM +0530, Amit Kapila wrote: > > > > On Thu, Feb 15,

RE: Synchronizing slots from primary to standby

2024-02-15 Thread Zhijie Hou (Fujitsu)
On Thursday, February 15, 2024 8:29 PM Amit Kapila wrote: > > On Thu, Feb 15, 2024 at 5:46 PM Bertrand Drouvot > wrote: > > > > On Thu, Feb 15, 2024 at 05:00:18PM +0530, Amit Kapila wrote: > > > On Thu, Feb 15, 2024 at 4:29 PM Zhijie Hou (Fujitsu) > > > wrote: > > > > Attach the v2 patch

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Bertrand Drouvot
Hi, On Thu, Feb 15, 2024 at 05:58:47PM +0530, Amit Kapila wrote: > On Thu, Feb 15, 2024 at 5:46 PM Bertrand Drouvot > wrote: > > Also I was thinking: what about adding an output to > > pg_sync_replication_slots()? > > The output could be the number of sync slots that have been created and are >

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Bertrand Drouvot
Hi, On Thu, Feb 15, 2024 at 06:13:38PM +0530, Amit Kapila wrote: > On Thu, Feb 15, 2024 at 12:07 PM Zhijie Hou (Fujitsu) > wrote: > > > > Since the slotsync function is committed, I rebased remaining patches. > > And here is the V88 patch set. > > Thanks! > > Please find the improvements in

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Thu, Feb 15, 2024 at 12:07 PM Zhijie Hou (Fujitsu) wrote: > > Since the slotsync function is committed, I rebased remaining patches. > And here is the V88 patch set. > Please find the improvements in some of the comments in v88_0001* attached. Kindly include these in next version, if you are

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Thu, Feb 15, 2024 at 5:46 PM Bertrand Drouvot wrote: > > On Thu, Feb 15, 2024 at 05:00:18PM +0530, Amit Kapila wrote: > > On Thu, Feb 15, 2024 at 4:29 PM Zhijie Hou (Fujitsu) > > wrote: > > > Attach the v2 patch here. > > > > > > Apart from the new log message. I think we can add one more

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Bertrand Drouvot
Hi, On Thu, Feb 15, 2024 at 05:00:18PM +0530, Amit Kapila wrote: > On Thu, Feb 15, 2024 at 4:29 PM Zhijie Hou (Fujitsu) > wrote: > > Attach the v2 patch here. > > > > Apart from the new log message. I think we can add one more debug message in > > reserve_wal_for_local_slot, this could be useful

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Thu, Feb 15, 2024 at 4:29 PM Zhijie Hou (Fujitsu) wrote: > > On Thursday, February 15, 2024 5:20 PM Amit Kapila > wrote: > > On Thu, Feb 15, 2024 at 9:05 AM Zhijie Hou (Fujitsu) > > > > wrote: > > > > > > On Thursday, February 15, 2024 10:49 AM Amit Kapila > > wrote: > > > > > > > > On

RE: Synchronizing slots from primary to standby

2024-02-15 Thread Zhijie Hou (Fujitsu)
On Thursday, February 15, 2024 5:20 PM Amit Kapila wrote: > On Thu, Feb 15, 2024 at 9:05 AM Zhijie Hou (Fujitsu) > wrote: > > > > On Thursday, February 15, 2024 10:49 AM Amit Kapila > wrote: > > > > > > On Wed, Feb 14, 2024 at 7:26 PM Bertrand Drouvot > > > > > > Right, we can do that or

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Bertrand Drouvot
Hi, On Thu, Feb 15, 2024 at 02:49:54PM +0530, Amit Kapila wrote: > On Thu, Feb 15, 2024 at 9:05 AM Zhijie Hou (Fujitsu) > wrote: > > > > On Thursday, February 15, 2024 10:49 AM Amit Kapila > > wrote: > > > > > > On Wed, Feb 14, 2024 at 7:26 PM Bertrand Drouvot > > > > > > Right, we can do that

Re: Synchronizing slots from primary to standby

2024-02-15 Thread Amit Kapila
On Thu, Feb 15, 2024 at 9:05 AM Zhijie Hou (Fujitsu) wrote: > > On Thursday, February 15, 2024 10:49 AM Amit Kapila > wrote: > > > > On Wed, Feb 14, 2024 at 7:26 PM Bertrand Drouvot > > > > Right, we can do that or probably this test would have made more sense with > > a > > worker patch where

RE: Synchronizing slots from primary to standby

2024-02-14 Thread Zhijie Hou (Fujitsu)
On Thursday, February 15, 2024 10:49 AM Amit Kapila wrote: > > On Wed, Feb 14, 2024 at 7:26 PM Bertrand Drouvot > wrote: > > > > On Wed, Feb 14, 2024 at 10:40:11AM +, Zhijie Hou (Fujitsu) wrote: > > > On Wednesday, February 14, 2024 6:05 PM Amit Kapila > wrote: > > > > > > > > To ensure

Re: Synchronizing slots from primary to standby

2024-02-14 Thread Amit Kapila
On Wed, Feb 14, 2024 at 7:26 PM Bertrand Drouvot wrote: > > On Wed, Feb 14, 2024 at 10:40:11AM +, Zhijie Hou (Fujitsu) wrote: > > On Wednesday, February 14, 2024 6:05 PM Amit Kapila > > wrote: > > > > > > To ensure that restart_lsn has been moved to a recent position, we need > > > to log

Re: Synchronizing slots from primary to standby

2024-02-14 Thread Bertrand Drouvot
Hi, On Wed, Feb 14, 2024 at 10:40:11AM +, Zhijie Hou (Fujitsu) wrote: > On Wednesday, February 14, 2024 6:05 PM Amit Kapila > wrote: > > > > On Wed, Feb 14, 2024 at 2:14 PM Amit Kapila wrote: > > > > > > On Wed, Feb 14, 2024 at 9:34 AM Zhijie Hou (Fujitsu) > > > wrote: > > > > > > > >

RE: Synchronizing slots from primary to standby

2024-02-14 Thread Zhijie Hou (Fujitsu)
On Wednesday, February 14, 2024 6:05 PM Amit Kapila wrote: > > On Wed, Feb 14, 2024 at 2:14 PM Amit Kapila wrote: > > > > On Wed, Feb 14, 2024 at 9:34 AM Zhijie Hou (Fujitsu) > > wrote: > > > > > > Here is V87 patch that adds test for the suggested cases. > > > > > > > I have pushed this

Re: Synchronizing slots from primary to standby

2024-02-14 Thread Amit Kapila
On Wed, Feb 14, 2024 at 2:14 PM Amit Kapila wrote: > > On Wed, Feb 14, 2024 at 9:34 AM Zhijie Hou (Fujitsu) > wrote: > > > > Here is V87 patch that adds test for the suggested cases. > > > > I have pushed this patch and it leads to a BF failure: >

Re: Synchronizing slots from primary to standby

2024-02-14 Thread Amit Kapila
On Wed, Feb 14, 2024 at 9:34 AM Zhijie Hou (Fujitsu) wrote: > > Here is V87 patch that adds test for the suggested cases. > I have pushed this patch and it leads to a BF failure: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=flaviventris=2024-02-14%2004%3A43%3A37 The test failures

RE: Synchronizing slots from primary to standby

2024-02-13 Thread Zhijie Hou (Fujitsu)
On Wednesday, February 14, 2024 10:40 AM Amit Kapila wrote: > > On Tue, Feb 13, 2024 at 9:25 PM Bertrand Drouvot > wrote: > > > > On Tue, Feb 13, 2024 at 05:20:35PM +0530, Amit Kapila wrote: > > > On Tue, Feb 13, 2024 at 4:59 PM Bertrand Drouvot > > > wrote: > > > > - 84% of the slotsync.c

Re: Synchronizing slots from primary to standby

2024-02-13 Thread Amit Kapila
On Tue, Feb 13, 2024 at 9:25 PM Bertrand Drouvot wrote: > > On Tue, Feb 13, 2024 at 05:20:35PM +0530, Amit Kapila wrote: > > On Tue, Feb 13, 2024 at 4:59 PM Bertrand Drouvot > > wrote: > > > - 84% of the slotsync.c code is covered, the parts that are not are mainly > > > related to "errors". > >

Re: Synchronizing slots from primary to standby

2024-02-13 Thread Bertrand Drouvot
Hi, On Tue, Feb 13, 2024 at 05:20:35PM +0530, Amit Kapila wrote: > On Tue, Feb 13, 2024 at 4:59 PM Bertrand Drouvot > wrote: > > - 84% of the slotsync.c code is covered, the parts that are not are mainly > > related to "errors". > > > > Worth to try to extend the coverage? (I've in mind 731,

RE: Synchronizing slots from primary to standby

2024-02-13 Thread Zhijie Hou (Fujitsu)
On Tuesday, February 13, 2024 7:30 PM Bertrand Drouvot wrote: > > On Tue, Feb 13, 2024 at 04:08:23AM +, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, February 13, 2024 9:16 AM Zhijie Hou (Fujitsu) > wrote: > > > > > > Here is the new version patch which addressed above and most of > > >

RE: Synchronizing slots from primary to standby

2024-02-13 Thread Zhijie Hou (Fujitsu)
On Tuesday, February 13, 2024 2:51 PM Amit Kapila wrote: > > On Tue, Feb 13, 2024 at 9:38 AM Zhijie Hou (Fujitsu) > wrote: > > > > Here is the V85_2 patch set that added the test and fixed one typo, > > there are no other code changes. > > > > Few comments on the latest changes: Thanks for

Re: Synchronizing slots from primary to standby

2024-02-13 Thread Amit Kapila
On Tue, Feb 13, 2024 at 4:59 PM Bertrand Drouvot wrote: > > On Tue, Feb 13, 2024 at 04:08:23AM +, Zhijie Hou (Fujitsu) wrote: > > On Tuesday, February 13, 2024 9:16 AM Zhijie Hou (Fujitsu) > > wrote: > > > > > > Here is the new version patch which addressed above and most of Bertrand's > >

Re: Synchronizing slots from primary to standby

2024-02-13 Thread Bertrand Drouvot
Hi, On Tue, Feb 13, 2024 at 04:08:23AM +, Zhijie Hou (Fujitsu) wrote: > On Tuesday, February 13, 2024 9:16 AM Zhijie Hou (Fujitsu) > wrote: > > > > Here is the new version patch which addressed above and most of Bertrand's > > comments. > > > > TODO: trying to add one test for the case

Re: Synchronizing slots from primary to standby

2024-02-13 Thread shveta malik
On Fri, Feb 9, 2024 at 10:04 AM Amit Kapila wrote: > > +reserve_wal_for_local_slot(XLogRecPtr restart_lsn) > { > ... > + /* > + * Find the oldest existing WAL segment file. > + * > + * Normally, we can determine it by using the last removed segment > + * number. However, if no WAL segment files

Re: Synchronizing slots from primary to standby

2024-02-12 Thread Amit Kapila
On Tue, Feb 13, 2024 at 9:38 AM Zhijie Hou (Fujitsu) wrote: > > Here is the V85_2 patch set that added the test and fixed one typo, > there are no other code changes. > Few comments on the latest changes: == 1. +# Confirm that the invalidated slot has been dropped.

RE: Synchronizing slots from primary to standby

2024-02-12 Thread Zhijie Hou (Fujitsu)
On Tuesday, February 13, 2024 9:16 AM Zhijie Hou (Fujitsu) wrote: > > Here is the new version patch which addressed above and most of Bertrand's > comments. > > TODO: trying to add one test for the case the slot is valid on primary while > the > synced slots is invalidated on the standby.

Re: Synchronizing slots from primary to standby

2024-02-12 Thread shveta malik
On Tue, Feb 13, 2024 at 6:45 AM Zhijie Hou (Fujitsu) wrote: > > On Monday, February 12, 2024 5:40 PM Amit Kapila > wrote: > > Thanks for the comments, I have addressed them. > > Here is the new version patch which addressed above and > most of Bertrand's comments. Thanks for the patch. I am

RE: Synchronizing slots from primary to standby

2024-02-12 Thread Zhijie Hou (Fujitsu)
On Monday, February 12, 2024 5:40 PM Amit Kapila wrote: > > On Sun, Feb 11, 2024 at 6:53 PM Zhijie Hou (Fujitsu) > wrote: > > > > Agreed. Here is the V84 patch which addressed this. > > > > Few comments: > = > 1. Isn't the new function (pg_sync_replication_slots()) allowed to sync

RE: Synchronizing slots from primary to standby

2024-02-12 Thread Zhijie Hou (Fujitsu)
On Monday, February 12, 2024 6:03 PM Bertrand Drouvot wrote: > > Hi, > > On Sun, Feb 11, 2024 at 01:23:19PM +, Zhijie Hou (Fujitsu) wrote: > > On Saturday, February 10, 2024 9:10 PM Amit Kapila > wrote: > > > > > > On Sat, Feb 10, 2024 at 5:31 PM Masahiko Sawada > > > > > > wrote: > > >

Re: Synchronizing slots from primary to standby

2024-02-12 Thread Bertrand Drouvot
Hi, On Mon, Feb 12, 2024 at 04:19:33PM +0530, Amit Kapila wrote: > On Mon, Feb 12, 2024 at 3:33 PM Bertrand Drouvot > wrote: > > > > A few random comments: > > > > > > 003 === > > > > + If, after executing the function, > > + > > + hot_standby_feedback is disabled on

Re: Synchronizing slots from primary to standby

2024-02-12 Thread Amit Kapila
On Mon, Feb 12, 2024 at 3:33 PM Bertrand Drouvot wrote: > > A few random comments: > > > 003 === > > + If, after executing the function, > + > + hot_standby_feedback is disabled on > + the standby or the physical slot configured in > + > +

Re: Synchronizing slots from primary to standby

2024-02-12 Thread Bertrand Drouvot
Hi, On Sun, Feb 11, 2024 at 01:23:19PM +, Zhijie Hou (Fujitsu) wrote: > On Saturday, February 10, 2024 9:10 PM Amit Kapila > wrote: > > > > On Sat, Feb 10, 2024 at 5:31 PM Masahiko Sawada > > wrote: > > > > > > On Fri, Feb 9, 2024 at 4:08 PM Zhijie Hou (Fujitsu) > > > wrote: > > > > > >

Re: Synchronizing slots from primary to standby

2024-02-12 Thread Amit Kapila
On Sun, Feb 11, 2024 at 6:53 PM Zhijie Hou (Fujitsu) wrote: > > Agreed. Here is the V84 patch which addressed this. > Few comments: = 1. Isn't the new function (pg_sync_replication_slots()) allowed to sync the slots from physical standby to another cascading standby? Won't it be

RE: Synchronizing slots from primary to standby

2024-02-11 Thread Zhijie Hou (Fujitsu)
On Saturday, February 10, 2024 9:10 PM Amit Kapila wrote: > > On Sat, Feb 10, 2024 at 5:31 PM Masahiko Sawada > wrote: > > > > On Fri, Feb 9, 2024 at 4:08 PM Zhijie Hou (Fujitsu) > > wrote: > > > > > Another alternative is to register the callback when calling > > > slotsync functions and

Re: Synchronizing slots from primary to standby

2024-02-10 Thread Amit Kapila
On Sat, Feb 10, 2024 at 5:31 PM Masahiko Sawada wrote: > > On Fri, Feb 9, 2024 at 4:08 PM Zhijie Hou (Fujitsu) > wrote: > > > Another alternative is to register the callback when calling slotsync > > functions > > and unregister it after the function call. And register the callback in > >

Re: Synchronizing slots from primary to standby

2024-02-10 Thread Masahiko Sawada
On Fri, Feb 9, 2024 at 4:08 PM Zhijie Hou (Fujitsu) wrote: > > On Friday, February 9, 2024 2:44 PM Masahiko Sawada > wrote: > > > > On Thu, Feb 8, 2024 at 8:01 PM shveta malik wrote: > > > > > > On Thu, Feb 8, 2024 at 12:08 PM Peter Smith > > wrote: > > > > > > > > Here are some review

RE: Synchronizing slots from primary to standby

2024-02-10 Thread Zhijie Hou (Fujitsu)
On Saturday, February 10, 2024 11:37 AM Zhijie Hou (Fujitsu) wrote: > > Attach the V83 patch which addressed Peter[1][2], Amit and Sawada-san's[3] > comments. Only 0001 is sent in this version, we will send other patches after > rebasing. > > [1] >

RE: Synchronizing slots from primary to standby

2024-02-09 Thread Zhijie Hou (Fujitsu)
On Friday, February 9, 2024 4:13 PM Peter Smith wrote: > > FYI -- I checked patch v81-0001 to find which of the #includes are strictly > needed. Thanks! > > 1. > ... > > Many of these #includes seem unnecessary. e.g. I was able to remove > all those that are commented-out below, and the

RE: Synchronizing slots from primary to standby

2024-02-09 Thread Zhijie Hou (Fujitsu)
On Friday, February 9, 2024 12:27 PM Peter Smith wrote: > > Here are some review comments for patch v81-0001. Thanks for the comments. > . GENERAL - ReplicationSlotInvalidationCause enum. > > I was thinking that the ReplicationSlotInvalidationCause should > explicitly set RS_INVAL_NONE = 0

RE: Synchronizing slots from primary to standby

2024-02-09 Thread Zhijie Hou (Fujitsu)
On Friday, February 9, 2024 6:45 PM Amit Kapila wrote: > > On Fri, Feb 9, 2024 at 10:00 AM Zhijie Hou (Fujitsu) > wrote: > > > > Few comments on 0001 > === > 1. Shouldn't pg_sync_replication_slots() check whether the user has > replication > privilege? Yes, added. > > 2.

Re: Synchronizing slots from primary to standby

2024-02-09 Thread Amit Kapila
On Fri, Feb 9, 2024 at 10:00 AM Zhijie Hou (Fujitsu) wrote: > Few comments on 0001 === 1. Shouldn't pg_sync_replication_slots() check whether the user has replication privilege? 2. The function declarations in slotsync.h don't seem to be in the same order as they are defined in

Re: Synchronizing slots from primary to standby

2024-02-09 Thread Peter Smith
FYI -- I checked patch v81-0001 to find which of the #includes are strictly needed. == src/backend/replication/logical/slotsync.c 1. +#include "postgres.h" + +#include + +#include "access/genam.h" +#include "access/table.h" +#include "access/xlog_internal.h" +#include

RE: Synchronizing slots from primary to standby

2024-02-08 Thread Zhijie Hou (Fujitsu)
On Friday, February 9, 2024 2:44 PM Masahiko Sawada wrote: > > On Thu, Feb 8, 2024 at 8:01 PM shveta malik wrote: > > > > On Thu, Feb 8, 2024 at 12:08 PM Peter Smith > wrote: > > > > > > Here are some review comments for patch v80_2-0001. > > > > Thanks for the feedback Peter. Addressed the

Re: Synchronizing slots from primary to standby

2024-02-08 Thread Masahiko Sawada
On Thu, Feb 8, 2024 at 8:01 PM shveta malik wrote: > > On Thu, Feb 8, 2024 at 12:08 PM Peter Smith wrote: > > > > Here are some review comments for patch v80_2-0001. > > Thanks for the feedback Peter. Addressed the comments in v81. > Attached patch001 for early feedback. Rest of the patches need

Re: Synchronizing slots from primary to standby

2024-02-08 Thread Amit Kapila
On Fri, Feb 9, 2024 at 9:57 AM Peter Smith wrote: > > Here are some review comments for patch v81-0001. > > == > > 1. GENERAL - ReplicationSlotInvalidationCause enum. > > I was thinking that the ReplicationSlotInvalidationCause should > explicitly set RS_INVAL_NONE = 0 (it's zero anyway, but

Re: Synchronizing slots from primary to standby

2024-02-08 Thread Amit Kapila
On Fri, Feb 9, 2024 at 10:00 AM Zhijie Hou (Fujitsu) wrote: > > Here is the V82 patch set which includes the following changes: > +reserve_wal_for_local_slot(XLogRecPtr restart_lsn) { ... + /* + * Find the oldest existing WAL segment file. + * + * Normally, we can determine it by using the last

Re: Synchronizing slots from primary to standby

2024-02-08 Thread Peter Smith
Here are some review comments for patch v81-0001. == 1. GENERAL - ReplicationSlotInvalidationCause enum. I was thinking that the ReplicationSlotInvalidationCause should explicitly set RS_INVAL_NONE = 0 (it's zero anyway, but making it explicit with a comment /* Must be zero. */. will stop

Re: Synchronizing slots from primary to standby

2024-02-08 Thread shveta malik
On Thu, Feb 8, 2024 at 4:31 PM shveta malik wrote: > > On Thu, Feb 8, 2024 at 12:08 PM Peter Smith wrote: > > > > Here are some review comments for patch v80_2-0001. > > Thanks for the feedback Peter. Addressed the comments in v81. Missed to mention, Hou-san helped in addressing some of these

Re: Synchronizing slots from primary to standby

2024-02-08 Thread shveta malik
On Thu, Feb 8, 2024 at 4:03 PM Amit Kapila wrote: > > Few comments on 0001 > === Thanks Amit. Addressed these in v81. > 1. > + * the slots on the standby and synchronize them. This is done on every call > + * to SQL function pg_sync_replication_slots. > > > > I think the second

Re: Synchronizing slots from primary to standby

2024-02-08 Thread shveta malik
On Thu, Feb 8, 2024 at 12:08 PM Peter Smith wrote: > > Here are some review comments for patch v80_2-0001. Thanks for the feedback Peter. Addressed the comments in v81. Attached patch001 for early feedback. Rest of the patches need rebasing and thus will post those later. It also addresses

Re: Synchronizing slots from primary to standby

2024-02-08 Thread Amit Kapila
On Wed, Feb 7, 2024 at 5:32 PM shveta malik wrote: > > Sure, made the suggested function name changes. Since there is no > other change, I kept the version as v80_2. > Few comments on 0001 === 1. + * the slots on the standby and synchronize them. This is done on every call + * to

Re: Synchronizing slots from primary to standby

2024-02-07 Thread Peter Smith
Here are some review comments for patch v80_2-0001. == Commit message 1. We may also see the the slots invalidated and dropped on the standby if the primary changes 'wal_level' to a level lower than logical. Changing the primary 'wal_level' to a level lower than logical is only possible if

RE: Synchronizing slots from primary to standby

2024-02-07 Thread Zhijie Hou (Fujitsu)
On Wednesday, February 7, 2024 9:13 AM Masahiko Sawada wrote: > > On Tue, Feb 6, 2024 at 8:21 PM Amit Kapila wrote: > > > > On Tue, Feb 6, 2024 at 3:33 PM Amit Kapila > wrote: > > > > > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada > wrote: > > > > > > > > On Tue, Feb 6, 2024 at 3:19 PM

Re: Synchronizing slots from primary to standby

2024-02-07 Thread Nisha Moond
We conducted stress testing for the patch with a setup of one primary node with 100 tables and five subscribers, each having 20 subscriptions. Then created three physical standbys syncing the logical replication slots from the primary node. All 100 slots were successfully synced on all three

Re: Synchronizing slots from primary to standby

2024-02-07 Thread Dilip Kumar
> > So now if we have such a functionality then it would be even better to > > extend it to selectively sync the slot. For example, if there is some > > issue in syncing all slots, maybe some bug or taking a long time to > > sync because there are a lot of slots but if the user needs to quickly >

Re: Synchronizing slots from primary to standby

2024-02-07 Thread Amit Kapila
On Tue, Feb 6, 2024 at 12:08 PM Peter Smith wrote: > > There seems some muddling of names here: > - "local" versus ? and "remote" versus "primary"; or sometimes the > function does not give an indication. > - "sync_slot" versus "synced_slot" versus nothing > - "check" versus "validate" > - etc. >

Re: Synchronizing slots from primary to standby

2024-02-07 Thread shveta malik
On Tue, Feb 6, 2024 at 12:25 PM Bertrand Drouvot wrote: > > Hi, > That said, I still think the commit message needs some re-wording, what about? > > = > If a logical slot on the primary is valid but is invalidated on the standby, > then that slot is dropped and can be recreated on the

Re: Synchronizing slots from primary to standby

2024-02-07 Thread Amit Kapila
On Mon, Feb 5, 2024 at 9:19 AM Zhijie Hou (Fujitsu) wrote: > > On Thursday, February 1, 2024 12:20 PM Amit Kapila > wrote: > > > > On Thu, Feb 1, 2024 at 8:15 AM Euler Taveira wrote: > > > > > > > > > While working on another patch I noticed a new NOTICE message: > > > > > > NOTICE: changed

Re: Synchronizing slots from primary to standby

2024-02-06 Thread shveta malik
On Tue, Feb 6, 2024 at 7:21 PM Zhijie Hou (Fujitsu) wrote: > > > --- > > +/* Slot sync worker objects */ > > +extern PGDLLIMPORT char *PrimaryConnInfo; extern PGDLLIMPORT char > > +*PrimarySlotName; > > > > These two variables are declared also in xlogrecovery.h. Is it intentional? > > If so, I

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Peter Smith
Here are some review comments for v79-0001 == Commit message 1. The logical replication slots on the primary can be synchronized to the hot standby by enabling the "failover" parameter during pg_create_logical_replication_slot() or by enabling "failover" option of the CREATE SUBSCRIPTION

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Masahiko Sawada
On Tue, Feb 6, 2024 at 8:21 PM Amit Kapila wrote: > > On Tue, Feb 6, 2024 at 3:33 PM Amit Kapila wrote: > > > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada > > wrote: > > > > > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila > > > wrote: > > > > > > > > I think users can refer to LOGs to see

RE: Synchronizing slots from primary to standby

2024-02-06 Thread Zhijie Hou (Fujitsu)
On Monday, February 5, 2024 10:25 PM Masahiko Sawada wrote: lvh.no-ip.org> > Subject: Re: Synchronizing slots from primary to standby > > On Mon, Feb 5, 2024 at 8:26 PM shveta malik > wrote: > > > > On Mon, Feb 5, 2024 at 10:57 AM Ajin Cherian wrote: > > >

RE: Synchronizing slots from primary to standby

2024-02-06 Thread Zhijie Hou (Fujitsu)
On Tuesday, February 6, 2024 3:39 PM Masahiko Sawada wrote: > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila wrote: > > > > On Mon, Feb 5, 2024 at 7:56 PM Masahiko Sawada > wrote: > > > > > > --- > > > Since Two processes (e.g. the slotsync worker and > > > pg_sync_replication_slots())

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 12:08 PM Peter Smith wrote: > > Hi, I took another high-level look at all the funtion names of the > slotsync.c file. > > > Below are some suggestions (some are unchanged); probably there are > better ideas for names but my point is that the current names could be >

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 3:57 PM Dilip Kumar wrote: > > On Tue, Feb 6, 2024 at 3:41 PM Amit Kapila wrote: > > > > On Tue, Feb 6, 2024 at 3:23 PM Dilip Kumar wrote: > > > > > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada > > > wrote: > > > > > > > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 3:33 PM Amit Kapila wrote: > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada wrote: > > > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila wrote: > > > > > > I think users can refer to LOGs to see if it has changed since the > > > first time it was configured. I tried by

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 9:35 AM Peter Smith wrote: > > == > GENERAL > > 1. > Should the "Chapter 30 Logical Replication" at least have another > section that mentions the feature of slot synchronization so the > information about it is easier to find? It doesn't need to say much -- > just give

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Dilip Kumar
On Tue, Feb 6, 2024 at 3:41 PM Amit Kapila wrote: > > On Tue, Feb 6, 2024 at 3:23 PM Dilip Kumar wrote: > > > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada > > wrote: > > > > > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila > > > wrote: > > > > > > > > On Mon, Feb 5, 2024 at 7:56 PM

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 3:23 PM Dilip Kumar wrote: > > On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada wrote: > > > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila wrote: > > > > > > On Mon, Feb 5, 2024 at 7:56 PM Masahiko Sawada > > > wrote: > > > > > > > > --- > > > > Since Two processes (e.g.

Re: Synchronizing slots from primary to standby

2024-02-06 Thread Amit Kapila
On Tue, Feb 6, 2024 at 1:09 PM Masahiko Sawada wrote: > > On Tue, Feb 6, 2024 at 3:19 PM Amit Kapila wrote: > > > > I think users can refer to LOGs to see if it has changed since the > > first time it was configured. I tried by existing parameter and see > > the following in LOG: > > LOG:

<    1   2   3   4   5   6   7   8   9   >