On Mon, Feb 24, 2025 at 2:51 PM Shubham Khanna
wrote:
>
> The attached patch contains the suggested changes.
>
Pushed with minor changes. In the latest patch, you have incorrectly
copied one of the checks from the previous version patch which I have
fixed before pushing the patch.
--
With Regar
On Mon, Feb 24, 2025 at 10:57 AM Amit Kapila wrote:
>
> On Tue, Feb 18, 2025 at 12:16 AM Shubham Khanna
> wrote:
> >
>
> -static void check_publisher(const struct LogicalRepInfo *dbinfo);
> -static char *setup_publisher(struct LogicalRepInfo *dbinfo);
> +static void check_publisher(const struct L
On Tue, Feb 18, 2025 at 12:16 AM Shubham Khanna
wrote:
>
-static void check_publisher(const struct LogicalRepInfo *dbinfo);
-static char *setup_publisher(struct LogicalRepInfo *dbinfo);
+static void check_publisher(const struct LogicalRepInfo *dbinfo, bool
two_phase);
+static char *setup_publishe
On Mon, Feb 17, 2025 at 9:43 PM vignesh C wrote:
>
> On Wed, 22 Jan 2025 at 16:23, Shubham Khanna
> wrote:
> >
> > Hi,
> >
> > Following the recent comments on Patch v13-0001, the patch was not
> > applied to the HEAD. I have addressed the feedback and prepared a
> > rebased version to incorporat
On Wed, 22 Jan 2025 at 16:23, Shubham Khanna
wrote:
>
> Hi,
>
> Following the recent comments on Patch v13-0001, the patch was not
> applied to the HEAD. I have addressed the feedback and prepared a
> rebased version to incorporate the necessary adjustments.
> Please find the updated patch.
The p
On Wed, 22 Jan 2025 at 16:23, Shubham Khanna
wrote:
>
> Hi,
>
> Following the recent comments on Patch v13-0001, the patch was not
> applied to the HEAD. I have addressed the feedback and prepared a
> rebased version to incorporate the necessary adjustments.
> Please find the updated patch.
>
> Th
Patch v14-0001 LGTM
==
Kind Regards,
Peter Smith.
Fujitsu Australia
Hi,
Following the recent comments on Patch v13-0001, the patch was not
applied to the HEAD. I have addressed the feedback and prepared a
rebased version to incorporate the necessary adjustments.
Please find the updated patch.
Thanks and regards,
Shubham Khanna.
v14-0001-Add-support-for-two-phas
On Mon, Jan 20, 2025 at 12:00 PM Peter Smith wrote:
>
> Hi Shubham,
>
> Patch v12-0001 LGTM.
>
> BTW, there is a precedent for passing the 'opt' arg but then only
> using one member from it. See function wait_for_end_recovery in this
> same file. So whether you decide to change the code per Shlok'
On Mon, Jan 20, 2025 at 1:34 AM Shlok Kyal wrote:
>
> On Fri, 17 Jan 2025 at 09:52, Shubham Khanna
> wrote:
> >
> > On Fri, Jan 17, 2025 at 5:43 AM Peter Smith wrote:
> > >
> > > Hi Shubham,
> > >
> > > Some review comments for patch v11-0001.
> > >
> > > ==
> > > src/sgml/ref/pg_createsubsc
Hi Shubham,
Patch v12-0001 LGTM.
BTW, there is a precedent for passing the 'opt' arg but then only
using one member from it. See function wait_for_end_recovery in this
same file. So whether you decide to change the code per Shlok's review
[1], or decide not to change it -- either way is OK for me
On Fri, 17 Jan 2025 at 09:52, Shubham Khanna
wrote:
>
> On Fri, Jan 17, 2025 at 5:43 AM Peter Smith wrote:
> >
> > Hi Shubham,
> >
> > Some review comments for patch v11-0001.
> >
> > ==
> > src/sgml/ref/pg_createsubscriber.sgml
> >
> > 1.
> > -must accept local connections.
> > +must
On Fri, Jan 17, 2025 at 5:43 AM Peter Smith wrote:
>
> Hi Shubham,
>
> Some review comments for patch v11-0001.
>
> ==
> src/sgml/ref/pg_createsubscriber.sgml
>
> 1.
> -must accept local connections.
> +must accept local connections. If you are planning to use the
> +--enable-two-p
Hi Shubham,
Some review comments for patch v11-0001.
==
src/sgml/ref/pg_createsubscriber.sgml
1.
-must accept local connections.
+must accept local connections. If you are planning to use the
+--enable-two-phase then you will also need to set the
+ appropriately.
The m
On Thu, Jan 16, 2025 at 3:15 PM Shlok Kyal wrote:
>
> On Wed, 15 Jan 2025 at 12:03, Shubham Khanna
> wrote:
> >
> > On Tue, Jan 14, 2025 at 4:53 PM Shlok Kyal wrote:
> > >
> > > On Fri, 27 Dec 2024 at 12:06, Shubham Khanna
> > > wrote:
> > > >
> > > > On Fri, Dec 27, 2024 at 11:30 AM vignesh C
On Thu, Jan 16, 2025 at 4:53 AM Peter Smith wrote:
>
> On Wed, Jan 15, 2025 at 9:24 PM Ajin Cherian wrote:
> >
> >
> >
> > On Wed, Jan 15, 2025 at 5:33 PM Shubham Khanna
> > wrote:
> > > Previously, the warning was necessary because the 'two-phase' option
> > > was not available, and users need
On Wed, Jan 15, 2025 at 3:54 PM Ajin Cherian wrote:
>
>
>
> On Wed, Jan 15, 2025 at 5:33 PM Shubham Khanna
> wrote:
> > Previously, the warning was necessary because the 'two-phase' option
> > was not available, and users needed to be informed about the default
> > behavior regarding 'two-phase'
On Wed, 15 Jan 2025 at 12:03, Shubham Khanna
wrote:
>
> On Tue, Jan 14, 2025 at 4:53 PM Shlok Kyal wrote:
> >
> > On Fri, 27 Dec 2024 at 12:06, Shubham Khanna
> > wrote:
> > >
> > > On Fri, Dec 27, 2024 at 11:30 AM vignesh C wrote:
> > > > > > > >
> > > > The documentation requires a minor upda
On Wed, Jan 15, 2025 at 9:24 PM Ajin Cherian wrote:
>
>
>
> On Wed, Jan 15, 2025 at 5:33 PM Shubham Khanna
> wrote:
> > Previously, the warning was necessary because the 'two-phase' option
> > was not available, and users needed to be informed about the default
> > behavior regarding 'two-phase'
On Wed, Jan 15, 2025 at 5:33 PM Shubham Khanna
wrote:
> Previously, the warning was necessary because the 'two-phase' option
> was not available, and users needed to be informed about the default
> behavior regarding 'two-phase' commit. However, with the recent
> addition of the 'two-phase' option
On Tue, Jan 14, 2025 at 4:53 PM Shlok Kyal wrote:
>
> On Fri, 27 Dec 2024 at 12:06, Shubham Khanna
> wrote:
> >
> > On Fri, Dec 27, 2024 at 11:30 AM vignesh C wrote:
> > > > > > >
> > > The documentation requires a minor update: instead of specifying
> > > subscriptions, the user will specify mu
On Fri, 27 Dec 2024 at 12:06, Shubham Khanna
wrote:
>
> On Fri, Dec 27, 2024 at 11:30 AM vignesh C wrote:
> > > > > >
> > The documentation requires a minor update: instead of specifying
> > subscriptions, the user will specify multiple databases, and the
> > subscription will be created on the s
On Fri, Jan 10, 2025 at 9:08 PM Ajin Cherian wrote:
>
> On Fri, Dec 27, 2024 at 5:36 PM Shubham Khanna
> wrote:
> >
> The patch no longer applies on HEAD. Please do rebase.
>
Sorry, I was mistaken. Ignore this. The patch does apply on HEAD.
regards,
Ajin Cherian
On Fri, Dec 27, 2024 at 5:36 PM Shubham Khanna
wrote:
>
The patch no longer applies on HEAD. Please do rebase.
regards,
Ajin Cherian
Fujitsu Australia
On Fri, Dec 27, 2024 at 11:30 AM vignesh C wrote:
> > > > >
> The documentation requires a minor update: instead of specifying
> subscriptions, the user will specify multiple databases, and the
> subscription will be created on the specified databases. Documentation
> should be updated accordingly
On Fri, 13 Dec 2024 at 15:33, Shubham Khanna
wrote:
>
> On Fri, Dec 13, 2024 at 2:39 PM vignesh C wrote:
> >
> > On Fri, 13 Dec 2024 at 13:01, Shubham Khanna
> > wrote:
> > >
> > > On Fri, Dec 13, 2024 at 12:20 PM Peter Smith
> > > wrote:
> > > >
> > > > Hi Shubham.
> > > >
> > > > Here are my
Hi Shubham.
The patch v8-0001 looks good to me.
FYI, there are a few pending patches [1][2][3] that might have some
tests/docs overlap with this one, so be on the lookout because if
those get pushed first your patch may require rebasing.
==
[1]
https://www.postgresql.org/message-id/flat/CAH
On Fri, Dec 13, 2024 at 2:39 PM vignesh C wrote:
>
> On Fri, 13 Dec 2024 at 13:01, Shubham Khanna
> wrote:
> >
> > On Fri, Dec 13, 2024 at 12:20 PM Peter Smith wrote:
> > >
> > > Hi Shubham.
> > >
> > > Here are my review comments for v6-0001.
> > >
> > > ==
> > > 1.
> > > +# Verify that the
On Fri, 13 Dec 2024 at 13:01, Shubham Khanna
wrote:
>
> On Fri, Dec 13, 2024 at 12:20 PM Peter Smith wrote:
> >
> > Hi Shubham.
> >
> > Here are my review comments for v6-0001.
> >
> > ==
> > 1.
> > +# Verify that the subtwophase is enabled ('e') in the pg_subscription
> > catalog
> > +$node
On Fri, Dec 13, 2024 at 12:20 PM Peter Smith wrote:
>
> Hi Shubham.
>
> Here are my review comments for v6-0001.
>
> ==
> 1.
> +# Verify that the subtwophase is enabled ('e') in the pg_subscription catalog
> +$node_s->poll_query_until('postgres',
> + "SELECT count(1) = 2 FROM pg_subscription W
Hi Shubham.
Here are my review comments for v6-0001.
==
1.
+# Verify that the subtwophase is enabled ('e') in the pg_subscription catalog
+$node_s->poll_query_until('postgres',
+ "SELECT count(1) = 2 FROM pg_subscription WHERE subtwophasestate = 'e';")
+ or die "Timed out while waiting for s
On Fri, Dec 13, 2024 at 4:42 AM Peter Smith wrote:
>
> Hi Shubham,
>
> Here are my review comments for the patch v5-0001.
>
> ==
> doc/src/sgml/ref/pg_createsubscriber.sgml
>
> 1.
> -must accept local connections.
> +must accept local connections. If you are planning to use the
> +
Hi Shubham,
Here are my review comments for the patch v5-0001.
==
doc/src/sgml/ref/pg_createsubscriber.sgml
1.
-must accept local connections.
+must accept local connections. If you are planning to use the
+--enable-two-phase switch then you will also need to set the
+ approp
On Thu, Dec 12, 2024 at 9:34 AM vignesh C wrote:
>
> On Thu, 12 Dec 2024 at 08:14, Hayato Kuroda (Fujitsu)
> wrote:
> >
> > Dear Shubham,
> >
> > > Thank you for pointing this out and for suggesting the changes. I
> > > agree with your approach.
> > > Also, I found a mistake in getopt_long and fi
On Thu, Dec 12, 2024 at 8:14 AM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Shubham,
>
> > Thank you for pointing this out and for suggesting the changes. I
> > agree with your approach.
> > Also, I found a mistake in getopt_long and fixed it in this version of
> > the patch.
> > The attached patch co
On Thu, Dec 12, 2024 at 6:04 AM Peter Smith wrote:
>
> Hi Shubham,
>
> Here are some review comments for the patch v4-0001.
>
> ==
> GENERAL.
>
> 1.
> After reading Vignesh's last review and then the pg_createsubscriber
> documentation I see there can be multiple databases simultaneously
> spe
On Thu, 12 Dec 2024 at 08:14, Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Shubham,
>
> > Thank you for pointing this out and for suggesting the changes. I
> > agree with your approach.
> > Also, I found a mistake in getopt_long and fixed it in this version of
> > the patch.
> > The attached patch cont
Dear Shubham,
> Thank you for pointing this out and for suggesting the changes. I
> agree with your approach.
> Also, I found a mistake in getopt_long and fixed it in this version of
> the patch.
> The attached patch contains the suggested changes.
Thanks for updating the patch. I think the patch
Hi Shubham,
Here are some review comments for the patch v4-0001.
==
GENERAL.
1.
After reading Vignesh's last review and then the pg_createsubscriber
documentation I see there can be multiple databases simultaneously
specified (by writing multiple -d switches) and in that case each one
gets i
On Wed, Dec 11, 2024 at 2:08 PM vignesh C wrote:
>
> On Wed, 11 Dec 2024 at 11:21, Shubham Khanna
> wrote:
> >
> > On Wed, Dec 11, 2024 at 4:21 AM Peter Smith wrote:
> >
> > I have fixed the given comments. The attached patch contains the
> > suggested changes.
>
> Since all the subscriptions ar
On Wed, 11 Dec 2024 at 11:21, Shubham Khanna
wrote:
>
> On Wed, Dec 11, 2024 at 4:21 AM Peter Smith wrote:
>
> I have fixed the given comments. The attached patch contains the
> suggested changes.
Since all the subscriptions are created based on the two_phase option
provided, there is no need to
On Wed, Dec 11, 2024 at 10:59 AM vignesh C wrote:
>
> On Tue, 10 Dec 2024 at 17:17, Shubham Khanna
> wrote:
> >
> > On Mon, Dec 9, 2024 at 7:43 PM vignesh C wrote:
> >
> > The attached Patch contains the required changes.
>
> Few comments:
> 1) This is not correct, currently enabling two_phase o
On Wed, Dec 11, 2024 at 4:21 AM Peter Smith wrote:
>
> Hi Shubham,
>
> Here are some review comments for patch v2-0001.
>
> ==
> GENERAL - the new option name
>
> 1.
> I am not sure if this new switch needed to be called
> '--enable-two-phase'; probably just calling it '--two-phase' would
> me
On Tue, 10 Dec 2024 at 17:17, Shubham Khanna
wrote:
>
> On Mon, Dec 9, 2024 at 7:43 PM vignesh C wrote:
>
> The attached Patch contains the required changes.
Few comments:
1) This is not correct, currently enabling two_phase option using
alter subscription is supported:
Notably, the replication
Hi Shubham,
Here are some review comments for patch v2-0001.
==
GENERAL - the new option name
1.
I am not sure if this new switch needed to be called
'--enable-two-phase'; probably just calling it '--two-phase' would
mean the same because specifying the switch already implies "enabling"
it t
On Tue, Dec 10, 2024 at 8:05 AM Peter Smith wrote:
>
> Hi Shubham,
>
> Here are some review comments for the patch v1-0001.
>
> Note -- I think Kuroda-san's idea to use a new switch like
> '--enable-two-phase' would eliminate lots of my review comments below
> about the inconsistencies with CREATE
On Tue, Dec 10, 2024 at 7:42 AM Hayato Kuroda (Fujitsu)
wrote:
>
> Dear Shubham,
>
> Thanks for the proposal!
>
> > I am writing to propose the addition of the two_phase option in
> > pg_createsubscriber. As discussed in [1], supporting this feature
> > during the development of pg_createsubscribe
On Mon, Dec 9, 2024 at 7:43 PM vignesh C wrote:
>
> On Mon, 9 Dec 2024 at 16:25, Shubham Khanna
> wrote:
> >
> > Hi all,
> >
> > I am writing to propose the addition of the two_phase option in
> > pg_createsubscriber. As discussed in [1], supporting this feature
> > during the development of pg_
Hi Shubham,
Here are some review comments for the patch v1-0001.
Note -- I think Kuroda-san's idea to use a new switch like
'--enable-two-phase' would eliminate lots of my review comments below
about the inconsistencies with CREATE SUBSCRIBER, and the current
confusion about defaults and argument
Dear Shubham,
Thanks for the proposal!
> I am writing to propose the addition of the two_phase option in
> pg_createsubscriber. As discussed in [1], supporting this feature
> during the development of pg_createsubscriber was planned for this
> version.
Yes, that was the pending item.
> The atta
On Mon, 9 Dec 2024 at 16:25, Shubham Khanna wrote:
>
> Hi all,
>
> I am writing to propose the addition of the two_phase option in
> pg_createsubscriber. As discussed in [1], supporting this feature
> during the development of pg_createsubscriber was planned for this
> version.
Yes this will be u
Hi all,
I am writing to propose the addition of the two_phase option in
pg_createsubscriber. As discussed in [1], supporting this feature
during the development of pg_createsubscriber was planned for this
version.
Currently, pg_createsubscriber creates subscriptions with the
two_phase option set t
52 matches
Mail list logo
