On Wed, Feb 10, 2021 at 8:39 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Wed, Feb 10, 2021 at 10:02 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > > > I don't find any problem with this approach as well, but I personally > > feel that the other approach where we don't wait in any API and just > > return the recovery pause state is much simpler and more flexible. So > > I will make the pending changes in that patch and let's see what are > > the other opinion and based on that we can conclude. Thanks for the > > patch. > > Here is an updated version of the patch which fixes the last two open problems > 1. In RecoveryRequiresIntParameter set the recovery pause state in the > loop so that if recovery resumed and pause requested again we can set > to pause again. > 2. If the recovery state is already 'paused' then don't set it back to > the 'pause requested'. > > One more point is that in 'pg_wal_replay_pause' even if we don't > change the state because it was already set to the 'paused' then also > we call the WakeupRecovery. But I don't think there is any problem > with that, if we think that this should be changed then we can make > SetRecoveryPause return a bool such that if it doesn't do state change > then it returns false and in that case we can avoid calling > WakeupRecovery, but I felt that is unnecessary. Any other thoughts on > this?
IMO, that WakeupRecovery should not be a problem, because even now, if we issue a simple select pg_reload_conf(); (without even changing any config parameter), WakeupRecovery gets called. Thanks for the patch. I tested the new function and it works as expected. I have no further comments on the v13 patch. With Regards, Bharath Rupireddy. EnterpriseDB: http://www.enterprisedb.com