On Fri, Feb 7, 2025 at 11:30 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Thu, Feb 6, 2025 at 9:30 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > 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? > > While the latter would be good for maintainability it would not be > advisable to change the behavior back and forth in back branches. I'd > like to make it clear what point of v2 patch (restring server startup > for pre-existing logical slots) is preferable over the first patch > (removing InHotStandby condition)? >
Sorry, I didn't understand your question. > If the approach of the first patch > is reasonably good, we could take it both in HEAD and backbranches. > > > > > > > > > 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. > > I think if we check standby mode here (i.e., adding StandbyMode), we > would not be able to invalidate logical slots during archive recovery. > That is, in the following scenario, we would hit the same assertion > failure: > > 1. setup the primary and the standby servers (with hot_standby=on). > 2. create a logical slot on the standby. > 3. on standby, set archive recovery settings (setting restore_command, > removing standby.signal, and creating recovery.signal etc.). > 4. on primary, lower wal_level to replica and restart. > 5. start standby (in archive recovery mode). > 6. execute the logical decoding after the archive recovery completes. > > And, you're right that InRecovery is true even for crash-recovery. But > is there any case where we invalidate logical slots due to > XLOG_PARAMETER_CHANGE during a crash recovery? > I am not sure but what about the cases where the server crashes immediately after logging this WAL record and the user reduced the wal_lvel before the next restart? I think in such a case we may ERROR out while restoring slots from the disk which will be before we will try to replay the WAL. However, I am not sure if it is a good idea to rely on that assumption. -- With Regards, Amit Kapila.
