Amit Kapila writes:
> Thanks for the patch and verification. It looks good to me as well.
> Shall we commit this today or wait for beta2 as per email by Tom [1]?
> We are on the borderline to see that most of the BF members have run
> with this, but as the change is straightforward, I think we can
On Fri, Jul 11, 2025 at 8:03 AM vignesh C wrote:
>
> On Thu, 10 Jul 2025 at 11:47, Dilip Kumar wrote:
> >
> > >
> > PFA, patch for v17.
>
> Thanks for working on this, I don't have any more comments.
>
Thanks for the patch and verification. It looks good to me as well.
Shall we commit this today
On Thu, 10 Jul 2025 at 11:47, Dilip Kumar wrote:
>
> On Thu, Jul 10, 2025 at 11:18 AM Dilip Kumar wrote:
> >
> > On Thu, Jul 10, 2025 at 11:11 AM vignesh C wrote:
> > >
> > > On Wed, 9 Jul 2025 at 17:47, Dilip Kumar wrote:
> > > >
> > > > On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera
> > > >
On Thu, Jul 10, 2025 at 2:35 PM Amit Kapila wrote:
>
> On Thu, Jul 10, 2025 at 2:23 PM vignesh C wrote:
> >
> > Few comments:
> > 1) With the current approach invalidation will not happen for logical
> > replication slots during upgrade operation, I felt we could retain
> > this assertion just in
On Thu, Jul 10, 2025 at 2:23 PM vignesh C wrote:
>
> Few comments:
> 1) With the current approach invalidation will not happen for logical
> replication slots during upgrade operation, I felt we could retain
> this assertion just in case in the future it gets called from
> elsewhere, do you feel t
On Thu, 10 Jul 2025 at 11:47, Dilip Kumar wrote:
>
> On Thu, Jul 10, 2025 at 11:18 AM Dilip Kumar wrote:
> >
> > On Thu, Jul 10, 2025 at 11:11 AM vignesh C wrote:
> > >
> > > On Wed, 9 Jul 2025 at 17:47, Dilip Kumar wrote:
> > > >
> > > > On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera
> > > >
On Thu, Jul 10, 2025 at 11:18 AM Dilip Kumar wrote:
>
> On Thu, Jul 10, 2025 at 11:11 AM vignesh C wrote:
> >
> > On Wed, 9 Jul 2025 at 17:47, Dilip Kumar wrote:
> > >
> > > On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera
> > > wrote:
> > > >
> > > > On 2025-Jul-09, Dilip Kumar wrote:
> > > >
>
On Thu, Jul 10, 2025 at 11:11 AM vignesh C wrote:
>
> On Wed, 9 Jul 2025 at 17:47, Dilip Kumar wrote:
> >
> > On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera wrote:
> > >
> > > On 2025-Jul-09, Dilip Kumar wrote:
> > >
> > > > On Wed, Jul 9, 2025 at 9:07 AM Dilip Kumar
> > > > wrote:
> > >
> > >
On Wed, 9 Jul 2025 at 17:47, Dilip Kumar wrote:
>
> On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera wrote:
> >
> > On 2025-Jul-09, Dilip Kumar wrote:
> >
> > > On Wed, Jul 9, 2025 at 9:07 AM Dilip Kumar wrote:
> >
> > > > After further consideration, I believe your proposed method is
> > > > super
On Thu, Jul 10, 2025 at 9:42 AM Tom Lane wrote:
>
> Amit Kapila writes:
> > What shall we do for exposed check_hook functions
> > check_max_slot_wal_keep_size() and
> > check_idle_replication_slot_timeout() in backbranch? Shall we remove
> > there as well or leave them to avoid the risk of breaki
Amit Kapila writes:
> What shall we do for exposed check_hook functions
> check_max_slot_wal_keep_size() and
> check_idle_replication_slot_timeout() in backbranch? Shall we remove
> there as well or leave them to avoid the risk of breaking any
> application?
It's impossible to believe that any ex
On Wed, Jul 9, 2025 at 5:46 PM Dilip Kumar wrote:
>
> On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera wrote:
> >
>
> > I propose a few comment updates on top of your patch.
>
> This comment updates LGTM, so included in v3.
>
What shall we do for exposed check_hook functions
check_max_slot_wal_keep
On Wed, Jul 9, 2025 at 5:29 PM Álvaro Herrera wrote:
>
> On 2025-Jul-09, Dilip Kumar wrote:
>
> > On Wed, Jul 9, 2025 at 9:07 AM Dilip Kumar wrote:
>
> > > After further consideration, I believe your proposed method is
> > > superior to forcing the max_slot_wal_keep_size to -1 via a check hook.
>
On 2025-Jul-09, Dilip Kumar wrote:
> On Wed, Jul 9, 2025 at 9:07 AM Dilip Kumar wrote:
> > After further consideration, I believe your proposed method is
> > superior to forcing the max_slot_wal_keep_size to -1 via a check hook.
> > The ultimate goal is to prevent WAL removal during a binary upg
On Wed, Jul 9, 2025 at 9:07 AM Dilip Kumar wrote:
>
> On Tue, Jul 8, 2025 at 5:24 PM Amit Kapila wrote:
> >
> > On Tue, Jul 8, 2025 at 4:49 PM Dilip Kumar wrote:
> > >
> > > On Tue, Jul 8, 2025 at 2:29 PM Amit Kapila
> > > wrote:
> > > >
> > > > On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar
>
On Tue, Jul 8, 2025 at 5:24 PM Amit Kapila wrote:
>
> On Tue, Jul 8, 2025 at 4:49 PM Dilip Kumar wrote:
> >
> > On Tue, Jul 8, 2025 at 2:29 PM Amit Kapila wrote:
> > >
> > > On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar wrote:
> > > >
> > > > On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
> > >
On Tue, Jul 8, 2025 at 4:49 PM Dilip Kumar wrote:
>
> On Tue, Jul 8, 2025 at 2:29 PM Amit Kapila wrote:
> >
> > On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar wrote:
> > >
> > > On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
> > > >
> > >
> > > > There's a bigger picture here, though. The fundam
On Tue, Jul 8, 2025 at 2:29 PM Amit Kapila wrote:
>
> On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar wrote:
> >
> > On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
> > >
> >
> > > There's a bigger picture here, though. The fundamental thing that
> > > I find wrong with the current code is that kno
On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar wrote:
>
> On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
> >
>
> > There's a bigger picture here, though. The fundamental thing that
> > I find wrong with the current code is that knowledge of and
> > responsibility for this max_slot_wal_keep_size ha
On Tue, Jul 8, 2025 at 11:32 AM Dilip Kumar wrote:
>
> On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
> >
> > Dilip Kumar writes:
> > >> IMHO we can just query the 'max_slot_wal_keep_size' after
> > >> start_postmaster() and check what is the final resultant value. So now
> > >> we will only th
On Mon, Jul 7, 2025 at 11:22 PM Tom Lane wrote:
>
> Dilip Kumar writes:
> >> IMHO we can just query the 'max_slot_wal_keep_size' after
> >> start_postmaster() and check what is the final resultant value. So now
> >> we will only throw an error if the final value is not -1. And we can
> >> remov
Dilip Kumar writes:
>> IMHO we can just query the 'max_slot_wal_keep_size' after
>> start_postmaster() and check what is the final resultant value. So now
>> we will only throw an error if the final value is not -1. And we can
>> remove the hook from the server all together. Thoughts?
> I coul
On Mon, Jul 7, 2025 at 9:47 AM Dilip Kumar wrote:
>
> On Sun, Jul 6, 2025 at 11:57 PM Tom Lane wrote:
> >
> > [ resurrecting an old thread ]
> >
> > Amit Kapila writes:
> > > +bool
> > > +check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
> > > +{
> > > + if (IsBinaryU
On Sun, Jul 6, 2025 at 11:57 PM Tom Lane wrote:
>
> [ resurrecting an old thread ]
>
> Amit Kapila writes:
> > +bool
> > +check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
> > +{
> > + if (IsBinaryUpgrade && *newval != -1)
> > + {
> > + GUC_check_errdet
[ resurrecting an old thread ]
Amit Kapila writes:
> +bool
> +check_max_slot_wal_keep_size(int *newval, void **extra, GucSource source)
> +{
> + if (IsBinaryUpgrade && *newval != -1)
> + {
> + GUC_check_errdetail("\"%s\" must be set to -1 during binary
> upgrade mode.",
> +
On Thu, Jan 11, 2024 at 10:01:16AM -0500, Tom Lane wrote:
> Alvaro Herrera writes:
>> "The logical replication launcher is disabled during binary upgrades, to
>> avoid logical replication workers running on the source cluster. That
>> would cause replication origins to move forward after having b
Alvaro Herrera writes:
> "The logical replication launcher is disabled during binary upgrades, to
> avoid logical replication workers running on the source cluster. That
> would cause replication origins to move forward after having been copied
> to the target cluster, potentially creating confli
On 2024-Jan-11, Michael Paquier wrote:
> Hence, how about something like that:
> "The logical replication launcher is disabled during binary upgrades,
> as a logical replication workers running on the cluster upgrading from
> could cause replication origins to move forward after they are copied
>
On Thu, Jan 11, 2024 at 11:25:44AM +0530, Amit Kapila wrote:
> On Thu, Jan 11, 2024 at 9:08 AM Michael Paquier wrote:
>> Hence, how about something like that:
>> "The logical replication launcher is disabled during binary upgrades,
>> as a logical replication workers running on the cluster upgradi
On Thu, Jan 11, 2024 at 9:08 AM Michael Paquier wrote:
>
> On Wed, Jan 10, 2024 at 06:02:12PM +0530, Amit Kapila wrote:
> > - if (max_logical_replication_workers == 0)
> > + /*
> > + * The logical replication launcher is disabled during binary upgrades,
> > + * as logical replication workers can s
On Wed, Jan 10, 2024 at 06:02:12PM +0530, Amit Kapila wrote:
> - if (max_logical_replication_workers == 0)
> + /*
> + * The logical replication launcher is disabled during binary upgrades,
> + * as logical replication workers can stream data during the upgrade
> + * which can cause replication orig
On Wed, Jan 10, 2024 at 10:11 AM Michael Paquier wrote:
>
> On Wed, Nov 15, 2023 at 07:58:06AM +0530, Amit Kapila wrote:
> > I am fine with this but there is no harm in doing this before or along
> > with the main patch. As of now, I don't see any problem but as the
> > main patch is still under r
On Wed, Nov 15, 2023 at 07:58:06AM +0530, Amit Kapila wrote:
> I am fine with this but there is no harm in doing this before or along
> with the main patch. As of now, I don't see any problem but as the
> main patch is still under review, so thought we could even wait for
> the patch to become "Rea
On Wed, Nov 15, 2023 at 07:58:06AM +0530, Amit Kapila wrote:
> I am fine with this but there is no harm in doing this before or along
> with the main patch. As of now, I don't see any problem but as the
> main patch is still under review, so thought we could even wait for
> the patch to become "Rea
On Mon, Nov 13, 2023 at 10:19 AM Michael Paquier wrote:
>
> On Mon, Nov 13, 2023 at 08:45:12AM +0530, Amit Kapila wrote:
> > I think we can be specific about logical replication stuff. I have not
> > done any study on autovacuum behavior related to this, so we can
> > update about it separately if
On Mon, Nov 13, 2023 at 08:45:12AM +0530, Amit Kapila wrote:
> I think we can be specific about logical replication stuff. I have not
> done any study on autovacuum behavior related to this, so we can
> update about it separately if required.
Autovacuum, as far as I recall, could decide to do some
On Sat, Nov 11, 2023 at 5:46 AM Michael Paquier wrote:
>
> On Fri, Nov 10, 2023 at 03:27:25PM +0530, Amit Kapila wrote:
> > I don't think this comment is correct because there won't be any apply
> > activity on the new cluster as after restoration subscriptions should
> > be disabled. On the old c
On Fri, Nov 10, 2023 at 03:27:25PM +0530, Amit Kapila wrote:
> I don't think this comment is correct because there won't be any apply
> activity on the new cluster as after restoration subscriptions should
> be disabled. On the old cluster, I think one problem is that the
> origins may move forward
On Fri, Nov 10, 2023 at 7:50 AM Michael Paquier wrote:
>
> On Thu, Nov 09, 2023 at 04:52:32PM +0900, Michael Paquier wrote:
> > Thanks!
>
> Also, please see also a patch about switching the logirep launcher to
> rely on IsBinaryUpgrade to prevent its startup. Any thoughts about
> that?
>
Prevent
On Thu, Nov 09, 2023 at 04:52:32PM +0900, Michael Paquier wrote:
> Thanks!
Also, please see also a patch about switching the logirep launcher to
rely on IsBinaryUpgrade to prevent its startup. Any thoughts about
that?
--
Michael
diff --git a/src/backend/replication/logical/launcher.c b/src/backen
On 2023-Nov-09, Amit Kapila wrote:
> These comments appear mostly repetitive to what is already mentioned
> in start_postmaster(). So, I have changed those referred to already
> written comments, and slightly adjusted the comments at another place.
> See attached.
I'd still rather mention check_o
On Thu, Nov 9, 2023 at 4:09 PM Alvaro Herrera wrote:
>
> On 2023-Nov-02, Kyotaro Horiguchi wrote:
>
> > diff --git a/src/backend/access/transam/xlog.c
> > b/src/backend/access/transam/xlog.c
> > index b541be8eec..46833f6ecd 100644
> > --- a/src/backend/access/transam/xlog.c
> > +++ b/src/backend/
On 2023-Nov-02, Kyotaro Horiguchi wrote:
> diff --git a/src/backend/access/transam/xlog.c
> b/src/backend/access/transam/xlog.c
> index b541be8eec..46833f6ecd 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -2063,6 +2063,29 @@ check_wal_segment_size
Dear Horiguchi-san, hackers,
> Thanks you for the comments!
Thanks for updating the patch!
I'm not sure it is intentional, but you might miss my post...I suggested to add
a
testcase.
I attached the updated version which is almost the same as Horiguchi-san's one,
but has a test. How do you think
On Thu, Nov 09, 2023 at 01:12:54PM +0530, Amit Kapila wrote:
> I think it is in an email[1].
Noted.
> I can take care of this unless we see some opposition to this idea.
Thanks!
--
Michael
signature.asc
Description: PGP signature
On Thu, Nov 9, 2023 at 12:38 PM Michael Paquier wrote:
>
> On Thu, Nov 09, 2023 at 05:04:28PM +1100, Peter Smith wrote:
> > Having a GUC hook for the "max_slot_wal_keep_size" seemed OK to me. If
> > the user overrides a GUC value (admittedly, maybe there is no reason
> > why they would want to) th
On Thu, Nov 09, 2023 at 05:04:28PM +1100, Peter Smith wrote:
> Having a GUC hook for the "max_slot_wal_keep_size" seemed OK to me. If
> the user overrides a GUC value (admittedly, maybe there is no reason
> why they would want to) then at least the hook will give an error,
> rather than us silently
At Thu, 9 Nov 2023 12:00:59 +0530, Amit Kapila wrote
in
> I have also proposed that as one of the alternatives but didn't get
> many votes. And, I think if the user is passing a special value of
> max_slot_wal_keep_size during the upgrade, it has to be a special
> case, and rejecting it upfront
On Thu, Nov 9, 2023 at 11:40 AM Kyotaro Horiguchi
wrote:
>
> At Thu, 9 Nov 2023 09:53:07 +0530, Amit Kapila
> wrote in
> > Michael, Horiguchi-San, and others, do you have any thoughts on what
> > is the best way to proceed?
>
> As I previously mentioned, I believe that if rejection is to be the
At Thu, 9 Nov 2023 09:53:07 +0530, Amit Kapila wrote
in
> Michael, Horiguchi-San, and others, do you have any thoughts on what
> is the best way to proceed?
As I previously mentioned, I believe that if rejection is to be the
course of action, it would be best to proceed with it sooner rather
th
On Thu, Nov 9, 2023 at 3:55 PM Michael Paquier wrote:
>
> On Thu, Nov 09, 2023 at 09:53:07AM +0530, Amit Kapila wrote:
> > On Tue, Nov 7, 2023 at 4:16 PM Amit Kapila wrote:
> >> But then we don't need the hardcoded value of
> >> max_logical_replication_workers as zero by pg_upgrade. I think doing
On Thu, Nov 09, 2023 at 09:53:07AM +0530, Amit Kapila wrote:
> On Tue, Nov 7, 2023 at 4:16 PM Amit Kapila wrote:
>> But then we don't need the hardcoded value of
>> max_logical_replication_workers as zero by pg_upgrade. I think doing
>> IsBinaryUpgrade for slots won't be neat, so we anyway need to
On Tue, Nov 7, 2023 at 4:16 PM Amit Kapila wrote:
>
> On Tue, Nov 7, 2023 at 8:12 AM Michael Paquier wrote:
> >
> > On Tue, Nov 07, 2023 at 07:59:46AM +0530, Amit Kapila wrote:
> > > Do you mean to say that if 'IsBinaryUpgrade' is true then let's not
> > > allow to launch launcher or apply worker
On Tue, Nov 7, 2023 at 8:12 AM Michael Paquier wrote:
>
> On Tue, Nov 07, 2023 at 07:59:46AM +0530, Amit Kapila wrote:
> > Do you mean to say that if 'IsBinaryUpgrade' is true then let's not
> > allow to launch launcher or apply worker? If so, I guess this won't be
> > any better than prohibiting
On Tue, Nov 07, 2023 at 07:59:46AM +0530, Amit Kapila wrote:
> Do you mean to say that if 'IsBinaryUpgrade' is true then let's not
> allow to launch launcher or apply worker? If so, I guess this won't be
> any better than prohibiting at an early stage or explicitly overriding
> those with internal
On Sun, Nov 5, 2023 at 5:33 AM Michael Paquier wrote:
>
> On Fri, Nov 03, 2023 at 01:33:26PM +1100, Peter Smith wrote:
> > On Fri, Nov 3, 2023 at 1:11 PM Amit Kapila wrote:
> >> Now, that Michael also committed another similar change in commit
> >> 7021d3b176, it is better to be consistent in bot
On Fri, Nov 03, 2023 at 01:33:26PM +1100, Peter Smith wrote:
> On Fri, Nov 3, 2023 at 1:11 PM Amit Kapila wrote:
>> Now, that Michael also committed another similar change in commit
>> 7021d3b176, it is better to be consistent in both cases. So, either we
>
> I agree. Both patches are setting a s
On Fri, Nov 3, 2023 at 1:11 PM Amit Kapila wrote:
>
> On Thu, Nov 2, 2023 at 2:36 PM Amit Kapila wrote:
> >
> > On Thu, Nov 2, 2023 at 11:32 AM Michael Paquier wrote:
> > >
> > > On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote:
> > > > On Thu, Nov 2, 2023 at 2:25 PM Peter Smith
> >
On Thu, Nov 2, 2023 at 2:36 PM Amit Kapila wrote:
>
> On Thu, Nov 2, 2023 at 11:32 AM Michael Paquier wrote:
> >
> > On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote:
> > > On Thu, Nov 2, 2023 at 2:25 PM Peter Smith wrote:
> > >> Checking this patch yesterday prompted me to create a n
On Thu, Nov 2, 2023 at 11:32 AM Michael Paquier wrote:
>
> On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote:
> > On Thu, Nov 2, 2023 at 2:25 PM Peter Smith wrote:
> >> Checking this patch yesterday prompted me to create a new thread
> >> questioning the inconsistencies of the "GUC name
On Thu, Nov 02, 2023 at 02:32:07PM +1100, Peter Smith wrote:
> On Thu, Nov 2, 2023 at 2:25 PM Peter Smith wrote:
>> Checking this patch yesterday prompted me to create a new thread
>> questioning the inconsistencies of the "GUC names in messages". In
>> that thread, Tom Lane replied and gave some
On Thu, Nov 2, 2023 at 2:25 PM Peter Smith wrote:
>
> On Thu, Nov 2, 2023 at 1:58 PM Kyotaro Horiguchi
> wrote:
> >
> > Thanks you for the comments!
> >
> > At Wed, 1 Nov 2023 18:08:19 +1100, Peter Smith
> > wrote in
> > > Hi, here are some minor review comments for the v3 patch.
> > >
> > > ==
On Thu, Nov 2, 2023 at 1:58 PM Kyotaro Horiguchi
wrote:
>
> Thanks you for the comments!
>
> At Wed, 1 Nov 2023 18:08:19 +1100, Peter Smith wrote
> in
> > Hi, here are some minor review comments for the v3 patch.
> >
> > ==
> > src/backend/access/transam/xlog.c
>
...
> > 2.
>
> > + GUC_check
Thanks you for the comments!
At Wed, 1 Nov 2023 18:08:19 +1100, Peter Smith wrote in
> Hi, here are some minor review comments for the v3 patch.
>
> ==
> src/backend/access/transam/xlog.c
> I asked ChatGPT to suggest alternative wording for that comment, and
> it came up with something tha
Hi, here are some minor review comments for the v3 patch.
==
src/backend/access/transam/xlog.c
1. check_max_slot_wal_keep_size
+/*
+ * GUC check_hook for max_slot_wal_keep_size
+ *
+ * If WALs required by logical replication slots are removed, the slots are
+ * unusable. While pg_upgrade se
Dear Horiguchi-san,
Thanks for making the patch!
> > 4. I think a test case to hit the error in the check hook in
> > 003_logical_slots.pl will help a lot here - not only covers the code
> > but also helps demonstrate how one can reach the error.
>
> Yeah, of course. I was planning to add tests
On Tue, Oct 31, 2023 at 4:00 PM Bharath Rupireddy
wrote:
>
> > > 5. A possible problem with this check_hook approach is that it doesn't
> > > let anyone setting max_slot_wal_keep_size to a value other than -1
> > > during pg_ugprade even if someone doesn't have logical slots or
> > > doesn't want
On Tue, Oct 31, 2023 at 2:19 PM Kyotaro Horiguchi
wrote:
>
> > GUC_check_errcode(ERRCODE_INVALID_PARAMETER_VALUE);
> > GUC_check_errmsg(""\"max_slot_wal_keep_size\" must be set
> > to -1 when in binary upgrade mode");
> > GUC_check_errdetail("A value of -1 preve
At Mon, 30 Oct 2023 14:55:01 +0530, Bharath Rupireddy
wrote in
> > I get it. I agree to go with just the assert because the GUC
> > check_hook kinda tightens the screws against setting
> > max_slot_wal_keep_size to a value other than -1 during the binary
> > upgrade,
Thanks for being on the sam
On Mon, Oct 30, 2023 at 2:31 PM Bharath Rupireddy
wrote:
>
Never mind. Previous message was accidentally sent before I finished
writing my comments.
> Yeah. The check_hook is called even after the param is specified in
> postgresql.conf during the upgrade, so I see no problem there.
>
> > The er
Dear Bharath,
> Will the check_hook approach work correctly?
I tested by using the first version and worked well (rejected). Please see the
log which recorded the output and log. Below lines were copied from server
log and found that max_slot_wal_keep_size must not be >= 0.
```
waiting for serve
On Mon, Oct 30, 2023 at 1:42 PM Kyotaro Horiguchi
wrote:
>
> At Mon, 30 Oct 2023 12:38:47 +0530, Bharath Rupireddy
> wrote in
> > Will the check_hook approach work correctly? I haven't checked that by
> > myself, but I see InitializeGUCOptions() getting called before
> > IsBinaryUpgrade is set t
At Mon, 30 Oct 2023 12:38:47 +0530, Bharath Rupireddy
wrote in
> Will the check_hook approach work correctly? I haven't checked that by
> myself, but I see InitializeGUCOptions() getting called before
> IsBinaryUpgrade is set to true and the passed-in config options ('c')
> are parsed.
I'm not
At Mon, 30 Oct 2023 03:36:41 +, "Zhijie Hou (Fujitsu)"
wrote in
> Thanks for the diff and I think the approach basically works.
>
> One notable behavior of this approach it will reject the GUC setting even if
> there
> are no slots on old cluster or user set the value to a big enough value
At Mon, 30 Oct 2023 08:51:18 +0530, Amit Kapila wrote
in
> I think we can simply change that error message to assert if we want
> to go with the check hook idea of yours. BTW, can we add
> GUC_check_errdetail() with a better message as some of the other check
> function uses? Also, I guess we ca
On Mon, Oct 30, 2023 at 8:51 AM Amit Kapila wrote:
>
> > This discussion seems like a bit off from my point. I suggested adding
> > a check for that setting when IsBinaryUpgraded is true at the GUC
> > level as shown in the attached patch. I believe Álvaro made a similar
> > suggestion. While the
On Monday, October 30, 2023 10:29 AM Kyotaro Horiguchi
wrote:
>
> At Fri, 27 Oct 2023 14:57:10 +0530, Amit Kapila
> wrote in
> > On Fri, Oct 27, 2023 at 2:02 PM Alvaro Herrera
> wrote:
> > >
> > > On 2023-Oct-27, Kyotaro Horiguchi wrote:
> > >
> > > > @@ -1433,8 +1433,8 @@
> InvalidatePossibly
On Mon, Oct 30, 2023 at 7:58 AM Kyotaro Horiguchi
wrote:
>
> At Fri, 27 Oct 2023 14:57:10 +0530, Amit Kapila
> wrote in
> > On Fri, Oct 27, 2023 at 2:02 PM Alvaro Herrera
> > wrote:
> > >
> > > On 2023-Oct-27, Kyotaro Horiguchi wrote:
> > >
> > > > @@ -1433,8 +1433,8 @@
> > > > InvalidatePoss
At Fri, 27 Oct 2023 14:57:10 +0530, Amit Kapila wrote
in
> On Fri, Oct 27, 2023 at 2:02 PM Alvaro Herrera
> wrote:
> >
> > On 2023-Oct-27, Kyotaro Horiguchi wrote:
> >
> > > @@ -1433,8 +1433,8 @@
> > > InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
> > >
On Fri, Oct 27, 2023 at 2:02 PM Alvaro Herrera wrote:
>
> On 2023-Oct-27, Kyotaro Horiguchi wrote:
>
> > @@ -1433,8 +1433,8 @@
> > InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
> > {
> > ereport(ERROR,
> >
On 2023-Oct-27, Kyotaro Horiguchi wrote:
> @@ -1433,8 +1433,8 @@
> InvalidatePossiblyObsoleteSlot(ReplicationSlotInvalidationCause cause,
> {
> ereport(ERROR,
>
> errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> -
At Fri, 27 Oct 2023 09:51:43 +0530, Amit Kapila wrote
in
> On Fri, Oct 27, 2023 at 9:37 AM Peter Smith wrote:
> > IIUC the only possible way to reach this error (according to the
> > comment preceding it) is by the user overriding the GUC value (which
> > was defaulted -1) on the command line.
On Fri, Oct 27, 2023 at 9:52 AM Amit Kapila wrote:
>
> > errmsg("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"),
> > errhint("Do not override \"max_slot_wal_keep_size\" using command line
> > options."));
> >
>
> But OTOH, we don't have a value of user-passed options to ensure t
On Fri, Oct 27, 2023 at 9:36 AM Amit Kapila wrote:
>
> On Fri, Oct 27, 2023 at 8:52 AM Bharath Rupireddy
> wrote:
> >
> > On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi:
> > The above errhint LGTM. How about a slightly different errmsg, like
> > the following?
> >
> > +errm
On Fri, Oct 27, 2023 at 9:37 AM Peter Smith wrote:
>
> On Fri, Oct 27, 2023 at 1:58 PM Kyotaro Horiguchi
> wrote:
> >
> > Hello.
> >
> > Some messages recently introduced by commit 29d0a77fa6 seem to deviate
> > slightly from our standards.
> >
> > + if (*invalidated && SlotIsLogica
On Fri, Oct 27, 2023 at 1:58 PM Kyotaro Horiguchi
wrote:
>
> Hello.
>
> Some messages recently introduced by commit 29d0a77fa6 seem to deviate
> slightly from our standards.
>
> + if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
> + {
> + e
On Fri, Oct 27, 2023 at 8:52 AM Bharath Rupireddy
wrote:
>
> On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi:
> The above errhint LGTM. How about a slightly different errmsg, like
> the following?
>
> +errmsg("cannot invalidate replication slots when
> in binary upgrade mode"
On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi
wrote:
>
> Hello.
>
> Some messages recently introduced by commit 29d0a77fa6 seem to deviate
> slightly from our standards.
>
> + if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
> + {
> + e
88 matches
Mail list logo
