Re: pause recovery if pitr target not reached

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: peter(dot)eisentraut(at)2ndquadrant(dot)com
Cc: leif(at)lako(dot)no, michael(at)paquier(dot)xyz, masao(dot)fujii(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pause recovery if pitr target not reached
Date: 2020-01-28 05:01:55
Message-ID: 20200128.140155.1763928318284077564.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello.

At Mon, 27 Jan 2020 12:16:02 +0100, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote in
> On 2020-01-15 05:02, Kyotaro Horiguchi wrote:
> > FWIW, I restate this (perhaps) more clearly.
> > At Wed, 15 Jan 2020 11:02:24 +0900 (JST), Kyotaro Horiguchi
> > <horikyota(dot)ntt(at)gmail(dot)com> wrote in
> >> recvoery_target_* is not cleared after startup. If a server crashed
> >> just after the last shutdown checkpoint, any recovery_target_* setting
> >> prevents the server from starting regardless of its value.
> > recvoery_target_* is not automatically cleared after a successful
> > archive recovery. After that, if the server crashed just after the
> > last shutdown checkpoint, any recovery_target_* setting prevents the
> > server from starting regardless of its value.
>
> Thank you for this clarification. Here is a new patch that addresses
> that and also the other comments raised about my previous patch.

The code looks fine, renaming reachedStopPoint to
reachedRecoveryTarget looks very nice. Doc part looks fine, too.

PostgresNode.pm
+Is has_restoring is used, standby mode is used by default. To use

"Is has_restoring used,", or "If has_restoring is used," ?

+ $params{standby} = 1 unless defined $params{standby};
..
- $self->enable_restoring($root_node) if $params{has_restoring};
+ $self->enable_restoring($root_node, $params{standby}) if $params{has_restoring};
...
+ if ($standby)
+ {
+ $self->set_standby_mode();
+ }
+ else
+ {
+ $self->set_recovery_mode();
+ }

The change seems aiming not to break compatibility with external test
scripts, but it looks quite confusing to me. The problem here is both
enable_streaming/restoring independently put trigger files, so don't
we separte placing of trigger files out of the functions?

As you know, set_standby_mode and set_recovery_mode are described as
"internal" but actually used by some test scripts. I think it's
preferable that the functions are added in POD rather than change the
callers not to used them.

The attached patch on top of yours does that. It might be too much but
what do you think about that?

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
0001-Refactor-init_from_backup.patch text/x-patch 6.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2020-01-28 05:05:24 Re: Should we add xid_current() or a int8->xid cast?
Previous Message Mark Dilger 2020-01-28 04:39:03 Re: [HACKERS] advanced partition matching algorithm for partition-wise join