fix logic for old blocks in blockstore gc (#295)

* fix logic for old blocks in blockstore gc
If we will remove all entries in a block and keep nothing we can't expect for the block to exist.
If we want to keep entries in a block, the block must exist, if not error and move entries to delete list.
Don't reset output block between blocks we are reading from, keep using it until it exceeds the max limit.
* changelog

1 files changed, 24 insertions, 21 deletions

diff --git a/src/zenstore/blockstore.cpp b/src/zenstore/blockstore.cpp
index 0593ff06b..f9e74fb0b 100644
--- a/src/zenstore/blockstore.cpp
+++ b/src/zenstore/blockstore.cpp
@@ -478,20 +478,7 @@ BlockStore::ReclaimSpace(const ReclaimSnapshotState&			Snapshot,
 				}
 			}
 
-			if (!OldBlockFile)
-			{
-				// If the block file pointed to does not exist, move them all to deleted list
-				ChunkIndexArray& KeepMap = BlockKeepChunks[ChunkMapIndex];
-				ZEN_ERROR("Expected to find block {} in {} - this should never happen, marking {} entries as deleted.",
-						  BlockIndex,
-						  m_BlocksBasePath,
-						  KeepMap.size());
-
-				BlockDeleteChunks[ChunkMapIndex].insert(BlockDeleteChunks[ChunkMapIndex].end(), KeepMap.begin(), KeepMap.end());
-				KeepMap.clear();
-			}
-
-			const ChunkIndexArray& KeepMap = BlockKeepChunks[ChunkMapIndex];
+			ChunkIndexArray& KeepMap = BlockKeepChunks[ChunkMapIndex];
 			if (KeepMap.empty())
 			{
 				const ChunkIndexArray& DeleteMap = BlockDeleteChunks[ChunkMapIndex];
@@ -520,8 +507,17 @@ BlockStore::ReclaimSpace(const ReclaimSnapshotState&			Snapshot,
 				}
 				continue;
 			}
+			else if (!OldBlockFile)
+			{
+				// If the block file pointed to does not exist, move any keep chunk them to deleted list
+				ZEN_ERROR("Expected to find block {} in {} - this should never happen, marking {} entries as deleted.",
+						  BlockIndex,
+						  m_BlocksBasePath,
+						  KeepMap.size());
 
-			ZEN_ASSERT(OldBlockFile);
+				BlockDeleteChunks[ChunkMapIndex].insert(BlockDeleteChunks[ChunkMapIndex].end(), KeepMap.begin(), KeepMap.end());
+				KeepMap.clear();
+			}
 
 			MovedChunksArray	 MovedChunks;
 			std::vector<uint8_t> Chunk;
@@ -616,7 +612,6 @@ BlockStore::ReclaimSpace(const ReclaimSnapshotState&			Snapshot,
 			if (NewBlockFile)
 			{
 				NewBlockFile->Flush();
-				NewBlockFile = nullptr;
 			}
 
 			const ChunkIndexArray& DeleteMap = BlockDeleteChunks[ChunkMapIndex];
@@ -637,13 +632,21 @@ BlockStore::ReclaimSpace(const ReclaimSnapshotState&			Snapshot,
 					ReadBlockTimeUs += ElapsedUs;
 					ReadBlockLongestTimeUs = std::max(ElapsedUs, ReadBlockLongestTimeUs);
 				});
-				ZEN_DEBUG("marking cas block store file '{}' for delete, block #{}", OldBlockFile->GetPath(), BlockIndex);
-				ZEN_ASSERT(m_ChunkBlocks[BlockIndex] == OldBlockFile);
-				m_ChunkBlocks.erase(BlockIndex);
-				m_TotalSize.fetch_sub(OldBlockFile->FileSize(), std::memory_order::relaxed);
-				OldBlockFile->MarkAsDeleteOnClose();
+				if (OldBlockFile)
+				{
+					ZEN_DEBUG("marking cas block store file '{}' for delete, block #{}", OldBlockFile->GetPath(), BlockIndex);
+					ZEN_ASSERT(m_ChunkBlocks[BlockIndex] == OldBlockFile);
+					m_ChunkBlocks.erase(BlockIndex);
+					m_TotalSize.fetch_sub(OldBlockFile->FileSize(), std::memory_order::relaxed);
+					OldBlockFile->MarkAsDeleteOnClose();
+				}
 			}
 		}
+		if (NewBlockFile)
+		{
+			NewBlockFile->Flush();
+			NewBlockFile = nullptr;
+		}
 	}
 	catch (std::exception& ex)
 	{