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:
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. the slotsync worker and
> > > pg_sync_replication_slots()) concurrently
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()) concurrently fetch and update the slot
> > information, there is a race condition where
Hi,
On Tue, Feb 06, 2024 at 03:19:11AM +, Zhijie Hou (Fujitsu) wrote:
> On Friday, February 2, 2024 2:03 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > On Thu, Feb 01, 2024 at 05:29:15PM +0530, shveta malik wrote:
> > > Attached v75 patch-set. Changes are:
> > >
> > > 1) Re-arranged the
Hi, I took another high-level look at all the funtion names of the
slotsync.c file.
==
src/backend/replication/logical/slotsync.c
+static bool
+local_slot_update(RemoteSlot * remote_slot, Oid remote_dbid)
+static List *
+get_local_synced_slots(void)
+static bool
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()) concurrently fetch and update the slot
> information, there is a race condition where slot's
> confirmed_flush_lsn goes backward.
>
Right, this is
Hi,
Previously ([1] #19 and #22) I had suggested that some conflict_reason
code could be split off and pushed first as a prerequisite/
preparatory/ independent patch.
At the time, my suggestion was premature because there was still a lot
of code under development.
But now the affected code is
Here are some review comments for v78-0001
==
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 a reference to the
On Friday, February 2, 2024 2:03 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Thu, Feb 01, 2024 at 05:29:15PM +0530, shveta malik wrote:
> > Attached v75 patch-set. Changes are:
> >
> > 1) Re-arranged the patches:
> > 1.1) 'libpqrc' related changes (from v74-001 and v74-004) are
> > separated out
On Mon, Feb 5, 2024 at 8:26 PM shveta malik wrote:
>
> On Mon, Feb 5, 2024 at 10:57 AM Ajin Cherian wrote:
> >
> > Just noticed that doc/src/sgml/config.sgml still refers to enable_synclot
> > instead of sync_replication_slots:
> >
> > The standbys corresponding to the physical replication
On Mon, Feb 5, 2024 at 4:36 PM Amit Kapila wrote:
>
> I have pushed the first patch. Next, a few comments on 0002 are as follows:
Thanks for the feedback Amit. Some of these are addressed in v78. Rest
will be addressed in the next version.
> 1.
> +static bool
>
On Mon, Feb 5, 2024 at 7:59 AM Zhijie Hou (Fujitsu)
wrote:
>
I have pushed the first patch. Next, a few comments on 0002 are as follows:
1.
+static bool
+validate_parameters_and_get_dbname(char **dbname, int elevel)
For 0002, we don't need dbname as out parameter. Also, we can rename
the
On Mon, Feb 5, 2024 at 1:29 PM Zhijie Hou (Fujitsu)
wrote:
> On Monday, February 5, 2024 10:17 AM Zhijie Hou (Fujitsu) <
> houzj.f...@fujitsu.com> wrote:
>
> There was one miss in the doc that cause CFbot failure,
> attach the correct version V77_2 here. There are no code changes compared
> to
On Thursday, February 1, 2024 12:20 PM Amit Kapila
wrote:
>
> On Thu, Feb 1, 2024 at 8:15 AM Euler Taveira wrote:
> >
> > On Mon, Jan 29, 2024, at 10:17 AM, Zhijie Hou (Fujitsu) wrote:
> >
> > Attach the V72-0001 which addressed above comments, other patches will
> be
> > rebased and posted
On Fri, Feb 2, 2024 at 11:18 PM shveta malik wrote:
>
> On Fri, Feb 2, 2024 at 12:25 PM Peter Smith wrote:
> >
> > Here are some review comments for v750002.
>
> Thanks for the feedback Peter. Addressed all in v76 except one.
>
> > (this is a WIP but this is what I found so far...)
>
> > I
On Fri, Feb 2, 2024 at 12:25 PM Peter Smith wrote:
>
> Here are some review comments for v750002.
Thanks for the feedback Peter. Addressed all in v76 except one.
> (this is a WIP but this is what I found so far...)
> I wonder if it is better to log all the problems in one go instead of
>
Hi,
On Fri, Feb 02, 2024 at 12:25:30PM +0530, Amit Kapila wrote:
> On Thu, Feb 1, 2024 at 5:29 PM shveta malik wrote:
> >
> > On Thu, Feb 1, 2024 at 2:35 PM Amit Kapila wrote:
> > >
> > > Agreed, and I am fine with merging 0001, 0002, and 0004 as suggested
> > > by you though I have a few minor
On Fri, Feb 2, 2024 at 10:53 AM Bertrand Drouvot
wrote:
>
> On Thu, Feb 01, 2024 at 04:12:43PM +0530, Amit Kapila wrote:
> > On Thu, Jan 25, 2024 at 11:26 AM Bertrand Drouvot
> >
> > I again thought on this point and feel that even if we start to sync
> > say physical slots their purpose would
On Thu, Feb 1, 2024 at 5:29 PM shveta malik wrote:
>
> On Thu, Feb 1, 2024 at 2:35 PM Amit Kapila wrote:
> >
> > Agreed, and I am fine with merging 0001, 0002, and 0004 as suggested
> > by you though I have a few minor comments on 0002 and 0004. I was
> > thinking about what will be a logical
Here are some review comments for v750002.
(this is a WIP but this is what I found so far...)
==
doc/src/sgml/protocol.sgml
1.
> > 2. ALTER_REPLICATION_SLOT slot_name ( option [, ...] ) #
> >
> >
> > - If true, the slot is enabled to be synced to the standbys.
> > +
On Fri, Feb 2, 2024 at 1:58 PM Amit Kapila wrote:
>
> On Fri, Feb 2, 2024 at 6:46 AM Masahiko Sawada wrote:
> >
> > On Thu, Feb 1, 2024 at 12:51 PM Amit Kapila wrote:
> > >
> > >
> > > > BTW I've tested the following switch/fail-back scenario but it seems
> > > > not to work fine. Am I missing
On Fri, Feb 2, 2024 at 9:50 AM Peter Smith wrote:
>
> Here are some review comments for v750001.
>
> ~~~
>
> 2.
> This patch also implements a new API libpqrcv_get_dbname_from_conninfo()
> to extract database name from the given connection-info
>
> ~
>
> /extract database name/the extract
Hi,
On Thu, Feb 01, 2024 at 05:29:15PM +0530, shveta malik wrote:
> Attached v75 patch-set. Changes are:
>
> 1) Re-arranged the patches:
> 1.1) 'libpqrc' related changes (from v74-001 and v74-004) are
> separated out in v75-001 as those are independent changes.
> 1.2) 'Add logical slot sync
Hi,
On Thu, Feb 01, 2024 at 04:12:43PM +0530, Amit Kapila wrote:
> On Thu, Jan 25, 2024 at 11:26 AM Bertrand Drouvot
> wrote:
> >
> > On Wed, Jan 24, 2024 at 04:09:15PM +0530, shveta malik wrote:
> > > On Wed, Jan 24, 2024 at 2:38 PM Bertrand Drouvot
> > > wrote:
> > > >
> > > > I also see
On Fri, Feb 2, 2024 at 6:46 AM Masahiko Sawada wrote:
>
> On Thu, Feb 1, 2024 at 12:51 PM Amit Kapila wrote:
> >
> >
> > > BTW I've tested the following switch/fail-back scenario but it seems
> > > not to work fine. Am I missing something?
> > >
> > > Setup:
> > > node1 is the primary, node2 is
Here are some review comments for v750001.
==
Commit message
1.
This patch provides support for non-replication connection
in libpqrcv_connect().
~
1a.
/connection/connections/
~
1b.
Maybe there needs to be a few more sentences just to describe what you
mean by "non-replication
On Thu, Feb 1, 2024 at 12:51 PM Amit Kapila wrote:
>
> On Wed, Jan 31, 2024 at 9:20 PM Masahiko Sawada wrote:
> >
> > On Wed, Jan 31, 2024 at 7:42 PM Amit Kapila wrote:
> > >
> > >
> > > Considering my previous where we don't want to restart for a required
> > > parameter change, isn't it
On Wed, Jan 31, 2024 at 2:02 PM Masahiko Sawada wrote:
>
> ---
> +static char *
> +wait_for_valid_params_and_get_dbname(void)
> +{
> + char *dbname;
> + int rc;
> +
> + /* Sanity check. */
> + Assert(enable_syncslot);
> +
> + for (;;)
> + {
> + if
On Thu, Feb 1, 2024 at 11:21 AM Peter Smith wrote:
>
> Here are some review comments for v740001.
Thanks Peter for the feedback.
> ==
> src/sgml/logicaldecoding.sgml
>
> 1.
> +
> +Replication Slot Synchronization
> +
> + A logical replication slot on the primary can be
On Wed, Jan 31, 2024 at 10:40 AM Peter Smith wrote:
>
> On Wed, Jan 31, 2024 at 2:18 PM Amit Kapila wrote:
> >
> > >
> > > I think is correct to say all those *other* properties (create_slot,
> > > enabled, copy_data) are forced to false because those otherwise have
> > > default true values.
>
On Thu, Jan 25, 2024 at 11:26 AM Bertrand Drouvot
wrote:
>
> On Wed, Jan 24, 2024 at 04:09:15PM +0530, shveta malik wrote:
> > On Wed, Jan 24, 2024 at 2:38 PM Bertrand Drouvot
> > wrote:
> > >
> > > I also see Sawada-San's point and I'd vote for "sync_replication_slots".
> > > Then for
> > >
On Wed, Jan 31, 2024 at 9:20 PM Masahiko Sawada wrote:
>
> On Wed, Jan 31, 2024 at 7:42 PM Amit Kapila wrote:
> >
> > On Wed, Jan 31, 2024 at 2:02 PM Masahiko Sawada
> > wrote:
> > >
> > > Thank you for updating the patches. As for the slotsync worker patch,
> > > is there any reason why 0001,
Here are some review comments for v740001.
==
src/sgml/logicaldecoding.sgml
1.
+
+Replication Slot Synchronization
+
+ A logical replication slot on the primary can be synchronized to the hot
+ standby by enabling the failover option during slot
+ creation and setting
On Tue, Jan 30, 2024 at 11:53 PM shveta malik
wrote:
> On Tue, Jan 30, 2024 at 4:06 PM shveta malik
> wrote:
> >
> > PFA v73-0001 which addresses the above comments. Other patches will be
> > rebased and posted after pushing this one.
>
> Since v73-0001 is pushed, PFA rest of the patches.
On Thu, Feb 1, 2024 at 8:15 AM Euler Taveira wrote:
>
> On Mon, Jan 29, 2024, at 10:17 AM, Zhijie Hou (Fujitsu) wrote:
>
> Attach the V72-0001 which addressed above comments, other patches will be
> rebased and posted after pushing first patch. Thanks Shveta for helping
> address
> the comments.
On Wed, Jan 31, 2024 at 9:20 PM Masahiko Sawada wrote:
>
> On Wed, Jan 31, 2024 at 7:42 PM Amit Kapila wrote:
> >
> >
> > Considering my previous where we don't want to restart for a required
> > parameter change, isn't it better to avoid repeated restart (say when
> > the user gave an invalid
On Mon, Jan 29, 2024, at 10:17 AM, Zhijie Hou (Fujitsu) wrote:
> Attach the V72-0001 which addressed above comments, other patches will be
> rebased and posted after pushing first patch. Thanks Shveta for helping
> address
> the comments.
While working on another patch I noticed a new NOTICE
On Wed, Jan 31, 2024 at 7:42 PM Amit Kapila wrote:
>
> On Wed, Jan 31, 2024 at 2:02 PM Masahiko Sawada wrote:
> >
> > Thank you for updating the patches. As for the slotsync worker patch,
> > is there any reason why 0001, 0002, and 0004 patches are still
> > separated?
> >
>
> No specific
On Wed, Jan 31, 2024 at 2:02 PM Masahiko Sawada wrote:
>
> Thank you for updating the patches. As for the slotsync worker patch,
> is there any reason why 0001, 0002, and 0004 patches are still
> separated?
>
No specific reason, it could be easier to review those parts.
>
> Beside, here are
On Tue, Jan 30, 2024 at 9:53 PM shveta malik wrote:
>
> On Tue, Jan 30, 2024 at 4:06 PM shveta malik wrote:
> >
> > PFA v73-0001 which addresses the above comments. Other patches will be
> > rebased and posted after pushing this one.
>
> Since v73-0001 is pushed, PFA rest of the patches.
On Wednesday, January 31, 2024 9:57 AM Peter Smith
wrote:
>
> Hi,
>
> I saw that v73-0001 was pushed, but it included some last-minute
> changes that I did not get a chance to check yesterday.
>
> Here are some review comments for the new parts of that patch.
>
> ==
>
On Wed, Jan 31, 2024 at 2:18 PM Amit Kapila wrote:
>
> On Wed, Jan 31, 2024 at 7:27 AM Peter Smith wrote:
> >
> > I saw that v73-0001 was pushed, but it included some last-minute
> > changes that I did not get a chance to check yesterday.
> >
> > Here are some review comments for the new parts
On Wed, Jan 31, 2024 at 7:27 AM Peter Smith wrote:
>
> I saw that v73-0001 was pushed, but it included some last-minute
> changes that I did not get a chance to check yesterday.
>
> Here are some review comments for the new parts of that patch.
>
> ==
>
Hi,
I saw that v73-0001 was pushed, but it included some last-minute
changes that I did not get a chance to check yesterday.
Here are some review comments for the new parts of that patch.
==
doc/src/sgml/ref/create_subscription.sgml
1.
connect (boolean)
Specifies whether the CREATE
Hi,
On Mon, Jan 29, 2024 at 09:15:57AM +, Bertrand Drouvot wrote:
> Hi,
>
> On Mon, Jan 29, 2024 at 02:35:52PM +0530, Amit Kapila wrote:
> > I think it is better to create a separate patch for two_phase after
> > this patch gets committed.
>
> Yeah, makes sense, will do, thanks!
It's done
On Tue, Jan 30, 2024 at 11:31 AM Amit Kapila wrote:
>
> In this regard, I feel we don't need to dump/restore the 'FAILOVER'
> option non-binary upgrade paths similar to the 'ENABLE' option. For
> binary upgrade, if the failover option is enabled, then we can enable
> it using Alter Subscription
On Mon, Jan 29, 2024 at 6:47 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, January 29, 2024 7:30 PM Amit Kapila
> wrote:
> >
>
> > ===
> > 1.
> > parse_subscription_options()
> > {
> > ...
> > /*
> > * We've been explicitly asked to not connect, that requires some
> > *
On Tue, Jan 30, 2024 at 7:29 AM Peter Smith wrote:
>
> Here are some review comments for v72-0001
>
> ==
> doc/src/sgml/ref/alter_subscription.sgml
>
> 1.
> + parameter value of the subscription. Otherwise, the slot on the
> + publisher may behave differently from what
Here are some review comments for v72-0001
==
doc/src/sgml/ref/alter_subscription.sgml
1.
+ parameter value of the subscription. Otherwise, the slot on the
+ publisher may behave differently from what subscription's
+ failover
+ option says. The slot on the publisher
On Monday, January 29, 2024 9:17 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Monday, January 29, 2024 7:30 PM Amit Kapila
> wrote:
> >
> > On Mon, Jan 29, 2024 at 3:11 PM shveta malik
> > wrote:
> > >
> > > PFA v71 patch set with above changes.
> > >
> >
> > Few comments on 0001
>
> Thanks for the
On Monday, January 29, 2024 7:30 PM Amit Kapila wrote:
>
> On Mon, Jan 29, 2024 at 3:11 PM shveta malik
> wrote:
> >
> > PFA v71 patch set with above changes.
> >
>
> Few comments on 0001
Thanks for the comments.
> ===
> 1.
> parse_subscription_options()
> {
> ...
> /*
> *
On Mon, Jan 29, 2024 at 3:11 PM shveta malik wrote:
>
> PFA v71 patch set with above changes.
>
Few comments on 0001
===
1.
parse_subscription_options()
{
...
/*
* We've been explicitly asked to not connect, that requires some
* additional processing.
*/
if (!opts->connect &&
Hi,
On Mon, Jan 29, 2024 at 02:35:52PM +0530, Amit Kapila wrote:
> On Mon, Jan 29, 2024 at 2:22 PM Bertrand Drouvot
> wrote:
> > Looking at 0001:
> >
> > + When altering the
> > + > linkend="sql-createsubscription-params-with-slot-name">slot_name,
> > + the failover property
On Mon, Jan 29, 2024 at 2:22 PM Bertrand Drouvot
wrote:
>
> On Mon, Jan 29, 2024 at 10:24:11AM +0530, shveta malik wrote:
> > On Sat, Jan 27, 2024 at 12:02 PM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > Attach the V70 patch set which addressed above comments and Bertrand's
> > > comments in [1]
Hi,
On Mon, Jan 29, 2024 at 10:24:11AM +0530, shveta malik wrote:
> On Sat, Jan 27, 2024 at 12:02 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Attach the V70 patch set which addressed above comments and Bertrand's
> > comments in [1]
> >
>
> Since v70-0001 is pushed, rebased and attached v70_2
On Mon, Jan 29, 2024 at 9:21 AM Peter Smith wrote:
>
> Here are some review comments for v70-0001.
>
> ==
> doc/src/sgml/protocol.sgml
>
> 1.
> Related to this, please also review my other patch to the same docs
> page protocol.sgml [1].
>
We can check that separately.
> ==
>
Here are some review comments for v70-0001.
==
doc/src/sgml/protocol.sgml
1.
Related to this, please also review my other patch to the same docs
page protocol.sgml [1].
==
src/backend/replication/logical/tablesync.c
2.
walrcv_create_slot(LogRepWorkerWalRcvConn,
slotname, false
On Thu, Jan 25, 2024 at 6:42 PM Zhijie Hou (Fujitsu)
wrote:
>
> Here is the V69 patch set which includes the following changes.
>
> V69-0001, V69-0002
>
Few minor comments on v69-0001
1. In libpqrcv_create_slot(), I see we are using two types of syntaxes
based on 'use_new_options_syntax' (aka
Hi,
On Thu, Jan 25, 2024 at 01:11:50PM +, Zhijie Hou (Fujitsu) wrote:
> Here is the V69 patch set which includes the following changes.
>
> V69-0001, V69-0002
> 1) Addressed Bertrand's comments[1].
Thanks!
V69-0001 LGTM.
As far V69-0002 I just have one more last remark:
+
Hi,
On Thu, Jan 25, 2024 at 03:54:45PM +0530, Amit Kapila wrote:
> On Thu, Jan 25, 2024 at 1:25 PM Bertrand Drouvot
> wrote:
> >
> > On Thu, Jan 25, 2024 at 02:57:30AM +, Zhijie Hou (Fujitsu) wrote:
> > >
> > > Agreed. I split the original 0001 patch into 3 patches as suggested.
> > > Here
On Thu, Jan 25, 2024 at 10:39 AM Peter Smith wrote:
> 2. synchronize_one_slot
>
> + /*
> + * Sanity check: Make sure that concerned WAL is received and flushed
> + * before syncing slot to target lsn received from the primary server.
> + *
> + * This check should never pass as on the primary
On Thu, Jan 25, 2024 at 1:25 PM Bertrand Drouvot
wrote:
>
> On Thu, Jan 25, 2024 at 02:57:30AM +, Zhijie Hou (Fujitsu) wrote:
> >
> > Agreed. I split the original 0001 patch into 3 patches as suggested.
> > Here is the V68 patch set.
Thanks, I have pushed 0001.
>
> Thanks!
>
> Some
Hi,
On Thu, Jan 25, 2024 at 02:57:30AM +, Zhijie Hou (Fujitsu) wrote:
> On Wednesday, January 24, 2024 6:31 PM Amit Kapila
> wrote:
> >
> > On Tue, Jan 23, 2024 at 5:13 PM shveta malik wrote:
> > >
> > > Thanks Ajin for testing the patch. PFA v66 which fixes this issue.
> > >
> >
> > I
On Thu, Jan 25, 2024 at 9:13 AM Amit Kapila wrote:
>
> > 3) Removed syncing 'failover' on standby from remote_slot. The
> > 'failover' field will be false for synced slots. Since we do not
> > support sync to cascading standbys yet, thus failover=true was
> > misleading and unused there.
> >
>
>
Hi,
On Wed, Jan 24, 2024 at 04:09:15PM +0530, shveta malik wrote:
> On Wed, Jan 24, 2024 at 2:38 PM Bertrand Drouvot
> wrote:
> >
> > I also see Sawada-San's point and I'd vote for "sync_replication_slots".
> > Then for
> > the current feature I think "failover" and "on" should be the values to
Here are some review comments for v67-0002.
==
src/backend/replication/logical/slotsync.c
1.
+/* The sleep time (ms) between slot-sync cycles varies dynamically
+ * (within a MIN/MAX range) according to slot activity. See
+ * wait_for_slot_activity() for details.
+ */
+#define
On Wednesday, January 24, 2024 1:11 PM Masahiko Sawada
wrote:
> Here are random comments on slotsyncworker.c (v66):
Thanks for the comments:
>
> ---
> + elog(ERROR,
> +"cannot synchronize local slot \"%s\" LSN(%X/%X)"
> +" to remote slot's
On Wed, Jan 24, 2024 at 5:17 PM shveta malik wrote:
>
> PFA v67. Note that the GUC (enable_syncslot) name is unchanged. Once
> we have final agreement on the name, we can make the change in the
> next version.
>
> Changes in v67 are:
>
> 1) Addressed comments by Peter given in [1].
> 2) Addressed
Here are some review comments for the patch v67-0001.
==
1.
There are a couple of places checking for failover usage on a standby.
+ if (RecoveryInProgress() && failover)
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot enable failover for a replication slot"
+
On Wed, Jan 24, 2024 at 2:38 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Wed, Jan 24, 2024 at 01:51:54PM +0530, Amit Kapila wrote:
> > On Wed, Jan 24, 2024 at 11:24 AM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Jan 24, 2024 at 2:43 PM Amit Kapila
> > > wrote:
> > > >
> > > > >
> > > > > +/*
On Tue, Jan 23, 2024 at 5:13 PM shveta malik wrote:
>
> Thanks Ajin for testing the patch. PFA v66 which fixes this issue.
>
I think we should try to commit the patch as all of the design
concerns are resolved now. To achieve that, can we split the failover
setting patch into the following: (a)
Hi,
On Wed, Jan 24, 2024 at 01:51:54PM +0530, Amit Kapila wrote:
> On Wed, Jan 24, 2024 at 11:24 AM Masahiko Sawada
> wrote:
> >
> > On Wed, Jan 24, 2024 at 2:43 PM Amit Kapila wrote:
> > >
> > > >
> > > > +/* GUC variable */
> > > > +bool enable_syncslot = false;
> > > >
> > > > Is
On Wed, Jan 24, 2024 at 11:24 AM Masahiko Sawada wrote:
>
> On Wed, Jan 24, 2024 at 2:43 PM Amit Kapila wrote:
> >
> > >
> > > +/* GUC variable */
> > > +bool enable_syncslot = false;
> > >
> > > Is enable_syncslot a really good name? We use "enable" prefix only for
> > > planner
On Wed, Jan 24, 2024 at 2:43 PM Amit Kapila wrote:
>
> On Wed, Jan 24, 2024 at 10:41 AM Masahiko Sawada
> wrote:
> >
> > On Mon, Jan 22, 2024 at 3:58 PM Amit Kapila wrote:
> > >
> > > Can we think of using GetStandbyFlushRecPtr()? We probably need to
> > > expose this function, if this works
On Wed, Jan 24, 2024 at 10:41 AM Masahiko Sawada wrote:
>
> On Mon, Jan 22, 2024 at 3:58 PM Amit Kapila wrote:
> >
> > Can we think of using GetStandbyFlushRecPtr()? We probably need to
> > expose this function, if this works for the required purpose.
>
> GetStandbyFlushRecPtr() seems good. But
On Mon, Jan 22, 2024 at 3:58 PM Amit Kapila wrote:
>
> On Fri, Jan 19, 2024 at 3:55 PM shveta malik wrote:
> >
> > On Fri, Jan 19, 2024 at 10:35 AM Masahiko Sawada
> > wrote:
> > >
> > >
> > > Thank you for updating the patch. I have some comments:
> > >
> > > ---
> > > +latestWalEnd =
On Wed, Jan 24, 2024 at 8:52 AM Peter Smith wrote:
>
> Here are some comments for patch v66-0001.
>
> ==
> doc/src/sgml/catalogs.sgml
>
> 1.
> +
> + If true, the associated replication slots (i.e. the main slot and the
> + table sync slots) in the upstream database are
Here are some comments for patch v66-0001.
==
doc/src/sgml/catalogs.sgml
1.
+
+ If true, the associated replication slots (i.e. the main slot and the
+ table sync slots) in the upstream database are enabled to be
+ synchronized to the physical standbys
+
On Tue, Jan 23, 2024 at 9:45 AM Peter Smith wrote:
>
> Here are some review comments for v65-0002
Thanks Peter for the feedback. I have addressed these in v66.
>
> 4. GetStandbyFlushRecPtr
>
> /*
> - * Returns the latest point in WAL that has been safely flushed to disk, and
> - * can be sent
On Mon, Jan 22, 2024 at 10:30 PM shveta malik
wrote:
>
> On Mon, Jan 22, 2024 at 3:10 PM Amit Kapila
wrote:
> >
> > minor comments on the patch:
> > ===
>
> PFA v65 addressing the comments.
>
> Addressed comments by Peter in [1], comments by Hou-San in [2],
> comments by Amit
On Mon, Jan 22, 2024 at 8:42 PM Masahiko Sawada wrote:
>
> On Mon, Jan 22, 2024 at 9:26 PM Amit Kapila wrote:
> >
> > >
> > > Yes. IIUC the slotsync worker uses libpqwalreceiver to establish a
> > > non-replication connection and to execute SQL query. But neither of
> > > them are relevant with
Here are some review comments for v65-0002
==
0. General - GUCs in messages
I think it would be better for the GUC names to all be quoted. It's
not a rule (yet), but OTOH it seems to be the consensus most people
want. See [1].
This might impact the following messages:
0.1
+ ereport(ERROR,
On Mon, Jan 22, 2024 at 9:26 PM Amit Kapila wrote:
>
> On Mon, Jan 22, 2024 at 5:28 PM Masahiko Sawada wrote:
> >
> > On Sat, Jan 20, 2024 at 7:44 PM Amit Kapila wrote:
> > >
> > > On Sat, Jan 20, 2024 at 10:52 AM Dilip Kumar
> > > wrote:
> > > >
> > > > On Fri, Jan 19, 2024 at 5:24 PM Amit
On Mon, Jan 22, 2024 at 5:28 PM Masahiko Sawada wrote:
>
> On Sat, Jan 20, 2024 at 7:44 PM Amit Kapila wrote:
> >
> > On Sat, Jan 20, 2024 at 10:52 AM Dilip Kumar wrote:
> > >
> > > On Fri, Jan 19, 2024 at 5:24 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Wed, Jan 17, 2024 at 4:00 PM shveta
On Sat, Jan 20, 2024 at 7:44 PM Amit Kapila wrote:
>
> On Sat, Jan 20, 2024 at 10:52 AM Dilip Kumar wrote:
> >
> > On Fri, Jan 19, 2024 at 5:24 PM Amit Kapila wrote:
> > >
> > > On Wed, Jan 17, 2024 at 4:00 PM shveta malik
> > > wrote:
> > > >
> > >
> > > I had some off-list discussions with
On Fri, Jan 19, 2024 at 11:48 AM Peter Smith wrote:
>
> Here are some review comments for patch v63-0003.
Thanks Peter. I have addressed all in v65.
>
> 4b.
> It was a bit different when there were ERRORs but now they are LOGs
> somehow it seems wrong for this function to say what the *caller*
On Mon, Jan 22, 2024 at 12:28 PM Amit Kapila wrote:
>
> On Fri, Jan 19, 2024 at 3:55 PM shveta malik wrote:
> >
> > On Fri, Jan 19, 2024 at 10:35 AM Masahiko Sawada
> > wrote:
> > >
> > >
> > > Thank you for updating the patch. I have some comments:
> > >
> > > ---
> > > +latestWalEnd
On Mon, Jan 22, 2024 at 1:11 PM Bertrand Drouvot
wrote:
>
> Hi,
>
Thanks for sharing the feedback.
>
> > Then we also discussed whether extending libpqwalreceiver's connect
> > API is a good idea and whether we need to further extend it in the
> > future. As far as I can see, slotsync worker's
On Monday, January 22, 2024 11:36 AM shveta malik
wrote:
Hi,
> On Fri, Jan 19, 2024 at 4:18 PM shveta malik wrote:
> >
> > PFA v64.
>
> V64 fails to apply to HEAD due to a recent commit. Rebased it. PFA v64_2. It
> has
> no new changes.
I noticed few things while analyzing the patch.
1.
Hi,
On Fri, Jan 19, 2024 at 05:23:53PM +0530, Amit Kapila wrote:
> On Wed, Jan 17, 2024 at 4:00 PM shveta malik wrote:
> >
>
> Now, the concerns related to this could be that users would probably
> need to change existing mechanisms/tools to update priamry_conninfo
Yeah, for the ones that want
On Fri, Jan 19, 2024 at 3:55 PM shveta malik wrote:
>
> On Fri, Jan 19, 2024 at 10:35 AM Masahiko Sawada
> wrote:
> >
> >
> > Thank you for updating the patch. I have some comments:
> >
> > ---
> > +latestWalEnd = GetWalRcvLatestWalEnd();
> > +if (remote_slot->confirmed_lsn >
On Fri, Jan 19, 2024 at 5:23 PM Amit Kapila wrote:
>
> On Wed, Jan 17, 2024 at 4:00 PM shveta malik wrote:
> >
>
> I had some off-list discussions with Sawada-San, Hou-San, and Shveta
> on the topic of extending replication commands instead of using the
> current model where we fetch the
On Sat, Jan 20, 2024 at 10:52 AM Dilip Kumar wrote:
>
> On Fri, Jan 19, 2024 at 5:24 PM Amit Kapila wrote:
> >
> > On Wed, Jan 17, 2024 at 4:00 PM shveta malik wrote:
> > >
> >
> > I had some off-list discussions with Sawada-San, Hou-San, and Shveta
> > on the topic of extending replication
301 - 400 of 847 matches
Mail list logo
