On Thu, Feb 11, 2021 at 3:20 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
>
> On Wed, Feb 10, 2021 at 8:39 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
> >
> > On Wed, Feb 10, 2021 at 10:02 AM Dilip Kumar <dilipbalaut@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
