From: | Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> |
---|---|
To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Robert Haas <robertmhaas(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com> |
Subject: | Re: Is Recovery actually paused? |
Date: | 2021-02-12 04:33:32 |
Message-ID: | 20210212133332.f7b35bcd9594455a7105c273@sraoss.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, 11 Feb 2021 16:36:55 +0530
Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Thu, Feb 11, 2021 at 3:20 PM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> >
> > On Wed, Feb 10, 2021 at 8:39 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Feb 10, 2021 at 10:02 AM Dilip Kumar <dilipbalaut(at)gmail(dot)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.
I don't think that we need to include the waiting approach in pg_get_wal_replay_pause_state
patch. However, Horiguchi-san's patch may be useful for some users who want
pg_wal_replay_pause to wait until recovery gets paused instead of polling the
state from applications. So, I shink we could discuss this patch in another
thread as another commitfest entry independent from pg_get_wal_replay_pause_state.
> > > 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.
I have no futher comments on the v13 patch, too. Also, I agree with
Robert Haas's suggestions.
Regards,
Yugo Nagata
--
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
From | Date | Subject | |
---|---|---|---|
Next Message | Ajin Cherian | 2021-02-12 04:37:53 | Re: Single transaction in the tablesync worker? |
Previous Message | Zhihong Yu | 2021-02-12 04:23:40 | Re: Parallel INSERT (INTO ... SELECT ...) |