mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Remove redundant unlink() in KeepFileRestoredFromArchive() for non-WIN32
KeepFileRestoredFromArchive() called unlink() on an existing WAL segment before durable_rename() replaced it. rename(2) atomically replaces a target on POSIX systems, and the extra unlink() created a small window where it would be possible for a concurrent WAL sender to see a segment as missing. Note that the window still exists on WIN32, as we lack a safe concurrent alternative. Perhaps something like ReplaceFile() could be looked at for this purpose. This problem is hard to reach in practice, so no backpatch is done. I also tend to be conservative regarding recovery changes, even small, in stable branches. Author: Stepan Neretin <slpmcf@gmail.com> Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru> Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com> Discussion: https://postgr.es/m/CA+Yyo5S0C25jS_pRWQiGgoiy+X=GyJS7ZJkEq=_SW1WWpkL3Sg@mail.gmail.com
This commit is contained in:
@@ -366,9 +366,8 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname)
|
|||||||
|
|
||||||
if (stat(xlogfpath, &statbuf) == 0)
|
if (stat(xlogfpath, &statbuf) == 0)
|
||||||
{
|
{
|
||||||
char oldpath[MAXPGPATH];
|
|
||||||
|
|
||||||
#ifdef WIN32
|
#ifdef WIN32
|
||||||
|
char oldpath[MAXPGPATH];
|
||||||
static unsigned int deletedcounter = 1;
|
static unsigned int deletedcounter = 1;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@@ -380,6 +379,10 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname)
|
|||||||
* name first. Use a counter to create a unique filename, because the
|
* name first. Use a counter to create a unique filename, because the
|
||||||
* same file might be restored from the archive multiple times, and a
|
* same file might be restored from the archive multiple times, and a
|
||||||
* walsender could still be holding onto an old deleted version of it.
|
* walsender could still be holding onto an old deleted version of it.
|
||||||
|
*
|
||||||
|
* Note that this leaves a short window where the target name does not
|
||||||
|
* exist, during which a concurrent walsender opening the segment
|
||||||
|
* could fail.
|
||||||
*/
|
*/
|
||||||
snprintf(oldpath, MAXPGPATH, "%s.deleted%u",
|
snprintf(oldpath, MAXPGPATH, "%s.deleted%u",
|
||||||
xlogfpath, deletedcounter++);
|
xlogfpath, deletedcounter++);
|
||||||
@@ -390,15 +393,13 @@ KeepFileRestoredFromArchive(const char *path, const char *xlogfname)
|
|||||||
errmsg("could not rename file \"%s\" to \"%s\": %m",
|
errmsg("could not rename file \"%s\" to \"%s\": %m",
|
||||||
xlogfpath, oldpath)));
|
xlogfpath, oldpath)));
|
||||||
}
|
}
|
||||||
#else
|
|
||||||
/* same-size buffers, so this never truncates */
|
|
||||||
strlcpy(oldpath, xlogfpath, MAXPGPATH);
|
|
||||||
#endif
|
|
||||||
if (unlink(oldpath) != 0)
|
if (unlink(oldpath) != 0)
|
||||||
ereport(FATAL,
|
ereport(FATAL,
|
||||||
(errcode_for_file_access(),
|
(errcode_for_file_access(),
|
||||||
errmsg("could not remove file \"%s\": %m",
|
errmsg("could not remove file \"%s\": %m",
|
||||||
xlogfpath)));
|
xlogfpath)));
|
||||||
|
#endif
|
||||||
reload = true;
|
reload = true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user