GC cleanup

1 files changed, 24 insertions, 26 deletions

diff --git a/zenstore/compactcas.cpp b/zenstore/compactcas.cpp
index 2171fd3c0..f80503757 100644
--- a/zenstore/compactcas.cpp
+++ b/zenstore/compactcas.cpp
@@ -150,7 +150,12 @@ CasContainerStrategy::ChunkBlock::Create(uint64_t InitialSize)
 {
 	ZEN_ASSERT(!m_IsOpened.load());
 
-	CreateDirectories(m_Path.parent_path());
+	auto ParentPath = m_Path.parent_path();
+	if (!std::filesystem::is_directory(ParentPath))
+	{
+		CreateDirectories(ParentPath);
+	}
+
 	m_File.Open(m_Path, true);
 	if (InitialSize > 0)
 	{
@@ -618,28 +623,22 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 
 	for (auto BlockIndex : BlocksToReWrite)
 	{
-		auto& ChunkMap = KeepChunks[BlockIndexToKeepChunksMap[BlockIndex]];
+		const auto& ChunkMap = KeepChunks[BlockIndexToKeepChunksMap[BlockIndex]];
 		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?
-
-			RwLock::ExclusiveLockScope _i(m_LocationMapLock);
-			auto					   BlockFile = m_ChunkBlocks[BlockIndex];
-			ZEN_INFO("marking cas store file for delete {}, block {}", m_ContainerBaseName, std::to_string(BlockIndex));
+			std::shared_ptr<ChunkBlock> BlockFile;
+			{
+				RwLock::ExclusiveLockScope _i(m_LocationMapLock);
+				CHECK(m_BlockedGcd.contains(BlockIndex));
+				m_ChunkBlocks[BlockIndex].swap(BlockFile);
+			}
+			ZEN_DEBUG("marking cas store file for delete {}, block {}", m_ContainerBaseName, std::to_string(BlockIndex));
 			std::error_code Ec;
 			BlockFile->MarkAsDeleteOnClose(Ec);
 			if (Ec)
 			{
 				ZEN_WARN("Failed to flag file '{}' for deletion: '{}'", BlockFile->GetPath(), Ec.message());
 			}
-			BlockFile.reset();
 			continue;
 		}
 
@@ -660,7 +659,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 
 				if (!NewBlockFile || (WriteOffset + Chunk.size() > m_MaxBlockSize))
 				{
-					NewBlockIndex = m_WriteBlockIndex.load();
+					uint32_t NextBlockIndex = m_WriteBlockIndex.load();
 
 					{
 						RwLock::ExclusiveLockScope _l(m_LocationMapLock);
@@ -683,13 +682,13 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 						{
 							throw std::runtime_error(fmt::format("unable to allocate a new block in {}", m_ContainerBaseName));
 						}
-						while (m_ChunkBlocks.contains(NewBlockIndex))
+						while (m_ChunkBlocks.contains(NextBlockIndex))
 						{
-							NewBlockIndex = (NewBlockIndex + 1) & CasDiskLocation::MaxBlockIndex;
+							NextBlockIndex = (NextBlockIndex + 1) & CasDiskLocation::MaxBlockIndex;
 						}
-						auto NewBlockPath			 = BuildUcasPath(m_BlocksBasePath, NewBlockIndex);
+						auto NewBlockPath			 = BuildUcasPath(m_BlocksBasePath, NextBlockIndex);
 						NewBlockFile				 = std::make_shared<ChunkBlock>(NewBlockPath);
-						m_ChunkBlocks[NewBlockIndex] = NewBlockFile;
+						m_ChunkBlocks[NextBlockIndex] = NewBlockFile;
 					}
 
 					std::error_code Error;
@@ -710,14 +709,14 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 									 m_MaxBlockSize,
 									 NiceBytes(Space.Free));
 							RwLock::ExclusiveLockScope _l(m_LocationMapLock);
-							m_ChunkBlocks.erase(NewBlockIndex);
+							m_ChunkBlocks.erase(NextBlockIndex);
 							return;
 						}
 
 						ZEN_INFO("using gc reserve for '{}', disk free {}",
 								 m_Config.RootDirectory / m_ContainerBaseName,
 								 NiceBytes(Space.Free));
-						auto NewBlockPath = BuildUcasPath(m_BlocksBasePath, NewBlockIndex);
+						auto NewBlockPath = BuildUcasPath(m_BlocksBasePath, NextBlockIndex);
 						std::filesystem::rename(GCReservePath, NewBlockPath);
 						NewBlockFile->Open();
 					}
@@ -725,15 +724,14 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 					{
 						NewBlockFile->Create(m_MaxBlockSize);
 					}
-
+					NewBlockIndex = NextBlockIndex;
 					MovedBlocks.clear();
 					WriteOffset = 0;
 				}
 
 				NewBlockFile->Write(Chunk.data(), Chunk.size(), WriteOffset);
 				CasLocation NewChunkLocation(NewBlockIndex, WriteOffset, Chunk.size());
-				Entry.second			 = CasDiskLocation(NewChunkLocation, m_PayloadAlignment);
-				MovedBlocks[Entry.first] = Entry.second;
+				MovedBlocks.emplace(Entry.first, CasDiskLocation(NewChunkLocation, m_PayloadAlignment));
 				WriteOffset				 = RoundUp(WriteOffset + Chunk.size(), m_PayloadAlignment);
 			}
 			Chunk.clear();
@@ -748,7 +746,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx)
 					m_CasLog.Append({.Key = MovedEntry.first, .Location = MovedEntry.second});
 				}
 			}
-			ZEN_INFO("marking cas store file for delete {}, block index {}", m_ContainerBaseName, BlockIndex);
+			ZEN_DEBUG("marking cas store file for delete {}, block index {}", m_ContainerBaseName, BlockIndex);
 			std::error_code Ec;
 			BlockFile->MarkAsDeleteOnClose(Ec);
 			if (Ec)