diff options
Diffstat (limited to 'zenstore')
| -rw-r--r-- | zenstore/blockstore.cpp | 27 | ||||
| -rw-r--r-- | zenstore/compactcas.cpp | 21 | ||||
| -rw-r--r-- | zenstore/include/zenstore/blockstore.h | 5 |
3 files changed, 26 insertions, 27 deletions
diff --git a/zenstore/blockstore.cpp b/zenstore/blockstore.cpp index e43c3a00f..3d2d77b4e 100644 --- a/zenstore/blockstore.cpp +++ b/zenstore/blockstore.cpp @@ -24,7 +24,7 @@ BlockStoreFile::BlockStoreFile(const std::filesystem::path& BlockPath) : m_Path( BlockStoreFile::~BlockStoreFile() { - RwLock::ExclusiveLockScope _(m_OpenLock); + RwLock::ExclusiveLockScope _(m_FileLock); m_IoBuffer = IoBuffer(); m_File.Detach(); } @@ -36,8 +36,9 @@ BlockStoreFile::GetPath() const } void -BlockStoreFile::InternalOpen() +BlockStoreFile::Open() { + RwLock::ExclusiveLockScope _(m_FileLock); if (m_File.Handle()) { return; @@ -48,16 +49,9 @@ BlockStoreFile::InternalOpen() } void -BlockStoreFile::Open() -{ - RwLock::ExclusiveLockScope _(m_OpenLock); - InternalOpen(); -} - -void BlockStoreFile::Create(uint64_t InitialSize) { - RwLock::ExclusiveLockScope _(m_OpenLock); + RwLock::ExclusiveLockScope _(m_FileLock); auto ParentPath = m_Path.parent_path(); if (!std::filesystem::is_directory(ParentPath)) @@ -77,14 +71,14 @@ BlockStoreFile::Create(uint64_t InitialSize) uint64_t BlockStoreFile::FileSize() { - RwLock::SharedLockScope _(m_OpenLock); + RwLock::SharedLockScope _(m_FileLock); return m_File.FileSize(); } void BlockStoreFile::MarkAsDeleteOnClose(std::error_code& Ec) { - RwLock::ExclusiveLockScope _(m_OpenLock); + RwLock::ExclusiveLockScope _(m_FileLock); if (m_File.Handle()) { m_File.MarkAsDeleteOnClose(Ec); @@ -100,28 +94,27 @@ BlockStoreFile::MarkAsDeleteOnClose(std::error_code& Ec) IoBuffer BlockStoreFile::GetChunk(uint64_t Offset, uint64_t Size) { - InternalOpen(); return IoBuffer(m_IoBuffer, Offset, Size); } void BlockStoreFile::Read(void* Data, uint64_t Size, uint64_t FileOffset) { - RwLock::SharedLockScope _(m_OpenLock); + RwLock::SharedLockScope _(m_FileLock); m_File.Read(Data, Size, FileOffset); } void BlockStoreFile::Write(const void* Data, uint64_t Size, uint64_t FileOffset) { - RwLock::SharedLockScope _(m_OpenLock); + RwLock::SharedLockScope _(m_FileLock); m_File.Write(Data, Size, FileOffset); } void BlockStoreFile::Flush() { - RwLock::ExclusiveLockScope _(m_OpenLock); + RwLock::ExclusiveLockScope _(m_FileLock); if (!m_File.Handle()) { return; @@ -132,7 +125,7 @@ BlockStoreFile::Flush() void BlockStoreFile::StreamByteRange(uint64_t FileOffset, uint64_t Size, std::function<void(const void* Data, uint64_t Size)>&& ChunkFun) { - RwLock::SharedLockScope _(m_OpenLock); + RwLock::SharedLockScope _(m_FileLock); m_File.StreamByteRange(FileOffset, Size, std::move(ChunkFun)); } diff --git a/zenstore/compactcas.cpp b/zenstore/compactcas.cpp index 2bf96ce9d..f7cfed47b 100644 --- a/zenstore/compactcas.cpp +++ b/zenstore/compactcas.cpp @@ -623,15 +623,22 @@ CasContainerStrategy::InsertChunk(const void* ChunkData, size_t ChunkSize, const m_CurrentInsertOffset = 0; m_WriteBlock->Create(m_MaxBlockSize); } - else - { - m_WriteBlock->Open(); - } InsertOffset = m_CurrentInsertOffset; m_CurrentInsertOffset = RoundUp(InsertOffset + ChunkSize, m_PayloadAlignment); WriteBlock = m_WriteBlock; } + // We can end up in a situation that two InsertChunk writes the same chunk + // data in different locations. + // We release the insert lock once we have the correct WriteBlock ready and we know + // where to write the data. If a new InsertChunk requests comes in before we update + // m_LocationMap below we will have a race. + // The outcome of that is that we will occupy space more than once but the hash will + // only point to one of the chunks. We will in that case waste space until the next + // GC operation. + // This would be a rare occasion and the current flow reduces the time we block for + // reads, insert and GC. + BlockStoreDiskLocation Location({.BlockIndex = WriteBlockIndex, .Offset = InsertOffset, .Size = ChunkSize}, m_PayloadAlignment); const CasDiskIndexEntry IndexEntry{.Key = ChunkHash, .Location = Location}; @@ -1052,8 +1059,6 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx) continue; } - OldBlockFile->Open(); - std::vector<uint8_t> Chunk; for (const IoHash& ChunkHash : KeepMap) { @@ -1067,6 +1072,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx) uint32_t NextBlockIndex = m_WriteBlockIndex.load(std::memory_order::memory_order_relaxed); auto LogEntries = MakeCasDiskEntries(MovedBlockChunks, {}); m_CasLog.Append(LogEntries); + { RwLock::ExclusiveLockScope __(m_LocationMapLock); Stopwatch Timer; @@ -1098,7 +1104,6 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx) ZEN_ERROR("get disk space in {} FAILED, reason '{}'", m_ContainerBaseName, Error.message()); return; } - if (Space.Free < m_MaxBlockSize) { std::filesystem::path GCReservePath = GetGCReservePath(m_Config.RootDirectory, m_ContainerBaseName); @@ -1130,6 +1135,7 @@ CasContainerStrategy::CollectGarbage(GcContext& GcCtx) WriteOffset = 0; } + NewBlockFile->Write(Chunk.data(), Chunk.size(), WriteOffset); MovedBlockChunks.emplace( ChunkHash, @@ -1435,6 +1441,7 @@ CasContainerStrategy::OpenContainer(bool IsNewStore) } auto BlockPath = GetBlockPath(m_BlocksBasePath, BlockIndex); auto BlockFile = std::make_shared<BlockStoreFile>(BlockPath); + BlockFile->Open(); m_ChunkBlocks[BlockIndex] = BlockFile; } } diff --git a/zenstore/include/zenstore/blockstore.h b/zenstore/include/zenstore/blockstore.h index 5222ee50e..64fd1b299 100644 --- a/zenstore/include/zenstore/blockstore.h +++ b/zenstore/include/zenstore/blockstore.h @@ -93,11 +93,10 @@ struct BlockStoreFile void StreamByteRange(uint64_t FileOffset, uint64_t Size, std::function<void(const void* Data, uint64_t Size)>&& ChunkFun); private: - void InternalOpen(); const std::filesystem::path m_Path; - RwLock m_OpenLock; - BasicFile m_File; IoBuffer m_IoBuffer; + RwLock m_FileLock; + BasicFile m_File; }; void blockstore_forcelink(); |