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
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
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!
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
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
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 ===
>
> +
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
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
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 =
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
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
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
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
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
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
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
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
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
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
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)
> > > {
> > > -
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.
> >
> >
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)
> ...
> +
> +
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
>
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);
> >
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
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
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
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
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
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,
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
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
>
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
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
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
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
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
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
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
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
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
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
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:
> > > >
> > > >
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
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:
>
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
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
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".
> >
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,
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
> > >
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
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
> >
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
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
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.
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.
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
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
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:
> > >
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
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
> +
> +
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:
> > >
> > >
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
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
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
> >
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
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]
>
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
> > 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
>
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.
>
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
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
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
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
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
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:
> > >
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())
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
>
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
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
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
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
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.
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:
201 - 300 of 839 matches
Mail list logo