On Fri, Feb 7, 2025 at 12:11 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Wed, Feb 5, 2025 at 10:17 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Thu, Feb 6, 2025 at 12:30 AM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > > > > I've updated the patch accordingly. > > > > > > > Today, again thinking about the proposed fix, I was wondering about > > the following case. Say, on hot_standby, the user created a logical > > slot, then shut down hot_standby, turn off the hot_standby flag, and > > restart standby. This is allowed today but not after the patch. It is > > possible that the user can promote a non-hot_standby server after its > > restart in the previous step and can reuse the logical slot. So, is it > > okay to change this behavior? If not, then we may need to go back to > > the first fix proposed by you in this thread. > > While non hot standby, no one can advance logical slots, meaning the > standby server ends up accumulating WAL records and also causing the > bloat on the primary if hot_standby_feedback is enabled. Given this > outcome, I though that the current patch approach would be reasonable. >
Agreed as I also can't think of any case where the user would require a logical slot on a non-hotstandby server. > On the other hand, as you pointed out, it would not be good in terms > of compatibility as we end up limiting potentially existing use cases. > And it would be prefectly fine if users promote the standby server > shortly after starting up with hot_standby = off. True but it sounds like there is more harm than benefit. It seems reasonable to do this on HEAD. Shall we think of doing it differently in HEAD and back-branches or let's restrict as your v2 patch is doing and if by any chance users complain about it we can try to fix it in another way? > > But, similarly, is > there any concerns of my proposed fix? > - if (InRecovery && InHotStandby && + if (InRecovery && xlrec.wal_level < WAL_LEVEL_LOGICAL && Won't the InRecovery be true even for crash-recovery as well? I think we need to check standby mode here. -- With Regards, Amit Kapila.
