fix clean directory and make them use effective threading where appropriate (#625)v5.7.8-pre5v5.7.8-pre3v5.7.8-pre2

fix retry logic so it does not immediately sleep if file does not exist
make sure we don't try to delete target folder files if we have already wiped it

5 files changed, 76 insertions, 92 deletions

diff --git a/src/zen/cmds/builds_cmd.cpp b/src/zen/cmds/builds_cmd.cpp
index c8402caf6..15c9774b4 100644
--- a/src/zen/cmds/builds_cmd.cpp
+++ b/src/zen/cmds/builds_cmd.cpp
@@ -33,6 +33,7 @@
 #include <zenremotestore/chunking/chunkedcontent.h>
 #include <zenremotestore/chunking/chunkedfile.h>
 #include <zenremotestore/chunking/chunkingcontroller.h>
+#include <zenremotestore/filesystemutils.h>
 #include <zenremotestore/jupiter/jupiterhost.h>
 #include <zenutil/wildcard.h>
 #include <zenutil/workerpools.h>
@@ -564,6 +565,11 @@ namespace {
 		return Count * 1000000 / ElapsedWallTimeUS;
 	}
 
+	bool CleanAndRemoveDirectory(const std::filesystem::path& Directory)
+	{
+		return CleanAndRemoveDirectory(GetIOWorkerPool(), AbortFlag, PauseFlag, Directory);
+	}
+
 	void ValidateBuildPart(BuildStorageBase& Storage, const Oid& BuildId, Oid BuildPartId, const std::string_view BuildPartName)
 	{
 		ZEN_TRACE_CPU("ValidateBuildPart");
@@ -2337,11 +2343,7 @@ namespace {
 
 		ProgressBar::SetLogOperationProgress(ProgressMode, TaskSteps::Cleanup, TaskSteps::StepCount);
 
-		if (CleanDirectory(ZenTempFolder, {}))
-		{
-			std::error_code DummyEc;
-			RemoveDir(ZenTempFolder, DummyEc);
-		}
+		CleanAndRemoveDirectory(ZenTempFolder);
 	}
 
 	void ListBuild(StorageInstance&				Storage,
@@ -3628,13 +3630,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 														? MakeSafeAbsolutePath(std::filesystem::current_path()) / ZenFolderName
 														: MakeSafeAbsolutePath(m_ZenFolderPath);
 		CreateDirectories(ZenFolderPath);
-		auto _ = MakeGuard([ZenFolderPath]() {
-			if (CleanDirectory(ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([ZenFolderPath]() { CleanAndRemoveDirectory(ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -3719,13 +3715,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -3786,13 +3776,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -3859,13 +3843,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -4122,13 +4100,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -4207,13 +4179,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -4264,13 +4230,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		MakeSafeAbsolutePathÍnPlace(m_ZenFolderPath);
 
 		CreateDirectories(m_ZenFolderPath);
-		auto _ = MakeGuard([this]() {
-			if (CleanDirectory(m_ZenFolderPath, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_ZenFolderPath, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([this]() { CleanAndRemoveDirectory(m_ZenFolderPath); });
 
 		StorageInstance Storage = CreateBuildStorage(StorageStats,
 													 StorageCacheStats,
@@ -4304,7 +4264,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 		m_SystemRootDir = (GetRunningExecutablePath().parent_path() / ".tmpzensystem").make_preferred();
 		CreateDirectories(m_SystemRootDir);
-		CleanDirectory(m_SystemRootDir, {});
+		CleanDirectory(m_SystemRootDir, /*ForceRemoveReadOnlyFiles*/ true);
 		auto _ = MakeGuard([&]() { DeleteDirectories(m_SystemRootDir); });
 
 		ParsePath();
@@ -4411,7 +4371,7 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 
 		m_SystemRootDir = (GetRunningExecutablePath().parent_path() / ".tmpzensystem").make_preferred();
 		CreateDirectories(m_SystemRootDir);
-		CleanDirectory(m_SystemRootDir, {});
+		CleanDirectory(m_SystemRootDir, /*ForceRemoveReadOnlyFiles*/ true);
 		auto _ = MakeGuard([&]() { DeleteDirectories(m_SystemRootDir); });
 
 		ParsePath();
@@ -4419,8 +4379,8 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		if (m_OverrideHost.empty() && m_StoragePath.empty())
 		{
 			m_StoragePath = (GetRunningExecutablePath().parent_path() / ".tmpstore").make_preferred();
+			CleanAndRemoveDirectory(m_StoragePath);
 			CreateDirectories(m_StoragePath);
-			CleanDirectory(m_StoragePath, {});
 			m_StoragePath = m_StoragePath.generic_string();
 		}
 
@@ -4441,12 +4401,9 @@ BuildsCommand::Run(const ZenCliOptions& GlobalOptions, int argc, char** argv)
 		const std::filesystem::path DownloadPath3 = m_Path.parent_path() / (m_BuildPartName + "_test3");
 
 		auto ___ = MakeGuard([DownloadPath, DownloadPath2, DownloadPath3]() {
-			CleanDirectory(DownloadPath, true);
-			DeleteDirectories(DownloadPath);
-			CleanDirectory(DownloadPath2, true);
-			DeleteDirectories(DownloadPath2);
-			CleanDirectory(DownloadPath3, true);
-			DeleteDirectories(DownloadPath3);
+			CleanAndRemoveDirectory(DownloadPath);
+			CleanAndRemoveDirectory(DownloadPath2);
+			CleanAndRemoveDirectory(DownloadPath3);
 		});
 
 		if (m_ZenFolderPath.empty())
diff --git a/src/zenremotestore/builds/buildstorageoperations.cpp b/src/zenremotestore/builds/buildstorageoperations.cpp
index ecf5853b8..5c1b28695 100644
--- a/src/zenremotestore/builds/buildstorageoperations.cpp
+++ b/src/zenremotestore/builds/buildstorageoperations.cpp
@@ -7,6 +7,7 @@
 #include <zenremotestore/builds/buildstoragecache.h>
 #include <zenremotestore/chunking/chunkblock.h>
 #include <zenremotestore/chunking/chunkingcontroller.h>
+#include <zenremotestore/filesystemutils.h>
 
 #include <zencore/basicfile.h>
 #include <zencore/compactbinary.h>
@@ -19,7 +20,6 @@
 #include <zencore/string.h>
 #include <zencore/timer.h>
 #include <zencore/trace.h>
-#include "../filesystemutils.h"
 
 #include <numeric>
 
@@ -2124,7 +2124,7 @@ BuildsOperationUpdateFolder::Execute(FolderContent& OutLocalFolderState)
 						DeleteCount++;
 					}
 				}
-				else
+				else if (!m_Options.WipeTargetFolder)
 				{
 					// Delete local file as we did not scavenge the folder
 					RemoveLocalPathIndexes.push_back(LocalPathIndex);
@@ -4650,15 +4650,9 @@ BuildsOperationUploadFolder::Execute()
 
 		Stopwatch ProcessTimer;
 
+		CleanAndRemoveDirectory(m_IOWorkerPool, m_AbortFlag, m_PauseFlag, m_Options.TempDir);
 		CreateDirectories(m_Options.TempDir);
-		CleanDirectory(m_Options.TempDir, {});
-		auto _ = MakeGuard([&]() {
-			if (CleanDirectory(m_Options.TempDir, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(m_Options.TempDir, DummyEc);
-			}
-		});
+		auto _ = MakeGuard([&]() { CleanAndRemoveDirectory(m_IOWorkerPool, m_AbortFlag, m_PauseFlag, m_Options.TempDir); });
 
 		m_LogOutput.SetLogOperationProgress(TaskSteps::PrepareBuild, TaskSteps::StepCount);
 
@@ -6915,14 +6909,9 @@ BuildsOperationValidateBuildPart::Execute()
 
 		const std::filesystem::path TempFolder = ".zen-tmp";
 
+		CleanAndRemoveDirectory(m_IOWorkerPool, m_AbortFlag, m_PauseFlag, TempFolder);
 		CreateDirectories(TempFolder);
-		auto __ = MakeGuard([&TempFolder]() {
-			if (CleanDirectory(TempFolder, {}))
-			{
-				std::error_code DummyEc;
-				RemoveDir(TempFolder, DummyEc);
-			}
-		});
+		auto __ = MakeGuard([this, TempFolder]() { CleanAndRemoveDirectory(m_IOWorkerPool, m_AbortFlag, m_PauseFlag, TempFolder); });
 
 		m_LogOutput.SetLogOperationProgress(TaskSteps::ValidateBlobs, TaskSteps::StepCount);
 
diff --git a/src/zenremotestore/filesystemutils.cpp b/src/zenremotestore/filesystemutils.cpp
index 20ab3faea..8dff05c6b 100644
--- a/src/zenremotestore/filesystemutils.cpp
+++ b/src/zenremotestore/filesystemutils.cpp
@@ -1,6 +1,6 @@
 // Copyright Epic Games, Inc. All Rights Reserved.
 
-#include "filesystemutils.h"
+#include <zenremotestore/filesystemutils.h>
 
 #include <zenremotestore/chunking/chunkedcontent.h>
 
@@ -215,11 +215,11 @@ SetFileReadOnlyWithRetry(const std::filesystem::path& Path, bool ReadOnly)
 	bool			Result = SetFileReadOnly(Path, ReadOnly, Ec);
 	for (size_t Retries = 0; Ec && Retries < 3; Retries++)
 	{
-		Sleep(100 + int(Retries * 50));
 		if (!IsFileWithRetry(Path))
 		{
 			return false;
 		}
+		Sleep(100 + int(Retries * 50));
 		Ec.clear();
 		Result = SetFileReadOnly(Path, ReadOnly, Ec);
 	}
@@ -277,11 +277,11 @@ RemoveFileWithRetry(const std::filesystem::path& Path)
 	RemoveFile(Path, Ec);
 	for (size_t Retries = 0; Ec && Retries < 6; Retries++)
 	{
-		Sleep(100 + int(Retries * 50));
 		if (!IsFileWithRetry(Path))
 		{
 			return;
 		}
+		Sleep(100 + int(Retries * 50));
 		Ec.clear();
 		RemoveFile(Path, Ec);
 	}
@@ -426,12 +426,13 @@ CleanDirectory(
 							(void)SetFileReadOnly(FilePath, false, Ec);
 							for (size_t Retries = 0; Ec && Retries < 3; Retries++)
 							{
-								Sleep(100 + int(Retries * 50));
 								if (!IsFileWithRetry(FilePath))
 								{
 									IsRemoved = true;
+									Ec.clear();
 									break;
 								}
+								Sleep(100 + int(Retries * 50));
 								Ec.clear();
 								(void)SetFileReadOnly(FilePath, false, Ec);
 							}
@@ -440,12 +441,13 @@ CleanDirectory(
 								(void)RemoveFile(FilePath, Ec);
 								for (size_t Retries = 0; Ec && Retries < 6; Retries++)
 								{
-									Sleep(100 + int(Retries * 50));
 									if (!IsFileWithRetry(FilePath))
 									{
 										IsRemoved = true;
+										Ec.clear();
 										return;
 									}
+									Sleep(100 + int(Retries * 50));
 									Ec.clear();
 									(void)RemoveFile(FilePath, Ec);
 								}
@@ -542,25 +544,27 @@ CleanDirectory(
 
 			{
 				std::error_code Ec;
-				zen::CleanDirectory(DirectoryToDelete, true, Ec);
+				zen::CleanDirectory(DirectoryToDelete, /*ForceRemoveReadOnlyFiles*/ true, Ec);
 				if (Ec)
 				{
 					Sleep(200);
-					zen::CleanDirectory(DirectoryToDelete, true, Ec);
+					Ec.clear();
+					zen::CleanDirectory(DirectoryToDelete, /*ForceRemoveReadOnlyFiles*/ true, Ec);
 				}
 
 				if (!Ec)
 				{
-					RemoveDir(Path, Ec);
+					RemoveDir(DirectoryToDelete, Ec);
 					for (size_t Retries = 0; Ec && Retries < 3; Retries++)
 					{
-						Sleep(100 + int(Retries * 50));
-						if (!IsDir(Path))
+						if (!IsDir(DirectoryToDelete))
 						{
+							Ec.clear();
 							break;
 						}
+						Sleep(100 + int(Retries * 50));
 						Ec.clear();
-						RemoveDir(Path, Ec);
+						RemoveDir(DirectoryToDelete, Ec);
 					}
 				}
 				if (Ec)
@@ -592,4 +596,33 @@ CleanDirectory(
 	return Result;
 }
 
+bool
+CleanAndRemoveDirectory(WorkerThreadPool&			 WorkerPool,
+						std::atomic<bool>&			 AbortFlag,
+						std::atomic<bool>&			 PauseFlag,
+						const std::filesystem::path& Directory)
+{
+	if (!IsDir(Directory))
+	{
+		return true;
+	}
+	if (CleanDirectoryResult Res = CleanDirectory(
+			WorkerPool,
+			AbortFlag,
+			PauseFlag,
+			Directory,
+			{},
+			[](const std::string_view Details, uint64_t TotalCount, uint64_t RemainingCount, bool IsPaused, bool IsAborted) {
+				ZEN_UNUSED(Details, TotalCount, RemainingCount, IsPaused, IsAborted);
+			},
+			1000);
+		Res.FailedRemovePaths.empty())
+	{
+		std::error_code Ec;
+		RemoveDir(Directory, Ec);
+		return !Ec;
+	}
+	return false;
+}
+
 }  // namespace zen
diff --git a/src/zenremotestore/filesystemutils.h b/src/zenremotestore/include/zenremotestore/filesystemutils.h
index cfe6adc6c..a6c88e5cb 100644
--- a/src/zenremotestore/filesystemutils.h
+++ b/src/zenremotestore/include/zenremotestore/filesystemutils.h
@@ -111,4 +111,9 @@ CleanDirectoryResult CleanDirectory(
 			 ProgressFunc,
 	uint32_t ProgressUpdateDelayMS);
 
+bool CleanAndRemoveDirectory(WorkerThreadPool&			  WorkerPool,
+							 std::atomic<bool>&			  AbortFlag,
+							 std::atomic<bool>&			  PauseFlag,
+							 const std::filesystem::path& Directory);
+
 }  // namespace zen
diff --git a/src/zenstore/buildstore/buildstore.cpp b/src/zenstore/buildstore/buildstore.cpp
index 99cf7f16e..04a0781d3 100644
--- a/src/zenstore/buildstore/buildstore.cpp
+++ b/src/zenstore/buildstore/buildstore.cpp
@@ -171,7 +171,7 @@ BuildStore::BuildStore(const BuildStoreConfig& Config, GcManager& Gc, CidStore&
 
 		if (IsNew)
 		{
-			CleanDirectory(Config.RootDirectory, false);
+			CleanDirectory(Config.RootDirectory, /*ForceRemoveReadOnlyFiles*/ false);
 			CbObjectWriter ManifestWriter;
 			ManifestWriter.AddObjectId("id", Oid::NewOid());
 			ManifestWriter.AddInteger("version", blobstore::impl::ManifestVersion);