10.09.2019 14:42, Asim R P wrote:
Hi Anastasia
On Thu, Aug 22, 2019 at 9:43 PM Anastasia Lubennikova
<a.lubennik...@postgrespro.ru <mailto:a.lubennik...@postgrespro.ru>>
wrote:
>
> But during the review, I found a bug in the current implementation.
> New behavior must apply to crash-recovery only, now it applies to
archiveRecovery too.
> That can cause a silent loss of a tablespace during regular standby
operation
> since it never calls CheckRecoveryConsistency().
>
> Steps to reproduce:
> 1) run master and replica
> 2) create dir for tablespace:
> mkdir /tmp/tblspc1
>
> 3) create tablespace and database on the master:
> create tablespace tblspc1 location '/tmp/tblspc1';
> create database db1 tablespace tblspc1 ;
>
> 4) wait for replica to receive this changes and pause replication:
> select pg_wal_replay_pause();
>
> 5) move replica's tablespace symlink to some empty directory, i.e.
/tmp/tblspc2
> mkdir /tmp/tblspc2
> ln -sfn /tmp/tblspc2 postgresql_data_replica/pg_tblspc/16384
>
By changing the tablespace symlink target, we are silently nullifying
effects of a committed transaction from the standby data directory -
the directory structure created by the standby for create tablespace
transaction. This step, therefore, does not look like a valid test
case to me. Can you share a sequence of steps that does not involve
changing data directory manually?
Hi, the whole idea of the test is to reproduce a data loss. For example,
if the disk containing this tablespace failed.
Probably, simply deleting the directory
'postgresql_data_replica/pg_tblspc/16384'
would work as well, though I was afraid that it can be caught by some
earlier checks and my example won't be so illustrative.
Thank you for the review feedback. I agree with all the points. Let
me incorporate them (I plan to pick this work up and drive it to
completion as Paul got busy with other things).
But before that I'm revisiting another solution upthread, that of
creating restart points when replaying create/drop database commands
before making filesystem changes such as removing a directory. The
restart points should align with checkpoints on master. The concern
against this solution was creation of restart points will slow down
recovery. I don't think crash recovery is affected by this solution
because of the already existing enforcement of checkpoints. WAL
records prior to a create/drop database will not be seen by crash
recovery due to the checkpoint enforced during the command's normal
execution.
I haven't measured the impact of generating extra restart points in
previous solution, so I cannot tell whether concerns upthread are
justified. Still, I enjoy latest design more, since it is clear and
similar with the code of checking unexpected uninitialized pages. In
principle it works. And the issue, I described in previous review can be
easily fixed by several additional checks of InHotStandby macro.
--
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
