From: Sergei Kornilov <sk@zsrv.org>
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: "masao.fujii@oss.nttdata.com" <masao.fujii@oss.nttdata.com>,
	"a.lubennikova@postgrespro.ru" <a.lubennikova@postgrespro.ru>,
	"robertmhaas@gmail.com" <robertmhaas@gmail.com>,
	"michael@paquier.xyz" <michael@paquier.xyz>,
	"andres@anarazel.de" <andres@anarazel.de>,
	"peter.eisentraut@2ndquadrant.com" <peter.eisentraut@2ndquadrant.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
In-Reply-To: <20201110.205321.1681552377835477837.horikyota.ntt@gmail.com>
References: <108021604695421@mail.yandex.ru>
	<20201110.135420.826057682358861616.horikyota.ntt@gmail.com>
	<4363101605003835@mail.yandex.ru>
 <20201110.205321.1681552377835477837.horikyota.ntt@gmail.com>
Subject: Re: Allow some recovery parameters to be changed with reload
MIME-Version: 1.0
Date: Tue, 10 Nov 2020 23:52:08 +0300
Message-Id: <355121605038455@mail.yandex.ru>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=utf-8
Precedence: bulk

Hello

> Even if PITR is commanded, crash recovery can run before starting
> archive recovery if the server was not gracefully shut down.

Hmm... Still not sure how it's possible. Both readRecoverySignalFile and validateRecoveryParameters are called early in StartupXLOG. If PITR was commanded - we follow PITR logic. If requested recovery stop point is before consistent recovery point we shutdown the database with another FATAL.
I mean such place:
https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/access/transam/xlog.c;h=9d3f1c12fc56f61da4d2b9bf08c54d31b9757ef7;hb=29be9983a64c011eac0b9ee29895cce71e15ea77#l6891
If we start recovery by any reason and a archive recovery was requested - we start archive recovery instead of crash recovery.

> I don't know. I just think that it is not proper that "ALTER SYSTEM" +
> config-reload causes server stop.

I got your point. How about pause the recovery process? Like proposed in https://commitfest.postgresql.org/30/2489/
For example,
* restore_command become empty on SIGHUP while PITR was requested
* we set recovery to pause
* if user call pg_wal_replay_resume and restore_command is still empty - we shutdown the database
* if user fix restore_command - we continue restore.

But it seems complicated if we just don't need special handling here. We still require restore_command to be set to start recovery. In case the user later wants to set the restore_command to empty - let's assume that's correct (FATAL if PITR target is after the end of local pg_wal, promote otherwise).

>>  Why not use local pg_wal? There may be already enough WAL.
>
> Mmm. If the file to read is in pg_wal, restore_command won't be
> executed in the first place?

Startup process will call restore_command in any case regardless of pg_wal content. (xlogarchive.c, RestoreArchivedFile)

> * When doing archive recovery, we always prefer an archived log file even
> * if a file of the same name exists in XLOGDIR.  The reason is that the
> * file in XLOGDIR could be an old, un-filled or partly-filled version
> * that was copied and restored as part of backing up $PGDATA.

regards, Sergei