Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit61dd2ff

Browse files
committed
Avoid crashing when we have problems unlinking files post-commit.
smgrdounlink takes care to not throw an ERROR if it fails to unlinksomething, but that caution was rendered useless by commit3396000, which put an smgrexists call infront of it; smgrexists *does* throw error if anything looks funny, suchas getting a permissions error from trying to open the file. If thathappens post-commit, you get a PANIC, and what's worse the same logicappears in the WAL replay code, so the database even fails to restart.Restore the intended behavior by removing the smgrexists call --- it isn'taccomplishing anything that we can't do better by adjusting mdunlink'sideas of whether it ought to warn about ENOENT or not.Per report from Joseph Shraibman of unrecoverable crash after trying todrop a table whose FSM fork had somehow gotten chmod'd to 000 permissions.Backpatch to 8.4, where the bogus coding was introduced.
1 parent458a83a commit61dd2ff

File tree

4 files changed

+23
-29
lines changed

4 files changed

+23
-29
lines changed

‎src/backend/access/transam/twophase.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,8 +1325,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
13251325

13261326
for (fork=0;fork <=MAX_FORKNUM;fork++)
13271327
{
1328-
if (smgrexists(srel,fork))
1329-
smgrdounlink(srel,fork, false, false);
1328+
smgrdounlink(srel,fork, false, false);
13301329
}
13311330
smgrclose(srel);
13321331
}

‎src/backend/access/transam/xact.c

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4479,11 +4479,8 @@ xact_redo_commit(xl_xact_commit *xlrec, TransactionId xid, XLogRecPtr lsn)
44794479

44804480
for (fork=0;fork <=MAX_FORKNUM;fork++)
44814481
{
4482-
if (smgrexists(srel,fork))
4483-
{
4484-
XLogDropRelation(xlrec->xnodes[i],fork);
4485-
smgrdounlink(srel,fork, false, true);
4486-
}
4482+
XLogDropRelation(xlrec->xnodes[i],fork);
4483+
smgrdounlink(srel,fork, false, true);
44874484
}
44884485
smgrclose(srel);
44894486
}
@@ -4584,11 +4581,8 @@ xact_redo_abort(xl_xact_abort *xlrec, TransactionId xid)
45844581

45854582
for (fork=0;fork <=MAX_FORKNUM;fork++)
45864583
{
4587-
if (smgrexists(srel,fork))
4588-
{
4589-
XLogDropRelation(xlrec->xnodes[i],fork);
4590-
smgrdounlink(srel,fork, false, true);
4591-
}
4584+
XLogDropRelation(xlrec->xnodes[i],fork);
4585+
smgrdounlink(srel,fork, false, true);
45924586
}
45934587
smgrclose(srel);
45944588
}

‎src/backend/catalog/storage.c

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -325,11 +325,10 @@ smgrDoPendingDeletes(bool isCommit)
325325
srel=smgropen(pending->relnode);
326326
for (i=0;i <=MAX_FORKNUM;i++)
327327
{
328-
if (smgrexists(srel,i))
329-
smgrdounlink(srel,
330-
i,
331-
pending->isTemp,
332-
false);
328+
smgrdounlink(srel,
329+
i,
330+
pending->isTemp,
331+
false);
333332
}
334333
smgrclose(srel);
335334
}

‎src/backend/storage/smgr/md.c

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,13 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
312312
* number until it's safe, because relfilenode assignment skips over any
313313
* existing file.
314314
*
315-
* If isRedo is true, it's okay for the relation to be already gone.
315+
* All the above applies only to the relation's main fork; other forks can
316+
* just be removed immediately, since they are not needed to prevent the
317+
* relfilenode number from being recycled. Also, we do not carefully
318+
* track whether other forks have been created or not, but just attempt to
319+
* unlink them unconditionally; so we should never complain about ENOENT.
320+
*
321+
* If isRedo is true, it's unsurprising for the relation to be already gone.
316322
* Also, we should remove the file immediately instead of queuing a request
317323
* for later, since during redo there's no possibility of creating a
318324
* conflicting relation.
@@ -340,13 +346,10 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
340346
if (isRedo||forkNum!=MAIN_FORKNUM)
341347
{
342348
ret=unlink(path);
343-
if (ret<0)
344-
{
345-
if (!isRedo||errno!=ENOENT)
346-
ereport(WARNING,
347-
(errcode_for_file_access(),
348-
errmsg("could not remove file \"%s\": %m",path)));
349-
}
349+
if (ret<0&&errno!=ENOENT)
350+
ereport(WARNING,
351+
(errcode_for_file_access(),
352+
errmsg("could not remove file \"%s\": %m",path)));
350353
}
351354
else
352355
{
@@ -369,6 +372,9 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
369372
ereport(WARNING,
370373
(errcode_for_file_access(),
371374
errmsg("could not truncate file \"%s\": %m",path)));
375+
376+
/* Register request to unlink first segment later */
377+
register_unlink(rnode);
372378
}
373379

374380
/*
@@ -400,10 +406,6 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
400406
}
401407

402408
pfree(path);
403-
404-
/* Register request to unlink first segment later */
405-
if (!isRedo&&forkNum==MAIN_FORKNUM)
406-
register_unlink(rnode);
407409
}
408410

409411
/*

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp