Re: Unexpected Standby Shutdown on sync_replication_slots change

On Mon, Jul 28, 2025 at 6:55 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Jul 28, 2025 at 10:18 AM shveta malik <shveta.malik@gmail.com> wrote:
> >
> > On Fri, Jul 25, 2025 at 9:31 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > >
> > > On Fri, Jul 25, 2025 at 9:13 PM Hugo DUBOIS <hdubois@scaleway.com> wrote:
> > > > I'm not sure if there's a particular use case for wal_level and sync_replication_slots not matching on a primary. So, for me, Option 1 seems correct.
> > >
> > > I also prefer option #1.
> > >
> > > However, on second thought, if some users are already running a server
> > > (non-standby) with sync_replication_slots enabled and wal_level != logical
> > > in v17, switching to option #1 could break their setup after a minor
> > > version update. That would be surprising and confusing.
> > >
> > > To avoid that, I think we should go with option #2—at least for v17.
> > >
> >
> > I still prefer option #1. On HEAD, option #1 makes sense because
> > wal_level is a GUC that requires a server restart and thus using
> > ERROR/FATAL in this context is appropriate. It is also consistent with
> > the rest of the code (other modules) wherever wal_level checks are
> > performed during startup.
> >
> > Regarding the back branches, in the case of a primary server, if
> > sync_replication_slots is left ON while wal_level < logical, then
> > issuing a ERROR/FATAL is still more suitable. The user can simply
> > correct the configuration (disable sync_replication_slots) and proceed
> > with the startup after the minor version upgrade. Also, given that
> > option #1 is the better fit for HEAD, I don't think it's worth having
> > different behavior in the back branches.
> >
>
> +1.

I think it's basically not acceptable for a server to start up
successfully before a minor version update, but then fail to start with
the same configuration after the update. If that's absolutely necessary
to fix a bug, it might be justifiable. But in this case, I don't think
it's required.

Blocking startup when sync_replication_slots is enabled and wal_level
is not logical could be helpful. But that feels more like an improvement
than a bug fix. I'm fine adding that to master, but I don't think we should
apply it to old branches.

That said, both Shveta and Amit support backpatching this change,
so I'd like to hear more opinions before we decide.

Regards,

-- 
Fujii Masao