8000 Log the creation of an init fork unconditionally. · phan-pivotal/postgres@a00ac62 · GitHub
[go: up one dir, main page]
Skip to content

Commit a00ac62

Browse files
robertmhaasrobertmhaas
committed
Log the creation of an init fork unconditionally.
Previously, it was thought that this only needed to be done for the benefit of possible standbys, so wal_level = minimal skipped it. But that's not safe, because during crash recovery we might replay XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record which recursively removes the directory that contains the new init fork. So log it always. The user-visible effect of this bug is that if you create a database or tablespace, then create an unlogged table, then crash without checkpointing, then restart, accessing the table will fail, because the it won't have been properly reset. This commit fixes that. Michael Paquier, per a report from Konstantin Knizhnik. Wording of the comments per a suggestion from me.
1 parent 311bc14 commit a00ac62

File tree

3 files changed

+29
-20
lines changed

3 files changed

+29
-20
lines changed

‎src/backend/access/nbtree/nbtree.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -215,12 +215,17 @@ btbuildempty(PG_FUNCTION_ARGS)
215215
metapage = (Page) palloc(BLCKSZ);
216216
_bt_initmetapage(metapage, P_NONE, 0);
217217

218-
/* Write the page. If archiving/streaming, XLOG it. */
218+
/*
219+
* Write the page and log it. It might seem that an immediate sync
220+
* would be sufficient to guarantee that the file exists on disk, but
221+
* recovery itself might remove it while replaying, for example, an
222+
* XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE record. Therefore, we
223+
* need this even when wal_level=minimal.
224+
*/
219225
smgrwrite(index->rd_smgr, INIT_FORKNUM, BTREE_METAPAGE,
220226
(char *) metapage, true);
221-
if (XLogIsNeeded())
222-
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
223-
BTREE_METAPAGE, metapage);
227+
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
228+
BTREE_METAPAGE, metapage);
224229

225230
/*
226231
* An immediate sync is required even if we xlog'd the page, because the

‎src/backend/access/spgist/spginsert.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -164,30 +164,33 @@ spgbuildempty(PG_FUNCTION_ARGS)
164164
page = (Page) palloc(BLCKSZ);
165165
SpGistInitMetapage(page);
166166

167-
/* Write the page. If archiving/streaming, XLOG it. */
167+
/*
168+
* Write the page and log it unconditionally. This is important
169+
* particularly for indexes created on tablespaces and databases
170+
* whose creation happened after the last redo pointer as recovery
171+
* removes any of their existing content when the corresponding
172+
* create records are replayed.
173+
*/
168174
smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_METAPAGE_BLKNO,
169175
(char *) page, true);
170-
if (XLogIsNeeded())
171-
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
172-
SPGIST_METAPAGE_BLKNO, page);
176+
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
177+
SPGIST_METAPAGE_BLKNO, page);
173178

174179
/* Likewise for the root page. */
175180
SpGistInitPage(page, SPGIST_LEAF);
176181

177182
smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_ROOT_BLKNO,
178183
(char *) page, true);
179-
if (XLogIsNeeded())
180-
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
181-
SPGIST_ROOT_BLKNO, page);
184+
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
185+
SPGIST_ROOT_BLKNO, page);
182186

183187
/* Likewise for the null-tuples root page. */
184188
SpGistInitPage(page, SPGIST_LEAF | SPGIST_NULLS);
185189

186190
smgrwrite(index->rd_smgr, INIT_FORKNUM, SPGIST_NULL_BLKNO,
187191
(char *) page, true);
188-
if (XLogIsNeeded())
189-
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
190-
SPGIST_NULL_BLKNO, page);
192+
log_newpage(&index->rd_smgr->smgr_rnode.node, INIT_FORKNUM,
193+
SPGIST_NULL_BLKNO, page);
191194

192195
/*
193196
* An immediate sync is required even if we xlog'd the pages, because the

‎src/backend/catalog/heap.c

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,18 +1325,19 @@ heap_create_with_catalog(const char *relname,
13251325

13261326
/*
13271327
* Set up an init fork for an unlogged table so that it can be correctly
1328-
* reinitialized on restart. Since we're going to do an immediate sync, we
1329-
* only need to xlog this if archiving or streaming is enabled. And the
1330-
* immediate sync is required, because otherwise there's no guarantee that
1331-
* this will hit the disk before the next checkpoint moves the redo pointer.
1328+
* reinitialized on restart. An immediate sync is required even if the
1329+
* page has been logged, because the write did not go through
1330+
* shared_buffers and therefore a concurrent checkpoint may have moved
1331+
* the redo pointer past our xlog record. Recovery may as well remove it
1332+
* while replaying, for example, XLOG_DBASE_CREATE or XLOG_TBLSPC_CREATE
1333+
* record. Therefore, logging is necessary even if wal_level=minimal.
13321334
*/
13331335
void
13341336
heap_create_init_fork(Relation rel)
13351337
{
13361338
RelationOpenSmgr(rel);
13371339
smgrcreate(rel->rd_smgr, INIT_FORKNUM, false);
1338-
if (XLogIsNeeded())
1339-
log_smgrcreate(&rel->rd_smgr->smgr_rnode.node, INIT_FORKNUM);
1340+
log_smgrcreate(&rel->rd_smgr->smgr_rnode.node, INIT_FORKNUM);
13401341
smgrimmedsync(rel->rd_smgr, INIT_FORKNUM);
13411342
}
13421343

0 commit comments

Comments
 (0)
0