On Thu, Feb 11, 2021 at 3:20 PM Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com> wrote: > > 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.
Thanks for the review and testing. -- Regards, Dilip Kumar EnterpriseDB: http://www.enterprisedb.com
