Horton

Re: [Proposal] Fully WAL logged CREATE DATABASE - No Checkpoints

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Justin Pryzby <pryzby@telsasoft.com>, Ashutosh Sharma <ashu.coek88@gmail.com>, Maciek Sakrejda <m.sakrejda@gmail.com>, Bruce Momjian <bruce@momjian.us>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Andrew Dunstan <andrew@dunslane.net>, Heikki Linnakangas <hlinnaka@iki.fi>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Thomas Munro <thomas.munro@gmail.com>
Date: 2022-08-04T23:12:04Z
Lists: pgsql-hackers 
Hi,

On 2022-08-04 19:01:06 -0400, Tom Lane wrote:
> And while I'm piling on, how is this bit in RelationCopyStorageUsingBuffer
> not completely broken?
> 
>         /* Read block from source relation. */
>         srcBuf = ReadBufferWithoutRelcache(src->rd_locator, forkNum, blkno,
>                                            RBM_NORMAL, bstrategy_src,
>                                            permanent);
>         srcPage = BufferGetPage(srcBuf);
>         if (PageIsNew(srcPage) || PageIsEmpty(srcPage))
>         {
>             ReleaseBuffer(srcBuf);
>             continue;
>         }
> 
>         /* Use P_NEW to extend the destination relation. */
>         dstBuf = ReadBufferWithoutRelcache(dst->rd_locator, forkNum, P_NEW,
>                                            RBM_NORMAL, bstrategy_dst,
>                                            permanent);
> 
> You can't skip pages just because they are empty.  Well, maybe you could
> if you were doing something to ensure that you zero-fill the corresponding
> blocks on the destination side.  But this isn't doing that.  It's using
> P_NEW for dstBuf, which will have the effect of silently collapsing out
> such pages.  Maybe in isolation a heap could withstand that, but its
> indexes won't be happy (and I guess t_ctid chain links won't either).
> 
> I think you should just lose the if() stanza.  There's no optimization to
> be had here that's worth any extra complication.
> 
> (This seems worth fixing before beta3, as it looks like a rather
> nasty data corruption hazard.)

Ugh, yes. And even with this fixed I think this should grow at least an
assertion that the block numbers match, probably even an elog.

Greetings,

Andres

Commits

  1. When using the WAL-logged CREATE DATABASE strategy, bulk extend.

  2. Avoid using a fake relcache entry to own an SmgrRelation.

  3. Fix data-corruption hazard in WAL-logged CREATE DATABASE.

  4. initdb: When running CREATE DATABASE, use STRATEGY = WAL_COPY.

  5. Simplify a needlessly-complicated regular expression.

  6. In 020_createdb.pl, change order of command-line arguments.

  7. Add new block-by-block strategy for CREATE DATABASE.

  8. Fix replay of create database records on standby

  9. Refactor code for reading and writing relation map files.

  10. Replace RelationOpenSmgr() with RelationGetSmgr().

  11. Refactor the fsync queue for wider use.