From a98226842d218720094459b41a9ac774cd85ecea Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Tue, 23 Jun 2020 10:38:44 +0200 Subject: [PATCH] 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. --- .../executor/intermediate_results.c | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/src/backend/distributed/executor/intermediate_results.c b/src/backend/distributed/executor/intermediate_results.c index 3c299da53..90634c62a 100644 --- a/src/backend/distributed/executor/intermediate_results.c +++ b/src/backend/distributed/executor/intermediate_results.c @@ -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; }