Re: standby recovery fails (tablespace related) (tentative patch and discussion)

Anastasia Lubennikova <a.lubennikova@postgrespro.ru>

From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Paul Guo <pguo@pivotal.io>, Thomas Munro <thomas.munro@gmail.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Date: 2019-08-22T16:13:05Z
Lists: pgsql-hackers
22.08.2019 16:13, Paul Guo wrote:
> Thanks. I updated the patch to v5. It passes install-check testing and 
> recovery testing.
Hi,
Thank you for working on this fix.
The overall design of the latest version looks good to me.
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

6) create another database in tblspc1 on master:
create database db2 tablespace tblspc1 ;

7) resume replication on standby:
select pg_wal_replay_resume();

8) try to connect to db2 on standby

It's expected that dbase_redo() will fail because the directory on 
standby is not found.
While with the patch it suppresses the error until we attempt to connect 
db2 on the standby:

2019-08-22 18:34:39.178 MSK [21066] HINT:  Execute 
pg_wal_replay_resume() to continue.
2019-08-22 18:42:41.656 MSK [21066] WARNING:  Skip creating database 
directory "pg_tblspc/16384/PG_13_201908012". The dest tablespace may 
have been removed before abnormal shutdown. If the removal is illegal 
after later checking we will panic.
2019-08-22 18:42:41.656 MSK [21066] CONTEXT:  WAL redo at 0/3027738 for 
Database/CREATE: copy dir base/1 to pg_tblspc/16384/PG_13_201908012/16390
2019-08-22 18:42:46.096 MSK [21688] FATAL: 
"pg_tblspc/16384/PG_13_201908012/16390" is not a valid data directory
2019-08-22 18:42:46.096 MSK [21688] DETAIL:  File 
"pg_tblspc/16384/PG_13_201908012/16390/PG_VERSION" is missing.

Also some nitpicking about code style:
1) Please, add comment to forget_missing_directory().

2) +               elog(LOG, "Directory \"%s\" was missing during 
directory copying "
I think we'd better update this message elevel to WARNING.

3) Shouldn't we also move FlushDatabaseBuffers(xlrec->src_db_id); call under
     if (do_copydir) clause?
I don't see a reason to flush pages that we won't use later.

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Commits

  1. Improve recently-added test reliability

  2. Fix new recovery test for log_error_verbosity=verbose case

  3. Fix test instability

  4. Fix replay of create database records on standby

  5. Allow "in place" tablespaces.

  6. Fix get_dirent_type() for Windows junction points.

  7. Revert "Fix replay of create database records on standby"

  8. Add end-to-end testing of pg_basebackup's tar-format output.

  9. Make DROP DATABASE command generate less WAL records.

  10. Consolidate methods for translating a Perl path to a Windows path.