diff options
| author | Dan Engelbrecht <[email protected]> | 2023-11-08 15:25:56 +0100 |
|---|---|---|
| committer | GitHub <[email protected]> | 2023-11-08 15:25:56 +0100 |
| commit | b8460468bcdb9f331d06afb2b3b9967bdf915aab (patch) | |
| tree | d398e0b6c1d8590c237ff8611020621d942078f1 | |
| parent | implemented openssl-free encryption for Windows (#520) (diff) | |
| download | zen-b8460468bcdb9f331d06afb2b3b9967bdf915aab.tar.xz zen-b8460468bcdb9f331d06afb2b3b9967bdf915aab.zip | |
disk layer gc and error/warnings cleanup (#515)
- Improvement: Use GC reserve when writing index/manifest for a disk cache bucket when disk is low when available
- Improvement: Demote errors to warning for issues that are not critical and we handle gracefully
- Improvement: Treat more out of memory errors from windows as Out Of Memory errors
Fixed wrong sizeof() statement for compactcas index (luckily the two structs are of same size)
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | src/zencore/except.cpp | 2 | ||||
| -rw-r--r-- | src/zenhttp/auth/authmgr.cpp | 2 | ||||
| -rw-r--r-- | src/zenserver/cache/cachedisklayer.cpp | 115 | ||||
| -rw-r--r-- | src/zenserver/cache/cachedisklayer.h | 6 | ||||
| -rw-r--r-- | src/zenserver/projectstore/remoteprojectstore.cpp | 82 | ||||
| -rw-r--r-- | src/zenstore/compactcas.cpp | 4 |
7 files changed, 134 insertions, 80 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c21cd4667..1018afdd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ - Improvement: Added tests for BlockStore::CompactBlocks - Improvement: Reduce memory consumption in cache disk layer - Improvement: Refactored logging so that spdlog details are hidden from the majority of client code +- Improvement: Use GC reserve when writing index/manifest for a disk cache bucket when disk is low when available +- Improvement: Demote errors to warning for issues that are not critical and we handle gracefully +- Improvement: Treat more out of memory errors from windows as Out Of Memory errors - Improvement: Factored out some compiler / platform definitions into standalone `zenbase` header-only library, along with header-only helpers which can be used standalone. This is intended to support out-of-tree code like pluggable transports etc but also provides more fine grained dependencies - Improvement: Replaced use of openssl on Windows with bcrypt, which reduces executable size by some 40% - Improvement: We no longer put cache entries into the memory cache on Put, only on Get diff --git a/src/zencore/except.cpp b/src/zencore/except.cpp index f98743ea9..d5eabea9d 100644 --- a/src/zencore/except.cpp +++ b/src/zencore/except.cpp @@ -119,8 +119,10 @@ IsOOM(const std::system_error& SystemError) case ERROR_NOT_ENOUGH_MEMORY: case ERROR_OUTOFMEMORY: case ERROR_PAGEFILE_QUOTA_EXCEEDED: + case ERROR_NO_SYSTEM_RESOURCES: case ERROR_NONPAGED_SYSTEM_RESOURCES: case ERROR_PAGED_SYSTEM_RESOURCES: + case ERROR_WORKING_SET_QUOTA: case ERROR_PAGEFILE_QUOTA: case ERROR_COMMITMENT_LIMIT: return true; diff --git a/src/zenhttp/auth/authmgr.cpp b/src/zenhttp/auth/authmgr.cpp index e84f63a3f..18568a21d 100644 --- a/src/zenhttp/auth/authmgr.cpp +++ b/src/zenhttp/auth/authmgr.cpp @@ -369,7 +369,7 @@ private: } catch (std::exception& Err) { - ZEN_ERROR("serialize state FAILED, reason '{}'", Err.what()); + ZEN_WARN("serialize state FAILED, reason '{}'", Err.what()); } } diff --git a/src/zenserver/cache/cachedisklayer.cpp b/src/zenserver/cache/cachedisklayer.cpp index a9ac46cab..d66430f15 100644 --- a/src/zenserver/cache/cachedisklayer.cpp +++ b/src/zenserver/cache/cachedisklayer.cpp @@ -6,6 +6,7 @@ #include <zencore/compactbinarybuilder.h> #include <zencore/compactbinaryvalidation.h> #include <zencore/compress.h> +#include <zencore/except.h> #include <zencore/fmtutils.h> #include <zencore/jobqueue.h> #include <zencore/logging.h> @@ -157,12 +158,6 @@ LoadCompactBinaryObject(const fs::path& Path) return CbObject(); } -static void -SaveCompactBinaryObject(const fs::path& Path, const CbObject& Object) -{ - WriteFile(Path, Object.GetBuffer().AsIoBuffer()); -} - ////////////////////////////////////////////////////////////////////////// ZenCacheDiskLayer::CacheBucket::CacheBucket(GcManager& Gc, @@ -231,7 +226,7 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir, bo Writer << "BucketId"sv << m_BucketId; Writer << "Version"sv << CurrentDiskBucketVersion; Manifest = Writer.Save(); - SaveCompactBinaryObject(ManifestPath, Manifest); + WriteFile(m_BucketDir / "zen_manifest", Manifest.GetBuffer().AsIoBuffer()); IsNew = true; } else @@ -348,7 +343,7 @@ ZenCacheDiskLayer::CacheBucket::OpenOrCreate(std::filesystem::path BucketDir, bo } void -ZenCacheDiskLayer::CacheBucket::MakeIndexSnapshot() +ZenCacheDiskLayer::CacheBucket::MakeIndexSnapshot(const std::function<uint64_t()>& ClaimDiskReserveFunc) { ZEN_TRACE_CPU("Z$::Disk::Bucket::MakeIndexSnapshot"); @@ -405,6 +400,26 @@ ZenCacheDiskLayer::CacheBucket::MakeIndexSnapshot() } } + uint64_t IndexSize = sizeof(CacheBucketIndexHeader) + Entries.size() * sizeof(DiskIndexEntry); + std::error_code Error; + DiskSpace Space = DiskSpaceInfo(m_BucketDir, Error); + if (Error) + { + throw std::system_error(Error, fmt::format("get disk space in '{}' FAILED", m_BucketDir)); + } + + bool EnoughSpace = Space.Free >= IndexSize + 1024 * 512; + if (!EnoughSpace) + { + uint64_t ReclaimedSpace = ClaimDiskReserveFunc(); + EnoughSpace = (Space.Free + ReclaimedSpace) >= IndexSize + 1024 * 512; + } + if (!EnoughSpace) + { + throw std::runtime_error( + fmt::format("not enough free disk space in '{}' to save index of size {}", m_BucketDir, NiceBytes(IndexSize))); + } + BasicFile ObjectIndexFile; ObjectIndexFile.Open(IndexPath, BasicFile::Mode::kTruncate); CacheBucketIndexHeader Header = {.EntryCount = Entries.size(), @@ -412,7 +427,6 @@ ZenCacheDiskLayer::CacheBucket::MakeIndexSnapshot() .PayloadAlignment = gsl::narrow<uint32_t>(m_Configuration.PayloadAlignment)}; Header.Checksum = CacheBucketIndexHeader::ComputeChecksum(Header); - ObjectIndexFile.Write(&Header, sizeof(CacheBucketIndexHeader), 0); ObjectIndexFile.Write(Entries.data(), Entries.size() * sizeof(DiskIndexEntry), sizeof(CacheBucketIndexHeader)); ObjectIndexFile.Flush(); @@ -951,34 +965,61 @@ ZenCacheDiskLayer::CacheBucket::Flush() ZEN_INFO("Flushing bucket {}", m_BucketDir); - m_BlockStore.Flush(/*ForceNewBlock*/ false); - m_SlogFile.Flush(); + try + { + m_BlockStore.Flush(/*ForceNewBlock*/ false); + m_SlogFile.Flush(); - std::vector<AccessTime> AccessTimes; - std::vector<BucketPayload> Payloads; - IndexMap Index; + std::vector<AccessTime> AccessTimes; + std::vector<BucketPayload> Payloads; + IndexMap Index; + { + RwLock::SharedLockScope IndexLock(m_IndexLock); + MakeIndexSnapshot(); + Index = m_Index; + Payloads = m_Payloads; + AccessTimes = m_AccessTimes; + } + SaveManifest(MakeManifest(std::move(Index), std::move(AccessTimes), Payloads)); + } + catch (std::exception& Ex) { - RwLock::SharedLockScope IndexLock(m_IndexLock); - MakeIndexSnapshot(); - Index = m_Index; - Payloads = m_Payloads; - AccessTimes = m_AccessTimes; + ZEN_WARN("Failed to flush bucket in '{}'. Reason: '{}'", m_BucketDir, Ex.what()); } - SaveManifest(MakeManifest(std::move(Index), std::move(AccessTimes), Payloads)); } void -ZenCacheDiskLayer::CacheBucket::SaveManifest(CbObject&& Manifest) +ZenCacheDiskLayer::CacheBucket::SaveManifest(CbObject&& Manifest, const std::function<uint64_t()>& ClaimDiskReserveFunc) { ZEN_TRACE_CPU("Z$::Disk::Bucket::SaveManifest"); try { - SaveCompactBinaryObject(m_BucketDir / "zen_manifest", Manifest); + IoBuffer Buffer = Manifest.GetBuffer().AsIoBuffer(); + + std::error_code Error; + DiskSpace Space = DiskSpaceInfo(m_BucketDir, Error); + if (Error) + { + ZEN_WARN("get disk space in '{}' FAILED, reason: '{}'", m_BucketDir, Error.message()); + return; + } + bool EnoughSpace = Space.Free >= Buffer.GetSize() + 1024 * 512; + if (!EnoughSpace) + { + uint64_t ReclaimedSpace = ClaimDiskReserveFunc(); + EnoughSpace = (Space.Free + ReclaimedSpace) >= Buffer.GetSize() + 1024 * 512; + } + if (!EnoughSpace) + { + ZEN_WARN("not enough free disk space in '{}'. FAILED to save manifest of size {}", m_BucketDir, NiceBytes(Buffer.GetSize())); + return; + } + WriteFile(m_BucketDir / "zen_manifest", Buffer); } catch (std::exception& Err) { - ZEN_WARN("writing manifest FAILED, reason: '{}'", Err.what()); + ZEN_WARN("writing manifest in '{}' FAILED, reason: '{}'", m_BucketDir, Err.what()); } } @@ -1626,17 +1667,24 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx) } auto FlushingGuard = MakeGuard([&] { m_IsFlushing.store(false); }); - std::vector<AccessTime> AccessTimes; - std::vector<BucketPayload> Payloads; - IndexMap Index; + try { - RwLock::SharedLockScope IndexLock(m_IndexLock); - MakeIndexSnapshot(); - Index = m_Index; - Payloads = m_Payloads; - AccessTimes = m_AccessTimes; + std::vector<AccessTime> AccessTimes; + std::vector<BucketPayload> Payloads; + IndexMap Index; + { + RwLock::SharedLockScope IndexLock(m_IndexLock); + MakeIndexSnapshot([&]() { return GcCtx.ClaimGCReserve(); }); + Index = m_Index; + Payloads = m_Payloads; + AccessTimes = m_AccessTimes; + } + SaveManifest(MakeManifest(std::move(Index), std::move(AccessTimes), Payloads), [&]() { return GcCtx.ClaimGCReserve(); }); + } + catch (std::exception& Ex) + { + ZEN_WARN("Failed to write index and manifest after GC in '{}'. Reason: '{}'", m_BucketDir, Ex.what()); } - SaveManifest(MakeManifest(std::move(Index), std::move(AccessTimes), Payloads)); }); m_SlogFile.Flush(); @@ -1727,7 +1775,6 @@ ZenCacheDiskLayer::CacheBucket::CollectGarbage(GcContext& GcCtx) m_SlogFile.Append(ExpiredStandaloneEntries); } } - SaveManifest(MakeManifest(std::move(Index), std::move(AccessTimes), Payloads)); } if (GcCtx.IsDeletionMode()) @@ -3186,7 +3233,7 @@ ZenCacheDiskLayer::DiscoverBuckets() } catch (const std::exception& Err) { - ZEN_ERROR("creating bucket '{}' in '{}' FAILED, reason: '{}'", BucketName, BucketPath, Err.what()); + ZEN_ERROR("Opening bucket '{}' in '{}' FAILED, reason: '{}'", BucketName, BucketPath, Err.what()); return; } }); diff --git a/src/zenserver/cache/cachedisklayer.h b/src/zenserver/cache/cachedisklayer.h index 4b9d4ed1e..99c54e192 100644 --- a/src/zenserver/cache/cachedisklayer.h +++ b/src/zenserver/cache/cachedisklayer.h @@ -331,12 +331,14 @@ private: IoBuffer GetStandaloneCacheValue(ZenContentType ContentType, const IoHash& HashKey) const; void PutInlineCacheValue(const IoHash& HashKey, const ZenCacheValue& Value, std::span<IoHash> References); IoBuffer GetInlineCacheValue(const DiskLocation& Loc) const; - void MakeIndexSnapshot(); + void MakeIndexSnapshot(const std::function<uint64_t()>& ClaimDiskReserveFunc = []() { return 0; }); uint64_t ReadIndexFile(const std::filesystem::path& IndexPath, uint32_t& OutVersion); uint64_t ReadLog(const std::filesystem::path& LogPath, uint64_t LogPosition); void OpenLog(const bool IsNew); CbObject MakeManifest(IndexMap&& Index, std::vector<AccessTime>&& AccessTimes, const std::vector<BucketPayload>& Payloads); - void SaveManifest(CbObject&& Manifest); + void SaveManifest( + CbObject&& Manifest, + const std::function<uint64_t()>& ClaimDiskReserveFunc = []() { return 0; }); CacheValueDetails::ValueDetails GetValueDetails(const IoHash& Key, PayloadIndex Index) const; void CompactReferences(RwLock::ExclusiveLockScope&); void SetReferences(RwLock::ExclusiveLockScope&, ReferenceIndex& FirstReferenceIndex, std::span<IoHash> References); diff --git a/src/zenserver/projectstore/remoteprojectstore.cpp b/src/zenserver/projectstore/remoteprojectstore.cpp index 4194c4315..d5d229e42 100644 --- a/src/zenserver/projectstore/remoteprojectstore.cpp +++ b/src/zenserver/projectstore/remoteprojectstore.cpp @@ -893,7 +893,7 @@ UploadAttachments(WorkerThreadPool& WorkerPool, { RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::NotFound), "Invalid attachment", - fmt::format("Upload requested of unknown attachement '{}'", Needed)); + fmt::format("Upload requested of unknown attachment '{}'", Needed)); ZEN_ERROR("Failed to upload attachment '{}'. ({}). Reason: '{}'", Needed, RemoteResult.GetError(), @@ -963,10 +963,10 @@ UploadAttachments(WorkerThreadPool& WorkerPool, RemoteResult.SetError(gsl::narrow<int>(HttpResponseCode::NotFound), fmt::format("Failed to find attachment {}", RawHash), {}); - ZEN_ERROR("Failed to save attachment '{}' ({}). Reason: '{}'", - RawHash, - RemoteResult.GetErrorReason(), - RemoteResult.GetError()); + ZEN_WARN("Failed to save attachment '{}' ({}). Reason: '{}'", + RawHash, + RemoteResult.GetErrorReason(), + RemoteResult.GetError()); return; } @@ -975,11 +975,11 @@ UploadAttachments(WorkerThreadPool& WorkerPool, if (Result.ErrorCode) { RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text); - ZEN_ERROR("Failed to save attachment '{}', {} ({}). Reason: '{}'", - RawHash, - NiceBytes(Payload.GetSize()), - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to save attachment '{}', {} ({}). Reason: '{}'", + RawHash, + NiceBytes(Payload.GetSize()), + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } ZEN_DEBUG("Saved attachment {}, {} in {}", @@ -1025,11 +1025,11 @@ UploadAttachments(WorkerThreadPool& WorkerPool, if (Result.ErrorCode) { RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text); - ZEN_ERROR("Failed to save attachment '{}', {} ({}). Reason: '{}'", - RawHash, - NiceBytes(Payload.GetSize()), - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to save attachment '{}', {} ({}). Reason: '{}'", + RawHash, + NiceBytes(Payload.GetSize()), + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } @@ -1100,10 +1100,10 @@ UploadAttachments(WorkerThreadPool& WorkerPool, if (Result.ErrorCode) { RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text); - ZEN_ERROR("Failed to save attachments with {} chunks ({}). Reason: '{}'", - Chunks.size(), - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to save attachments with {} chunks ({}). Reason: '{}'", + Chunks.size(), + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } ZEN_DEBUG("Saved {} bulk attachments in {}", @@ -1227,7 +1227,7 @@ SaveOplog(CidStore& ChunkStore, if (Result.ErrorCode) { RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text); - ZEN_ERROR("Failed to save attachment ({}). Reason: '{}'", RemoteResult.GetErrorReason(), RemoteResult.GetError()); + ZEN_WARN("Failed to save attachment ({}). Reason: '{}'", RemoteResult.GetErrorReason(), RemoteResult.GetError()); return; } ZEN_DEBUG("Saved block {}, {}", BlockHash, NiceBytes(CompressedBlock.GetCompressedSize())); @@ -1366,10 +1366,10 @@ SaveOplog(CidStore& ChunkStore, if (ContainerFinalizeResult.ErrorCode) { RemoteResult.SetError(ContainerFinalizeResult.ErrorCode, ContainerFinalizeResult.Reason, ContainerFinalizeResult.Text); - ZEN_ERROR("Failed to finalize oplog container {} ({}). Reason: '{}'", - ContainerSaveResult.RawHash, - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to finalize oplog container {} ({}). Reason: '{}'", + ContainerSaveResult.RawHash, + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); } ZEN_DEBUG("Finalized container in {}", NiceTimeSpanMs(static_cast<uint64_t>(ContainerFinalizeResult.ElapsedSeconds * 1000))); if (ContainerFinalizeResult.Needs.empty()) @@ -1487,7 +1487,7 @@ SaveOplogContainer(ProjectStore::Oplog& Oplog, CbObject SectionObject = LoadCompactBinaryObject(SectionPayload); if (!SectionObject) { - ZEN_ERROR("Failed to save oplog container. Reason: '{}'", "Section has unexpected data type"); + ZEN_WARN("Failed to save oplog container. Reason: '{}'", "Section has unexpected data type"); return RemoteProjectStore::Result{gsl::narrow<int>(HttpResponseCode::BadRequest), Timer.GetElapsedTimeMs() / 1000.500, "Section has unexpected data type", @@ -1573,10 +1573,10 @@ LoadOplog(CidStore& ChunkStore, if (Result.ErrorCode) { RemoteResult.SetError(Result.ErrorCode, Result.Reason, Result.Text); - ZEN_ERROR("Failed to load attachments with {} chunks ({}). Reason: '{}'", - Chunks.size(), - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to load attachments with {} chunks ({}). Reason: '{}'", + Chunks.size(), + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } ZEN_DEBUG("Loaded {} bulk attachments in {}", @@ -1601,10 +1601,10 @@ LoadOplog(CidStore& ChunkStore, if (BlockResult.ErrorCode) { RemoteResult.SetError(BlockResult.ErrorCode, BlockResult.Reason, BlockResult.Text); - ZEN_ERROR("Failed to load oplog container, missing attachment {} ({}). Reason: '{}'", - BlockHash, - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to load oplog container, missing attachment {} ({}). Reason: '{}'", + BlockHash, + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } ZEN_DEBUG("Loaded block attachment in {}", NiceTimeSpanMs(static_cast<uint64_t>(BlockResult.ElapsedSeconds * 1000))); @@ -1616,10 +1616,10 @@ LoadOplog(CidStore& ChunkStore, RemoteResult.SetError(gsl::narrow<int32_t>(HttpResponseCode::InternalServerError), fmt::format("Invalid format for block {}", BlockHash), {}); - ZEN_ERROR("Failed to load oplog container, attachment {} has invalid format ({}). Reason: '{}'", - BlockHash, - RemoteResult.GetError(), - RemoteResult.GetErrorReason()); + ZEN_WARN("Failed to load oplog container, attachment {} has invalid format ({}). Reason: '{}'", + BlockHash, + RemoteResult.GetError(), + RemoteResult.GetErrorReason()); return; } }); @@ -1644,10 +1644,10 @@ LoadOplog(CidStore& ChunkStore, if (AttachmentResult.ErrorCode) { RemoteResult.SetError(AttachmentResult.ErrorCode, AttachmentResult.Reason, AttachmentResult.Text); - ZEN_ERROR("Failed to download attachment {}, reason: '{}', error code: {}", - RawHash, - AttachmentResult.Reason, - AttachmentResult.ErrorCode); + ZEN_WARN("Failed to download attachment {}, reason: '{}', error code: {}", + RawHash, + AttachmentResult.Reason, + AttachmentResult.ErrorCode); return; } ZEN_DEBUG("Loaded attachment in {}", NiceTimeSpanMs(static_cast<uint64_t>(AttachmentResult.ElapsedSeconds * 1000))); diff --git a/src/zenstore/compactcas.cpp b/src/zenstore/compactcas.cpp index de2800895..00a018948 100644 --- a/src/zenstore/compactcas.cpp +++ b/src/zenstore/compactcas.cpp @@ -885,8 +885,8 @@ CasContainerStrategy::MakeIndexSnapshot() Header.Checksum = CasDiskIndexHeader::ComputeChecksum(Header); - ObjectIndexFile.Write(&Header, sizeof(CasDiskIndexEntry), 0); - ObjectIndexFile.Write(Entries.data(), Entries.size() * sizeof(CasDiskIndexEntry), sizeof(CasDiskIndexEntry)); + ObjectIndexFile.Write(&Header, sizeof(CasDiskIndexHeader), 0); + ObjectIndexFile.Write(Entries.data(), Entries.size() * sizeof(CasDiskIndexEntry), sizeof(CasDiskIndexHeader)); ObjectIndexFile.Flush(); ObjectIndexFile.Close(); EntryCount = Entries.size(); |