On Wed, Jan 17, 2024 at 7:30 PM shveta malik wrote:
>
> On Wed, Jan 17, 2024 at 3:08 PM Bertrand Drouvot
> wrote:
> >
> > Hi,
> >
> > On Tue, Jan 16, 2024 at 05:27:05PM +0530, shveta malik wrote:
> > > PFA v62. Details:
> >
> > Thanks!
> >
> > > v62-003:
> > > It is a new patch which attempts to
On Thu, Jan 18, 2024 at 10:31 AM Peter Smith wrote:
>
> I have one question about the new code in v63-0002.
>
> ==
> src/backend/replication/logical/slotsync.c
>
> 1. ReplSlotSyncWorkerMain
>
> + Assert(SlotSyncWorker->pid == InvalidPid);
> +
> + /*
> + * Startup process signaled the slot
On Tue, Jan 9, 2024 at 11:15 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, January 9, 2024 9:17 AM Peter Smith wrote:
> >
...
> >
> > 2. ALTER_REPLICATION_SLOT ... FAILOVER
> >
> > +
> > +
> > +FAILOVER [ > class="parameter">boolean ]
> > +
> > +
> > +
On Wednesday, January 17, 2024 6:30 PM shveta malik
wrote:
> PFA v63.
I analyzed the security of the slotsync worker and replication connection a bit,
and didn't find issue. Here is detail:
1) data security
First, we are using the role used in primary_conninfo, the role used here is
On Wed, Jan 17, 2024 at 4:00 PM shveta malik wrote:
>
> PFA v63.
>
1.
+ /* User created slot with the same name exists, raise ERROR. */
+ if (!synced)
+ ereport(ERROR,
+ errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("exiting from slot synchronization on receiving"
+" the
I have one question about the new code in v63-0002.
==
src/backend/replication/logical/slotsync.c
1. ReplSlotSyncWorkerMain
+ Assert(SlotSyncWorker->pid == InvalidPid);
+
+ /*
+ * Startup process signaled the slot sync worker to stop, so if meanwhile
+ * postmaster ended up starting the
On Wed, Jan 17, 2024 at 3:08 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Tue, Jan 16, 2024 at 05:27:05PM +0530, shveta malik wrote:
> > PFA v62. Details:
>
> Thanks!
>
> > v62-003:
> > It is a new patch which attempts to implement slot-sync worker as a
> > special process which is neither a
Hi,
On Tue, Jan 16, 2024 at 05:27:05PM +0530, shveta malik wrote:
> PFA v62. Details:
Thanks!
> v62-003:
> It is a new patch which attempts to implement slot-sync worker as a
> special process which is neither a bgworker nor an Auxiliary process.
> Here we get the benefit of converting
A review on v62-006: failover-ready validation steps doc -
+ Next, check that the logical replication slots identified above exist on
+ the standby server. This step can be skipped if
+ standby_slot_names has been correctly configured.
+
+test_standby=# SELECT bool_and(synced
On Wed, Jan 17, 2024 at 6:43 AM Masahiko Sawada wrote:
>
> On Tue, Jan 16, 2024 at 6:40 PM shveta malik wrote:
> >
> > On Tue, Jan 16, 2024 at 12:59 PM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Jan 16, 2024 at 1:07 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Tue, Jan 16, 2024 at 9:03
On Tue, Jan 16, 2024 at 10:57 PM shveta malik wrote:
>
...
> v62-006:
> Separated the failover-ready validation steps into this separate
> doc-patch (which were earlier present in v61-002 and v61-003). Also
> addressed some of the doc comments by Peter in [1].
> Thanks Hou-San for providing this
Here is a review comment for the latest v62-0002 changes.
==
src/backend/replication/logical/slotsync.c
1.
+ if (namestrcmp(>data.plugin, remote_slot->plugin) == 0 &&
+ slot->data.database == dbid &&
+ remote_slot->restart_lsn == slot->data.restart_lsn &&
+ remote_slot->catalog_xmin ==
On Tue, Jan 16, 2024 at 6:40 PM shveta malik wrote:
>
> On Tue, Jan 16, 2024 at 12:59 PM Masahiko Sawada
> wrote:
> >
> > On Tue, Jan 16, 2024 at 1:07 PM Amit Kapila wrote:
> > >
> > > On Tue, Jan 16, 2024 at 9:03 AM shveta malik
> > > wrote:
> > > >
> > > > On Sat, Jan 13, 2024 at 12:54 PM
About v62-0001:
As stated in the patch comment:
But note that this commit does not yet include the capability to
actually sync the replication slot; the next patch will address that.
~~~
Because of this, I think it might be prudent to separate the
documentation portion from this patch so that
On Sat, Jan 13, 2024 at 12:54 PM Amit Kapila wrote:
>
> On Fri, Jan 12, 2024 at 5:50 PM shveta malik wrote:
> >
> > There are multiple approaches discussed and tried when it comes to
> > starting a slot-sync worker. I am summarizing all here:
> >
> > 1) Make slotsync worker as an Auxiliary
On Tuesday, January 16, 2024 9:27 AM Peter Smith wrote:
>
> Here are some review comments for patch v61-0002
Thanks for the comments.
>
> ==
> doc/src/sgml/logical-replication.sgml
>
> 1.
> +
> + Examples: logical replication failover
>
> The current documentation structure (after
On Tue, Jan 16, 2024 at 12:59 PM Masahiko Sawada wrote:
>
> On Tue, Jan 16, 2024 at 1:07 PM Amit Kapila wrote:
> >
> > On Tue, Jan 16, 2024 at 9:03 AM shveta malik wrote:
> > >
> > > On Sat, Jan 13, 2024 at 12:54 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Fri, Jan 12, 2024 at 5:50 PM
Hi,
On Sat, Jan 13, 2024 at 12:53:50PM +0530, Amit Kapila wrote:
> On Fri, Jan 12, 2024 at 5:50 PM shveta malik wrote:
> >
> > There are multiple approaches discussed and tried when it comes to
> > starting a slot-sync worker. I am summarizing all here:
> >
> > 1) Make slotsync worker as an
On Tue, Jan 16, 2024 at 1:07 PM Amit Kapila wrote:
>
> On Tue, Jan 16, 2024 at 9:03 AM shveta malik wrote:
> >
> > On Sat, Jan 13, 2024 at 12:54 PM Amit Kapila
> > wrote:
> > >
> > > On Fri, Jan 12, 2024 at 5:50 PM shveta malik
> > > wrote:
> > > >
> > > > There are multiple approaches
On Tue, Jan 16, 2024 at 9:37 AM Amit Kapila wrote:
>
> On Tue, Jan 16, 2024 at 9:03 AM shveta malik wrote:
> >
> Agreed and as said earlier I think it is better to make it a
> PGC_SIGHUP. Also, not sure we can say it is a non-standard way as
> already autovacuum launcher is handled in the same
On Tue, Jan 16, 2024 at 9:03 AM shveta malik wrote:
>
> On Sat, Jan 13, 2024 at 12:54 PM Amit Kapila wrote:
> >
> > On Fri, Jan 12, 2024 at 5:50 PM shveta malik wrote:
> > >
> > > There are multiple approaches discussed and tried when it comes to
> > > starting a slot-sync worker. I am
On Sat, Jan 13, 2024 at 12:54 PM Amit Kapila wrote:
>
> On Fri, Jan 12, 2024 at 5:50 PM shveta malik wrote:
> >
> > There are multiple approaches discussed and tried when it comes to
> > starting a slot-sync worker. I am summarizing all here:
> >
> > 1) Make slotsync worker as an Auxiliary
Here are some review comments for patch v61-0002
==
doc/src/sgml/logical-replication.sgml
1.
+
+ Examples: logical replication failover
The current documentation structure (after the patch is applied) looks
like this:
30.1. Publication
30.2. Subscription
30.2.1. Replication Slot
On Mon, Jan 15, 2024 at 2:54 PM Bertrand Drouvot
wrote:
>
> On Sat, Jan 13, 2024 at 10:05:52AM +0530, Amit Kapila wrote:
> > On Fri, Jan 12, 2024 at 12:07 PM Bertrand Drouvot
> > wrote:
> > > Maybe the "best" approach would be to have a way to detect that a slot
> > > has been
> > > re-created
Hi,
On Sat, Jan 13, 2024 at 10:05:52AM +0530, Amit Kapila wrote:
> On Fri, Jan 12, 2024 at 12:07 PM Bertrand Drouvot
> wrote:
> > Maybe the "best" approach would be to have a way to detect that a slot has
> > been
> > re-created on the primary (but that would mean rely on more than the slot
>
On Fri, Jan 12, 2024 at 5:50 PM shveta malik wrote:
>
> There are multiple approaches discussed and tried when it comes to
> starting a slot-sync worker. I am summarizing all here:
>
> 1) Make slotsync worker as an Auxiliary Process (like checkpointer,
> walwriter, walreceiver etc). The benefit
On Fri, Jan 12, 2024 at 12:07 PM Bertrand Drouvot
wrote:
>
> On Fri, Jan 12, 2024 at 08:42:39AM +0530, Amit Kapila wrote:
> > On Thu, Jan 11, 2024 at 9:11 PM Bertrand Drouvot
> > wrote:
> > >
> > > I'm not sure to follow here. If the remote slot is re-created then it
> > > would
> > > be also
On Fri, Jan 12, 2024 at 5:30 PM Masahiko Sawada wrote:
>
> On Thu, Jan 11, 2024 at 7:53 PM Amit Kapila wrote:
> >
> > On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
> > >
> > > +static bool
> > > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> > > {
> > > ...
> > >
On Thu, Jan 11, 2024 at 7:53 PM Amit Kapila wrote:
>
> On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
> >
> > +static bool
> > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> > {
> > ...
> > + /* Slot ready for sync, so sync it. */
> > + else
> > + {
> > + /*
> > + *
Hi,
On Fri, Jan 12, 2024 at 03:46:00AM +, Zhijie Hou (Fujitsu) wrote:
> On Thursday, January 11, 2024 11:42 PM Bertrand Drouvot
> wrote:
>
> Hi,
>
> > On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > IIUC, this would be a sync slot (so not usable until promotion) that
Hi,
On Fri, Jan 12, 2024 at 08:42:39AM +0530, Amit Kapila wrote:
> On Thu, Jan 11, 2024 at 9:11 PM Bertrand Drouvot
> wrote:
> >
> > On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > > >
> > > > To close the above race, I could think of the following ways:
> > > > 1. Drop and
On Thu, Jan 11, 2024 at 9:11 PM Bertrand Drouvot
wrote:
>
> On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
> > >
> > > +static bool
> > > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> > > {
> > > ...
On Thursday, January 11, 2024 11:42 PM Bertrand Drouvot
wrote:
Hi,
> On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila
> wrote:
> > >
> > > +static bool
> > > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot
> > >
On Thu, Jan 11, 2024 at 9:11 PM Bertrand Drouvot
wrote:
>
> On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> > On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
> > >
> > > +static bool
> > > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> > > {
> > > ...
Hi,
On Thu, Jan 11, 2024 at 04:22:56PM +0530, Amit Kapila wrote:
> On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
> >
> > +static bool
> > +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> > {
> > ...
> > + /* Slot ready for sync, so sync it. */
> > + else
> > + {
> >
On Thu, Jan 11, 2024 at 3:42 PM Dilip Kumar wrote:
>
> On Wed, Jan 10, 2024 at 5:53 PM Zhijie Hou (Fujitsu)
> wrote:
> >
>
> > > IIUC on the standby we just want to overwrite what we get from primary
> > > no? If
> > > so why we are using those APIs that are meant for the actual decoding
> > >
On Tue, Jan 9, 2024 at 6:39 PM Amit Kapila wrote:
>
> +static bool
> +synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
> {
> ...
> + /* Slot ready for sync, so sync it. */
> + else
> + {
> + /*
> + * Sanity check: With hot_standby_feedback enabled and
> + * invalidations
On Wed, Jan 10, 2024 at 5:53 PM Zhijie Hou (Fujitsu)
wrote:
>
> > IIUC on the standby we just want to overwrite what we get from primary no?
> > If
> > so why we are using those APIs that are meant for the actual decoding slots
> > where it needs to take certain logical decisions instead of
On Thu, Jan 11, 2024 at 7:28 AM Peter Smith wrote:
>
> Here are some review comments for patch v58-0002
Thank You for the feedback. These are addressed in v60. Please find my
response inline for a few.
> (FYI - I quickly checked with the latest v59-0002 and AFAIK all these
> review comments
Hi,
On Wed, Jan 10, 2024 at 12:23:14PM +, Zhijie Hou (Fujitsu) wrote:
> On Wednesday, January 10, 2024 2:26 PM Dilip Kumar
> wrote:
> >
> > + LogicalConfirmReceivedLocation(remote_slot->confirmed_lsn);
> > + LogicalIncreaseXminForSlot(remote_slot->confirmed_lsn,
> > +
Here are some review comments for patch v58-0002
(FYI - I quickly checked with the latest v59-0002 and AFAIK all these
review comments below are still relevant)
==
Commit message
1.
If a logical slot is invalidated on the primary, slot on the standby is also
invalidated.
~
/slot on the
On Tue, Jan 9, 2024 at 5:44 PM Zhijie Hou (Fujitsu)
wrote:
>
comments on 0002
1.
+/* Worker's nap time in case of regular activity on the primary server */
+#define WORKER_DEFAULT_NAPTIME_MS 10L /* 10 ms */
+
+/* Worker's nap time in case of no-activity on the primary server */
Here are some review comments for the patch v58-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 9, 2024 at 5:44 PM Zhijie Hou (Fujitsu)
wrote:
>
> V58-0002
>
+static bool
+synchronize_one_slot(WalReceiverConn *wrconn, RemoteSlot *remote_slot)
{
...
+ /* Slot ready for sync, so sync it. */
+ else
+ {
+ /*
+ * Sanity check: With hot_standby_feedback enabled and
+ * invalidations
On Tuesday, January 9, 2024 9:17 AM Peter Smith wrote:
>
> Here are some review comments for patch v57-0001.
Thanks for the comments!
>
> ==
> doc/src/sgml/protocol.sgml
>
> 1. CREATE_REPLICATION_SLOT ... FAILOVER
>
> +
> +FAILOVER [ class="parameter">boolean ]
> +
Here are some review comments for patch v57-0001.
==
doc/src/sgml/protocol.sgml
1. CREATE_REPLICATION_SLOT ... FAILOVER
+
+FAILOVER [ boolean ]
+
+
+ If true, the slot is enabled to be synced to the physical
+ standbys so that logical
On Fri, Jan 5, 2024 at 5:45 PM Amit Kapila wrote:
>
> On Fri, Jan 5, 2024 at 4:25 PM Dilip Kumar wrote:
> >
> > On Fri, Jan 5, 2024 at 8:59 AM shveta malik wrote:
> > >
> > I was going the the patch set again, I have a question. The below
> > comments say that we keep the failover option as
On Fri, Jan 5, 2024 at 4:25 PM Dilip Kumar wrote:
>
> On Fri, Jan 5, 2024 at 8:59 AM shveta malik wrote:
> >
> I was going the the patch set again, I have a question. The below
> comments say that we keep the failover option as PENDING until we have
> done the initial table sync which seems
On Fri, Jan 5, 2024 at 8:59 AM shveta malik wrote:
>
I was going the the patch set again, I have a question. The below
comments say that we keep the failover option as PENDING until we have
done the initial table sync which seems fine. But what happens if we
add a new table to the publication
Hi,
On Fri, Jan 05, 2024 at 10:00:53AM +0530, Amit Kapila wrote:
> On Fri, Jan 5, 2024 at 8:59 AM shveta malik wrote:
> >
> > On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot
> > wrote:
> > >
> > > 4 ===
> > >
> > > Looking closer, the only place where walrcv_connect() is called with
> > >
On Fri, Jan 5, 2024 at 8:59 AM shveta malik wrote:
>
> On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot
> wrote:
> >
> > 4 ===
> >
> > Looking closer, the only place where walrcv_connect() is called with
> > replication
> > set to false and logical set to false is in ReplSlotSyncWorkerMain().
>
On Thu, Jan 4, 2024 at 7:24 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Thu, Jan 04, 2024 at 10:27:31AM +0530, shveta malik wrote:
> > On Thu, Jan 4, 2024 at 9:18 AM shveta malik wrote:
> > >
> > > On Wed, Jan 3, 2024 at 6:33 PM Zhijie Hou (Fujitsu)
> > > wrote:
> > > >
> > > > On Tuesday,
Hi,
On Thu, Jan 04, 2024 at 10:27:31AM +0530, shveta malik wrote:
> On Thu, Jan 4, 2024 at 9:18 AM shveta malik wrote:
> >
> > On Wed, Jan 3, 2024 at 6:33 PM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Tuesday, January 2, 2024 6:32 PM shveta malik
> > > wrote:
> > > > On Fri, Dec 29, 2023
On Wed, Jan 3, 2024 at 4:57 PM Bertrand Drouvot
wrote:
>
> On Wed, Jan 03, 2024 at 04:20:03PM +0530, Amit Kapila wrote:
> > On Fri, Dec 29, 2023 at 12:32 PM Amit Kapila
> > wrote:
> > >
> > > On Fri, Dec 29, 2023 at 6:59 AM Masahiko Sawada
> > > wrote:
> > > >
> > > > On Wed, Dec 27, 2023 at
On Wed, Jan 3, 2024 at 4:20 PM Amit Kapila wrote:
>
> On Fri, Dec 29, 2023 at 12:32 PM Amit Kapila wrote:
> > I see your point and agree that users need to be careful. I was trying
> > to compare it with other places like the conninfo used with a
> > subscription where no separate dbname needs
On Wed, Jan 3, 2024 at 6:33 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Tuesday, January 2, 2024 6:32 PM shveta malik
> wrote:
> > On Fri, Dec 29, 2023 at 10:25 AM Amit Kapila
> >
> > The topup patch has also changed app_name to
> > {cluster_name}_slotsyncworker so that we do not confuse between
Hi,
On Wed, Jan 03, 2024 at 04:20:03PM +0530, Amit Kapila wrote:
> On Fri, Dec 29, 2023 at 12:32 PM Amit Kapila wrote:
> >
> > On Fri, Dec 29, 2023 at 6:59 AM Masahiko Sawada
> > wrote:
> > >
> > > On Wed, Dec 27, 2023 at 7:43 PM Amit Kapila
> > > wrote:
> > > >
> > > > >
> > > > > 3) The
On Fri, Dec 29, 2023 at 12:32 PM Amit Kapila wrote:
>
> On Fri, Dec 29, 2023 at 6:59 AM Masahiko Sawada wrote:
> >
> > On Wed, Dec 27, 2023 at 7:43 PM Amit Kapila wrote:
> > >
> > > >
> > > > 3) The slotsync worker uses primary_conninfo but also uses a new GUC
> > > > parameter, say
On Fri, Dec 29, 2023 at 6:59 AM Masahiko Sawada wrote:
>
> On Wed, Dec 27, 2023 at 7:43 PM Amit Kapila wrote:
> >
> > >
> > > 3) The slotsync worker uses primary_conninfo but also uses a new GUC
> > > parameter, say slot_sync_dbname, to specify the database to connect.
> > > The slot_sync_dbname
On Fri, Dec 29, 2023 at 7:18 AM Masahiko Sawada wrote:
>
> On Wed, Dec 27, 2023 at 7:13 PM shveta malik wrote:
> >
> > On Wed, Dec 27, 2023 at 11:36 AM Masahiko Sawada
> > wrote:
>
> > I was not aware if there is any way to connect if we
> > want to run SQL queries. I initially tried using
On Wed, Dec 27, 2023 at 7:13 PM shveta malik wrote:
>
> On Wed, Dec 27, 2023 at 11:36 AM Masahiko Sawada
> wrote:
> >
> > Hi,
> >
> > Thank you for working on this.
> >
> > On Tue, Dec 26, 2023 at 9:27 PM shveta malik wrote:
> > >
> > > On Tue, Dec 26, 2023 at 4:41 PM Zhijie Hou (Fujitsu)
> >
On Wed, Dec 27, 2023 at 7:43 PM Amit Kapila wrote:
>
> On Wed, Dec 27, 2023 at 11:36 AM Masahiko Sawada
> wrote:
> >
> > On Tue, Dec 26, 2023 at 9:27 PM shveta malik wrote:
> > >
> > > I would like to revisit the current dependency of slotsync worker on
> > > dbname used in 002 patch.
On Wed, Dec 27, 2023 at 11:36 AM Masahiko Sawada wrote:
>
> On Tue, Dec 26, 2023 at 9:27 PM shveta malik wrote:
> >
> > I would like to revisit the current dependency of slotsync worker on
> > dbname used in 002 patch. Currently we accept dbname in
> > primary_conninfo and thus the user has to
On Wed, Dec 27, 2023 at 11:36 AM Masahiko Sawada wrote:
>
> Hi,
>
> Thank you for working on this.
>
> On Tue, Dec 26, 2023 at 9:27 PM shveta malik wrote:
> >
> > On Tue, Dec 26, 2023 at 4:41 PM Zhijie Hou (Fujitsu)
> > wrote:
> > >
> > > On Wednesday, December 20, 2023 7:37 PM Amit Kapila
> >
Hi,
Thank you for working on this.
On Tue, Dec 26, 2023 at 9:27 PM shveta malik wrote:
>
> On Tue, Dec 26, 2023 at 4:41 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > On Wednesday, December 20, 2023 7:37 PM Amit Kapila
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 3:29 PM shveta malik
> > >
On Tue, Dec 26, 2023 at 4:41 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Wednesday, December 20, 2023 7:37 PM Amit Kapila
> wrote:
> >
> > On Wed, Dec 20, 2023 at 3:29 PM shveta malik
> > wrote:
> > >
> > > On Wed, Dec 20, 2023 at 9:12 AM Amit Kapila
> > wrote:
> > > >
> > > > On Tue, Dec 19, 2023
On Tue, Dec 26, 2023 at 3:00 PM Hayato Kuroda (Fujitsu)
wrote:
>
> > I think we should be able to detect it if we want but do we want to
> > add this restriction considering that users can always install the
> > required plugins after standby gets promoted? I think we can do either
> > way in
Dear Amit,
> I think we should be able to detect it if we want but do we want to
> add this restriction considering that users can always install the
> required plugins after standby gets promoted? I think we can do either
> way in this case but as we are not going to use these slots till the
>
On Thu, Dec 21, 2023 at 6:37 PM Hayato Kuroda (Fujitsu)
wrote:
>
> 10. synchronize_one_slot
>
> IIUC, this function can synchronize slots even if the used plugin on primary
> is
> not installed on the secondary server. If the slot is created by the slotsync
> worker, users will recognize it
On Fri, Dec 22, 2023 at 7:59 PM Bertrand Drouvot
wrote:
>
> Hi,
>
> On Fri, Dec 22, 2023 at 04:02:21PM +0530, shveta malik wrote:
> > PFA v53. Changes are:
>
> Thanks!
>
> > patch002:
> > 2) Addressed comments in [2] for v52-002.
> > 3) Fixed CFBot failure. The failure was caused by an assert in
Hi,
On Fri, Dec 22, 2023 at 04:02:21PM +0530, shveta malik wrote:
> PFA v53. Changes are:
Thanks!
> patch002:
> 2) Addressed comments in [2] for v52-002.
> 3) Fixed CFBot failure. The failure was caused by an assert in
> wait_for_primary_slot_catchup() for null confirmed_lsn received. In
>
On Thu, Dec 21, 2023 at 6:37 PM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Shveta,
>
> Thanks for updating the patch! Here is my comments for v52-0002.
Thanks for the feedback Kuroda-san. I have addressed these in v53.
> ~
> system-views.sgml
>
> 01.
>
> ```
> +
> +
> +
> +
On Thursday, December 21, 2023 5:39 PM Bertrand Drouvot
wrote:
>
> On Thu, Dec 21, 2023 at 02:23:12AM +, Zhijie Hou (Fujitsu) wrote:
> > On Wednesday, December 20, 2023 8:42 PM Zhijie Hou (Fujitsu)
> wrote:
> > >
> > > Attach the V51 patch set which addressed Kuroda-san's comments.
> > > I
Dear Shveta,
Thanks for updating the patch! Here is my comments for v52-0002.
~
system-views.sgml
01.
```
+
+
+
+ sync_state char
+
+
+ Defines slot synchronization state. This is meaningful on the physical
+ standby which has configured =
true.
+
Hi,
On Thu, Dec 21, 2023 at 02:23:12AM +, Zhijie Hou (Fujitsu) wrote:
> On Wednesday, December 20, 2023 8:42 PM Zhijie Hou (Fujitsu)
> wrote:
> >
> > Attach the V51 patch set which addressed Kuroda-san's comments.
> > I also tried to improve the test in 0003 to make it stable.
>
> The
On Wed, Dec 20, 2023 at 12:02 PM Peter Smith wrote:
>
> Here are some comments for the patch v50-0002.
Thank You for the feedback. I have addressed these in v52.
> ==
> GENERAL
>
> (I made a short study of all the ereports in this patch -- here are
> some findings)
>
> ~~~
>
> 0.1 Don't
On Thursday, December 21, 2023 12:25 PM Peter Smith
wrote:
>
> Here is a minor comment for v51-0001
>
> ==
> src/backend/replication/slot.c
>
> 1.
> +void
> +RereadConfigAndReInitSlotList(List **standby_slots) {
> + char*pre_standby_slot_names;
> +
> + /*
> + * If we are running on a
Here is a minor comment for v51-0001
==
src/backend/replication/slot.c
1.
+void
+RereadConfigAndReInitSlotList(List **standby_slots)
+{
+ char*pre_standby_slot_names;
+
+ /*
+ * If we are running on a standby, there is no need to reload
+ * standby_slot_names since we do not support
On Tuesday, December 19, 2023 9:05 PM Kuroda, Hayato/黒田 隼人
wrote:
>
> Dear Shveta,
>
> I resumed to review the patch. I will play more about it, but I can post some
> cosmetic comments.
Thanks for the comments.
>
>
> walsender.c
>
> 01. WalSndWaitForStandbyConfirmation
>
> ```
> +
On Wed, Dec 20, 2023 at 3:29 PM shveta malik wrote:
>
> On Wed, Dec 20, 2023 at 9:12 AM Amit Kapila wrote:
> >
> > On Tue, Dec 19, 2023 at 5:30 PM shveta malik wrote:
> > >
> > > Thanks for reviewing. I have addressed these in v50.
> > >
> >
> > I was looking at this patch to see if something
On Wed, Dec 20, 2023 at 9:12 AM Amit Kapila wrote:
>
> On Tue, Dec 19, 2023 at 5:30 PM shveta malik wrote:
> >
> > Thanks for reviewing. I have addressed these in v50.
> >
>
> I was looking at this patch to see if something smaller could be
> independently committable. I think we can extract
>
Dear Amit, Shveta,
> > walsender.c
> >
> > 01. WalSndWaitForStandbyConfirmation
> >
> > ```
> > +sleeptime = WalSndComputeSleeptime(GetCurrentTimestamp());
> > ```
> >
> > It works well, but I'm not sure whether we should use
> WalSndComputeSleeptime()
> > because the function won't be
Here are some comments for the patch v50-0002.
==
GENERAL
(I made a short study of all the ereports in this patch -- here are
some findings)
~~~
0.1 Don't need the parentheses.
Checking all the ereports I see that half of them have the redundant
parentheses and half of them do not; You
On Tue, Dec 19, 2023 at 6:35 PM Hayato Kuroda (Fujitsu)
wrote:
>
>
> walsender.c
>
> 01. WalSndWaitForStandbyConfirmation
>
> ```
> +sleeptime = WalSndComputeSleeptime(GetCurrentTimestamp());
> ```
>
> It works well, but I'm not sure whether we should use WalSndComputeSleeptime()
>
On Tue, Dec 19, 2023 at 5:30 PM shveta malik wrote:
>
> Thanks for reviewing. I have addressed these in v50.
>
I was looking at this patch to see if something smaller could be
independently committable. I think we can extract
pg_get_slot_invalidation_cause() and commit it as that function could
On Tue, Dec 19, 2023 at 5:17 PM shveta malik wrote:
>
> On Mon, Dec 18, 2023 at 4:22 PM Amit Kapila wrote:
> >
> > On Fri, Dec 15, 2023 at 11:03 AM shveta malik
> > wrote:
> > >
> > > Sorry, I missed attaching the patch. PFA v48.
> > >
> >
> > Few comments on v48_0002
> >
Dear Shveta,
I resumed to review the patch. I will play more about it, but I can post some
cosmetic comments.
walsender.c
01. WalSndWaitForStandbyConfirmation
```
+sleeptime = WalSndComputeSleeptime(GetCurrentTimestamp());
```
It works well, but I'm not sure whether we should use
On Tue, Dec 19, 2023 at 6:58 AM Peter Smith wrote:
>
> Here are some comments for the patch v49-0002.
>
Thanks for reviewing. I have addressed these in v50.
> (This is in addition to my review comments for v48-0002 [1])
>
> ==
> src/backend/access/transam/xlogrecovery.c
>
>
> 1.
On Tue, Dec 19, 2023 at 4:51 AM Peter Smith wrote:
>
> Here are some review comments for v48-0002
>
Thanks for reviewing. Most of these are addressed in v50. Please find
my comments for the rest.
> ==
> doc/src/sgml/config.sgml
>
> 1.
> + If slot synchronization is enabled then it
On Mon, Dec 18, 2023 at 4:22 PM Amit Kapila wrote:
>
> On Fri, Dec 15, 2023 at 11:03 AM shveta malik wrote:
> >
> > Sorry, I missed attaching the patch. PFA v48.
> >
>
> Few comments on v48_0002
>
Thanks for reviewing. These are addressed in v50. Please find my
comments
On Tue, Dec 19, 2023 at 4:51 AM Peter Smith wrote:
>
>
> ==
> doc/src/sgml/system-views.sgml
>
> 3.
> +
> + The hot standby can have any of these sync_state values for the slots
> but
> + on a hot standby, the slots with state 'r' and 'i' can neither be used
> + for
On Tue, Dec 19, 2023 at 6:58 AM Peter Smith wrote:
>
> Here are some comments for the patch v49-0002.
>
> (This is in addition to my review comments for v48-0002 [1])
>
> ==
> src/backend/access/transam/xlogrecovery.c
>
>
> 1. FinishWalRecovery
>
> + *
> + * We do not update the sync_state
Here are some comments for the patch v49-0002.
(This is in addition to my review comments for v48-0002 [1])
==
src/backend/access/transam/xlogrecovery.c
1. FinishWalRecovery
+ *
+ * We do not update the sync_state from READY to NONE here, as any failed
+ * update could leave some slots in
Here are some review comments for v48-0002
==
doc/src/sgml/config.sgml
1.
+ If slot synchronization is enabled then it is also necessary to
+ specify dbname in the
+ primary_conninfo string. This will only
be used for
+ slot synchronization. It is ignored
On Monday, December 11, 2023 5:31 PM shveta malik
wrote:
>
> On Thu, Dec 7, 2023 at 1:33 PM Peter Smith
> wrote:
> >
> > Hi.
> >
> > Here are my review comments for patch v43-0002.
> >
>
> > ==
> > src/backend/access/transam/xlogrecovery.c
> >
> > 13.
> > + /*
> > + * Shutdown the slot
On Fri, Dec 15, 2023 at 11:03 AM shveta malik wrote:
>
> Sorry, I missed attaching the patch. PFA v48.
>
Few comments on v48_0002
1.
+static void
+slotsync_reread_config(WalReceiverConn *wrconn)
{
...
+ pfree(old_primary_conninfo);
+ pfree(old_primary_slotname);
+
+ if
On Fri, Dec 15, 2023 at 5:55 PM Nisha Moond wrote:
>
> (1)
> When we try to create a subscription on standby using a synced slot
> that is in 'r' sync_state, the subscription will be created at the
> subscriber, and on standby, two actions will take place -
> (i) As copy_data is true by
Review for v47 patch -
(1)
When we try to create a subscription on standby using a synced slot
that is in 'r' sync_state, the subscription will be created at the
subscriber, and on standby, two actions will take place -
(i) As copy_data is true by default, it will switch the failover
state
On Thu, Dec 14, 2023 at 10:15 AM Peter Smith wrote:
>
> A review comment for v47-0001
>
Thanks for reviewing. I have addressed these in v48. There is some
design change around the code part where we were checking cascading
and were revalidating new GUC values on conf-reload. So code has
changed
On Wed, Dec 13, 2023 at 3:53 PM Peter Smith wrote:
>
> Hi Shveta, here are some review comments for v45-0002.
>
Thanks for the feedback. Addressed these in v48. Please find my
comments on some.
> ==
> doc/src/sgml/bgworker.sgml
>
> 1.
> +
> +
> + BgWorkerStart_PostmasterStart
> +
401 - 500 of 839 matches
Mail list logo
