Merge pull request #3423 from citusdata/remove-directory-even-if-new-files-added

CitusRemoveDirectory: loop when directory is not empty

src/backend/distributed/executor/intermediate_results.c

@@ -667,10 +667,7 @@ RemoveIntermediateResultsDirectory(void)
 {
 	if (CreatedResultsDirectory)
 	{
-		StringInfo resultsDirectory = makeStringInfo();
+		CitusRemoveDirectory(IntermediateResultsDirectory());
-		appendStringInfoString(resultsDirectory, IntermediateResultsDirectory());
-
-		CitusRemoveDirectory(resultsDirectory);
 
 		CreatedResultsDirectory = false;
 	}

src/backend/distributed/executor/multi_server_executor.c

@@ -187,7 +187,7 @@ void
 RemoveJobDirectory(uint64 jobId)
 {
 	StringInfo jobDirectoryName = MasterJobDirectoryName(jobId);
-	CitusRemoveDirectory(jobDirectoryName);
+	CitusRemoveDirectory(jobDirectoryName->data);
 
 	ResourceOwnerForgetJobDirectory(CurrentResourceOwner, jobId);
 }

src/backend/distributed/test/file_utils.c

@@ -23,7 +23,7 @@ citus_rm_job_directory(PG_FUNCTION_ARGS)
 	appendStringInfo(jobCacheDirectory, "base/%s/%s%0*" INT64_MODIFIER "u",
 					 PG_JOB_CACHE_DIR, JOB_DIRECTORY_PREFIX,
 					 MIN_JOB_DIRNAME_WIDTH, jobId);
-	CitusRemoveDirectory(jobCacheDirectory);
+	CitusRemoveDirectory(jobCacheDirectory->data);
 	FreeStringInfo(jobCacheDirectory);
 
 	PG_RETURN_VOID();

src/backend/distributed/worker/task_tracker.c

@@ -344,7 +344,7 @@ TrackerCleanupJobDirectories(void)
 	StringInfo jobCacheDirectory = makeStringInfo();
 	appendStringInfo(jobCacheDirectory, "base/%s", PG_JOB_CACHE_DIR);
 
-	CitusRemoveDirectory(jobCacheDirectory);
+	CitusRemoveDirectory(jobCacheDirectory->data);
 	CitusCreateDirectory(jobCacheDirectory);
 
 	FreeStringInfo(jobCacheDirectory);

src/backend/distributed/worker/task_tracker_protocol.c

@@ -231,7 +231,7 @@ task_tracker_cleanup_job(PG_FUNCTION_ARGS)
 	 * writing to a table within the schema.
 	 */
 	StringInfo jobDirectoryName = JobDirectoryName(jobId);
-	CitusRemoveDirectory(jobDirectoryName);
+	CitusRemoveDirectory(jobDirectoryName->data);
 
 	RemoveJobSchema(jobSchemaName);
 	UnlockJobResource(jobId, AccessExclusiveLock);

src/backend/distributed/worker/worker_merge_protocol.c

@@ -97,7 +97,7 @@ worker_repartition_cleanup(PG_FUNCTION_ARGS)
 	Oid schemaId = get_namespace_oid(jobSchemaName->data, false);
 
 	EnsureSchemaOwner(schemaId);
-	CitusRemoveDirectory(jobDirectoryName);
+	CitusRemoveDirectory(jobDirectoryName->data);
 	RemoveJobSchema(jobSchemaName);
 	PG_RETURN_VOID();
 }

src/backend/distributed/worker/worker_partition_protocol.c

@@ -85,7 +85,7 @@ static uint32 RangePartitionId(Datum partitionValue, Oid partitionCollation,
 static uint32 HashPartitionId(Datum partitionValue, Oid partitionCollation,
 							  const void *context);
 static StringInfo UserPartitionFilename(StringInfo directoryName, uint32 partitionId);
-static bool FileIsLink(char *filename, struct stat filestat);
+static bool FileIsLink(const char *filename, struct stat filestat);
 
 
 /* exports for SQL callable functions */
@@ -158,7 +158,7 @@ worker_range_partition_table(PG_FUNCTION_ARGS)
 
 	/* close partition files and atomically rename (commit) them */
 	ClosePartitionFiles(partitionFileArray, fileCount);
-	CitusRemoveDirectory(taskDirectory);
+	CitusRemoveDirectory(taskDirectory->data);
 	RenameDirectory(taskAttemptDirectory, taskDirectory);
 
 	PG_RETURN_VOID();
@@ -232,7 +232,7 @@ worker_hash_partition_table(PG_FUNCTION_ARGS)
 
 	/* close partition files and atomically rename (commit) them */
 	ClosePartitionFiles(partitionFileArray, fileCount);
-	CitusRemoveDirectory(taskDirectory);
+	CitusRemoveDirectory(taskDirectory->data);
 	RenameDirectory(taskAttemptDirectory, taskDirectory);
 
 	PG_RETURN_VOID();
@@ -698,7 +698,7 @@ FileIsLink(char *filename, struct stat filestat)
 
 #else
 static bool
-FileIsLink(char *filename, struct stat filestat)
+FileIsLink(const char *filename, struct stat filestat)
 {
 	return S_ISLNK(filestat.st_mode);
 }
@@ -714,12 +714,15 @@ FileIsLink(char *filename, struct stat filestat)
  * system library's remove_all() method.
  */
 void
-CitusRemoveDirectory(StringInfo filename)
+CitusRemoveDirectory(const char *filename)
+{
+	/* files may be added during execution, loop when that occurs */
+	while (true)
 	{
 		struct stat fileStat;
 		int removed = 0;
 
-	int fileStated = stat(filename->data, &fileStat);
+		int fileStated = stat(filename, &fileStat);
 		if (fileStated < 0)
 		{
 			if (errno == ENOENT)
@@ -729,7 +732,7 @@ CitusRemoveDirectory(StringInfo filename)
 			else
 			{
 				ereport(ERROR, (errcode_for_file_access(),
-							errmsg("could not stat file \"%s\": %m", filename->data)));
+								errmsg("could not stat file \"%s\": %m", filename)));
 			}
 		}
 
@@ -738,9 +741,9 @@ CitusRemoveDirectory(StringInfo filename)
 		 * content, recurse into this function. Also, make sure that we do not
 		 * recurse into symbolic links.
 		 */
-	if (S_ISDIR(fileStat.st_mode) && !FileIsLink(filename->data, fileStat))
+		if (S_ISDIR(fileStat.st_mode) && !FileIsLink(filename, fileStat))
 		{
-		const char *directoryName = filename->data;
+			const char *directoryName = filename;
 
 			DIR *directory = AllocateDir(directoryName);
 			if (directory == NULL)
@@ -750,8 +753,10 @@ CitusRemoveDirectory(StringInfo filename)
 									   directoryName)));
 			}
 
+			StringInfo fullFilename = makeStringInfo();
 			struct dirent *directoryEntry = ReadDir(directory, directoryName);
-		for (; directoryEntry != NULL; directoryEntry = ReadDir(directory, directoryName))
+			for (; directoryEntry != NULL; directoryEntry = ReadDir(directory,
+																	directoryName))
 			{
 				const char *baseFilename = directoryEntry->d_name;
 
@@ -762,31 +767,38 @@ CitusRemoveDirectory(StringInfo filename)
 					continue;
 				}
 
-			StringInfo fullFilename = makeStringInfo();
+				resetStringInfo(fullFilename);
 				appendStringInfo(fullFilename, "%s/%s", directoryName, baseFilename);
 
-			CitusRemoveDirectory(fullFilename);
+				CitusRemoveDirectory(fullFilename->data);
-
-			FreeStringInfo(fullFilename);
 			}
 
+			FreeStringInfo(fullFilename);
 			FreeDir(directory);
 		}
 
 		/* we now have an empty directory or a regular file, remove it */
 		if (S_ISDIR(fileStat.st_mode))
 		{
-		removed = rmdir(filename->data);
+			removed = rmdir(filename);
+
+			if (errno == ENOTEMPTY || errno == EEXIST)
+			{
+				continue;
+			}
 		}
 		else
 		{
-		removed = unlink(filename->data);
+			removed = unlink(filename);
 		}
 
 		if (removed != 0 && errno != ENOENT)
 		{
 			ereport(ERROR, (errcode_for_file_access(),
-						errmsg("could not remove file \"%s\": %m", filename->data)));
+							errmsg("could not remove file \"%s\": %m", filename)));
+		}
+
+		return;
 	}
 }
 

src/include/distributed/worker_protocol.h

@@ -117,7 +117,7 @@ extern bool CacheDirectoryElement(const char *filename);
 extern bool JobDirectoryElement(const char *filename);
 extern bool DirectoryExists(StringInfo directoryName);
 extern void CitusCreateDirectory(StringInfo directoryName);
-extern void CitusRemoveDirectory(StringInfo filename);
+extern void CitusRemoveDirectory(const char *filename);
 extern StringInfo InitTaskDirectory(uint64 jobId, uint32 taskId);
 extern void RemoveJobSchema(StringInfo schemaName);
 extern Datum * DeconstructArrayObject(ArrayType *arrayObject);