mirror of https://github.com/citusdata/citus.git
Use rename to make sure no files are inserted while deleting (#3912)
As suggested by @marcocitus in https://github.com/citusdata/citus/pull/3911#issuecomment-643978531, there was a regression in #3893. If another backend would write a file during deletion of the intermediate results directory, this file would not necessarily be deleted. The approach used in `CitusRemoveDirectory` is to try recursive removal of the directory again if it has failed. This does not work here, since when a file can not be removed for other reasons (e.g. `EPERM`) it will not throw an error anymore. So then we would get into an infinite removal loop. Instead I now `rename` the directory before removing it. That way other backends will not write files to it anymore.pull/3872/head^2
parent
0e0695481c
commit
a98226842d
|
@ -701,7 +701,33 @@ RemoveIntermediateResultsDirectory(void)
|
|||
{
|
||||
if (CreatedResultsDirectory)
|
||||
{
|
||||
PathNameDeleteTemporaryDir(IntermediateResultsDirectory());
|
||||
/*
|
||||
* The shared directory is renamed before deleting it. Otherwise it
|
||||
* would be possible for another backend to write a file, while we are
|
||||
* deleting the directory. Since rename is atomic by POSIX standards
|
||||
* that's not possible. The current PID is included in the new
|
||||
* filename, so there can be no collisions with other backends.
|
||||
*/
|
||||
char *sharedName = IntermediateResultsDirectory();
|
||||
StringInfo privateName = makeStringInfo();
|
||||
appendStringInfo(privateName, "%s.removed-by-%d", sharedName, MyProcPid);
|
||||
if (rename(sharedName, privateName->data))
|
||||
{
|
||||
ereport(LOG,
|
||||
(errcode_for_file_access(),
|
||||
errmsg(
|
||||
"could not rename intermediate results directory \"%s\" to \"%s\": %m",
|
||||
sharedName, privateName->data)));
|
||||
|
||||
/* rename failed for some reason, we do a best effort removal of
|
||||
* the shared directory */
|
||||
|
||||
PathNameDeleteTemporaryDir(sharedName);
|
||||
}
|
||||
else
|
||||
{
|
||||
PathNameDeleteTemporaryDir(privateName->data);
|
||||
}
|
||||
|
||||
CreatedResultsDirectory = false;
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue