Delete GCd blocks on close.

Still some work to do with holding the file handle alive

1 files changed, 46 insertions, 5 deletions

diff --git a/zenstore/compactcas.cpp b/zenstore/compactcas.cpp
index 18dcc7fc8..ed8ee23ce 100644
--- a/zenstore/compactcas.cpp
+++ b/zenstore/compactcas.cpp
@@ -1,14 +1,15 @@
 // Copyright Epic Games, Inc. All Rights Reserved.
 
+#include "compactcas.h"
+
 #include <zenstore/cas.h>
 
+#include <zencore/except.h>
 #include <zencore/filesystem.h>
 #include <zencore/fmtutils.h>
 #include <zencore/logging.h>
 #include <zencore/testing.h>
-#include <zenstore/gc.h>
 #include <gsl/gsl-lite.hpp>
-#include "compactcas.h"
 
 #if ZEN_WITH_TESTS
 #	include <zencore/compactbinarybuilder.h>
@@ -31,6 +32,30 @@ namespace {
 	{
 		return RootDirectory / (std::string(ContainerBaseName) + "." + (std::to_string(BlockIndex) + ".ucas"));
 	}
+
+	void MarkFileAsDeleteOnClose(void* FileHandle)
+	{
+#if ZEN_PLATFORM_WINDOWS
+		FILE_DISPOSITION_INFO Fdi{};
+		Fdi.DeleteFile = TRUE;
+		BOOL Success   = SetFileInformationByHandle(FileHandle, FileDispositionInfo, &Fdi, sizeof Fdi);
+		if (!Success)
+		{
+			ZEN_WARN("Failed to flag temporary payload file '{}' for deletion: '{}'", PathFromHandle(FileHandle), GetLastErrorAsString());
+		}
+#elif ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC
+		std::filesystem::path SourcePath = PathFromHandle(FileRef.FileHandle);
+		if (unlink(SourcePath.c_str()) < 0)
+		{
+			int UnlinkError = zen::GetLastError();
+			if (UnlinkError != ENOENT)
+			{
+				ZEN_WARN("unlink of CAS payload file failed ('{}')", GetSystemErrorAsString(UnlinkError));
+			}
+		}
+#endif
+	}
+
 }  // namespace
 
 CasContainerStrategy::CasContainerStrategy(const CasStoreConfiguration& Config, CasGc& Gc)
@@ -385,6 +410,8 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 			m_TotalSize.fetch_sub(static_cast<uint64_t>(ChunkLocation.Size));
 		}
 
+		// TODO: Be smarter about terminating current block - we should probably not rewrite if there is just
+		// a small amount of bytes to gain.
 		if (BlocksToReWrite.contains(m_CurrentBlockIndex))
 		{
 			uint16_t NewBlockIndex = m_CurrentBlockIndex + 1;
@@ -422,14 +449,22 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 		if (ChunkMap.empty())
 		{
 			// The block has no references to it, it should be removed as soon as no references is held on the file
+
 			// TODO: We currently don't know if someone is holding a IoBuffer for this block at this point!
+			// We want one IoBuffer that owns each block and use that to keep stuff alive using Owning strategy
+			// From that IoBuffer we fetch the file handle
+			// When we are done we dispose that IoBuffer and drop the file handle
+			// Can we create a Sub-IoBuffer from our main buffer even if the size has grown past the initial
+			// size when creating it?
 
-			// std::filesystem::path	   BlockPath = BuildUcasPath(m_Config.RootDirectory, m_ContainerBaseName, BlockIndex);
-			// RwLock::ExclusiveLockScope _i(m_InsertLock);
-			// auto BlockFile = m_OpenBlocks[BlockIndex];
+			RwLock::ExclusiveLockScope _i(m_InsertLock);
+			auto					   BlockFile  = m_OpenBlocks[BlockIndex];
+			auto					   FileHandle = BlockFile->Handle();
 			// m_OpenBlocks.erase(BlockIndex);
 			// BlockFile->Close();
 			// fs::remove(BlockPath);
+			ZEN_INFO("marking cas store file for delete {}, count limit {} exeeded", m_ContainerBaseName, PathFromHandle(FileHandle));
+			MarkFileAsDeleteOnClose(FileHandle);
 			continue;
 		}
 
@@ -524,6 +559,12 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 					m_CasLog.Append({.Key = MovedEntry.first, .Location = MovedEntry.second});
 				}
 			}
+			auto FileHandle = BlockFile->Handle();
+			// m_OpenBlocks.erase(BlockIndex);
+			// BlockFile->Close();
+			// fs::remove(BlockPath);
+			ZEN_INFO("marking cas store file for delete {}, count limit {} exeeded", m_ContainerBaseName, PathFromHandle(FileHandle));
+			MarkFileAsDeleteOnClose(FileHandle);
 		}
 	}