From c0f8fed887b18fdb7e71899899b012020b72b0d2 Mon Sep 17 00:00:00 2001 From: Suhas Narreddy Date: Wed, 11 Nov 2026 00:20:06 +0100 Subject: [PATCH] Dump: load property pages in one copy per page --- .../dump/EmbeddingPropertyContainerLoader.cpp | 112 ++++++------ storage/dump/GraphDumpHelper.h | 63 +++++++ storage/dump/ListPropertyContainerLoader.cpp | 69 +------ storage/dump/MapPropertyContainerLoader.cpp | 69 +------ storage/dump/PropertyContainerDumpConstants.h | 10 +- storage/dump/PropertyContainerLoader.h | 156 +++------------- test/storage/CMakeLists.txt | 1 + .../dump/PropertyLoaderPageCountTest.cpp | 170 ++++++++++++++++++ 8 files changed, 331 insertions(+), 319 deletions(-) create mode 100644 test/storage/dump/PropertyLoaderPageCountTest.cpp diff --git a/storage/dump/EmbeddingPropertyContainerLoader.cpp b/storage/dump/EmbeddingPropertyContainerLoader.cpp index 9348b030f0..f80ec1c31c 100644 --- a/storage/dump/EmbeddingPropertyContainerLoader.cpp +++ b/storage/dump/EmbeddingPropertyContainerLoader.cpp @@ -42,82 +42,70 @@ DumpResult> EmbeddingPropertyContainerLoader: const uint64_t nullCount = it.get(); const uint64_t nullPageCount = it.get(); - auto* container = new TypedPropertyContainer(dimension); - container->_ids.resize(propCount); - - // Loading IDs - size_t offset = 0; - for (size_t i = 0; i < idPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); + const size_t floatsPerPage = GraphDumpHelper::getCountPerPage(); + + uint64_t floatCount = 0; + const bool floatCountOverflows = __builtin_mul_overflow(propCount, dimension, &floatCount); + const bool headerInvalid = (dimension == 0) + || floatCountOverflows + || (GraphDumpHelper::getPageCountForItems(floatCount, floatsPerPage) != floatPageCount); + if (headerInvalid) { + return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); + } - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } + std::unique_ptr> container = std::make_unique>(dimension); + container->_ids.resize(propCount); - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - container->_ids[j + offset] = it.get(); - } - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, idPageCount, std::span {container->_ids}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } - // Loading float data — read flat floats and alloc into the container - std::vector embeddingBuffer(dimension); - size_t floatOffset = 0; + std::vector pageFloats(floatsPerPage); + std::vector partialEmbedding(dimension); + size_t partialCount = 0; + size_t embeddingCount = 0; for (size_t i = 0; i < floatPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); + size_t countInPage = 0; + if (auto res = GraphDumpHelper::loadPage(_reader, std::span {pageFloats}, countInPage, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - const size_t bufIdx = floatOffset % dimension; - embeddingBuffer[bufIdx] = it.get(); - floatOffset++; - - if (bufIdx == dimension - 1) { - container->_values.alloc(embeddingBuffer); + std::span remaining = std::span {pageFloats}.first(countInPage); + while (!remaining.empty()) { + if (embeddingCount == propCount) { + return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); } - } - } - // Loading null ids - container->_nullIds.resize(nullCount); - offset = 0; - - for (size_t i = 0; i < nullPageCount; i++) { - _reader.nextPage(); + const size_t taken = std::min(dimension - partialCount, remaining.size()); + const bool wholeInPage = (partialCount == 0 && taken == dimension); + + if (wholeInPage) { + container->_values.alloc(remaining.first(dimension)); + embeddingCount++; + } else { + std::copy_n(remaining.begin(), taken, partialEmbedding.begin() + partialCount); + partialCount += taken; + + if (partialCount == dimension) { + container->_values.alloc(partialEmbedding); + partialCount = 0; + embeddingCount++; + } + } - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); + remaining = remaining.subspan(taken); } + } - it = _reader.begin(); + if (embeddingCount != propCount) { + return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); + } - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } + container->_nullIds.resize(nullCount); - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - container->_nullIds[j + offset] = it.get(); - } - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, nullPageCount, std::span {container->_nullIds}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } // Reconstruct the entity ID -> index map. Nulls go in after the values, as sort() @@ -136,5 +124,5 @@ DumpResult> EmbeddingPropertyContainerLoader: container->_sorted = std::is_sorted(ids.begin(), ids.end()); - return {std::unique_ptr {container}}; + return {std::unique_ptr {container.release()}}; } diff --git a/storage/dump/GraphDumpHelper.h b/storage/dump/GraphDumpHelper.h index 3dbb0d4287..3c0fa1c02a 100644 --- a/storage/dump/GraphDumpHelper.h +++ b/storage/dump/GraphDumpHelper.h @@ -1,11 +1,15 @@ #pragma once +#include +#include #include #include +#include #include "ID.h" #include "DumpResult.h" #include "FilePageWriter.h" +#include "FilePageReader.h" #include "DumpConfig.h" #include "FileReader.h" #include "FileWriter.h" @@ -46,6 +50,65 @@ class GraphDumpHelper { } } + template + [[nodiscard]] static constexpr size_t getCountPerPage() { + return (DumpConfig::PAGE_SIZE - sizeof(uint64_t)) / sizeof(T); + } + + template + [[nodiscard]] static DumpResult loadPage(fs::FilePageReader& reader, + std::span out, + size_t& countInPage, + DumpErrorType errorType) { + static_assert(std::is_trivially_copyable_v); + + reader.nextPage(); + + if (reader.errorOccured()) { + return DumpError::result(errorType, reader.error().value()); + } + + fs::AlignedBufferIterator it = reader.begin(); + + if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { + return DumpError::result(errorType); + } + + const uint64_t count = it.get(); + const bool overflowsPage = count > getCountPerPage(); + const bool overflowsOut = count > out.size(); + if (overflowsPage || overflowsOut) { + return DumpError::result(errorType); + } + + std::copy_n(it.data(), count * sizeof(T), reinterpret_cast(out.data())); + countInPage = count; + + return {}; + } + + template + [[nodiscard]] static DumpResult loadPages(fs::FilePageReader& reader, + size_t pageCount, + std::span out, + DumpErrorType errorType) { + size_t offset = 0; + for (size_t i = 0; i < pageCount; i++) { + size_t countInPage = 0; + if (auto res = loadPage(reader, out.subspan(offset), countInPage, errorType); !res) { + return res; + } + + offset += countInPage; + } + + if (offset != out.size()) { + return DumpError::result(errorType); + } + + return {}; + } + static void writeFileHeader(fs::FilePageWriter& writer) { writer.writeToCurrentPage(DumpConfig::ONE_BAD_CAFE); writer.writeToCurrentPage(DumpConfig::VERSION); diff --git a/storage/dump/ListPropertyContainerLoader.cpp b/storage/dump/ListPropertyContainerLoader.cpp index da1c095ddd..3ecd3ebd4c 100644 --- a/storage/dump/ListPropertyContainerLoader.cpp +++ b/storage/dump/ListPropertyContainerLoader.cpp @@ -45,51 +45,17 @@ DumpResult> ListPropertyContainerLoader::load const uint64_t nullCount = it.get(); const uint64_t nullPageCount = it.get(); - auto* container = new TypedPropertyContainer; + std::unique_ptr> container = std::make_unique>(); container->_ids.resize(propCount); - size_t offset = 0; - for (size_t i = 0; i < idPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - container->_ids[j + offset] = it.get(); - } - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, idPageCount, std::span {container->_ids}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } std::vector stream(totalBytes); - size_t byteOffset = 0; - - for (size_t i = 0; i < bytePageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - it = _reader.begin(); - - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - stream[byteOffset + j] = std::byte {it.get()}; - } - byteOffset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, bytePageCount, std::span {stream}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } ListContainer& values = container->_values; @@ -115,28 +81,9 @@ DumpResult> ListPropertyContainerLoader::load } container->_nullIds.resize(nullCount); - offset = 0; - - for (size_t i = 0; i < nullPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_nullIds[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, nullPageCount, std::span {container->_nullIds}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } // Reconstruct the entity ID -> index map; it is not dumped. Nulls go in after the @@ -155,5 +102,5 @@ DumpResult> ListPropertyContainerLoader::load container->_sorted = std::is_sorted(ids.begin(), ids.end()); - return {std::unique_ptr {container}}; + return {std::unique_ptr {container.release()}}; } diff --git a/storage/dump/MapPropertyContainerLoader.cpp b/storage/dump/MapPropertyContainerLoader.cpp index b85fbb482b..358e718292 100644 --- a/storage/dump/MapPropertyContainerLoader.cpp +++ b/storage/dump/MapPropertyContainerLoader.cpp @@ -45,51 +45,17 @@ DumpResult> MapPropertyContainerLoader::load( const uint64_t nullCount = it.get(); const uint64_t nullPageCount = it.get(); - auto* container = new TypedPropertyContainer; + std::unique_ptr> container = std::make_unique>(); container->_ids.resize(propCount); - size_t offset = 0; - for (size_t i = 0; i < idPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - container->_ids[j + offset] = it.get(); - } - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, idPageCount, std::span {container->_ids}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } std::vector stream(totalBytes); - size_t byteOffset = 0; - - for (size_t i = 0; i < bytePageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - it = _reader.begin(); - - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - stream[byteOffset + j] = std::byte {it.get()}; - } - byteOffset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, bytePageCount, std::span {stream}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } MapContainer& values = container->_values; @@ -115,28 +81,9 @@ DumpResult> MapPropertyContainerLoader::load( } container->_nullIds.resize(nullCount); - offset = 0; - - for (size_t i = 0; i < nullPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_nullIds[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, nullPageCount, std::span {container->_nullIds}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } // Reconstruct the entity ID -> index map; it is not dumped. Nulls go in after the @@ -155,5 +102,5 @@ DumpResult> MapPropertyContainerLoader::load( container->_sorted = std::is_sorted(ids.begin(), ids.end()); - return {std::unique_ptr {container}}; + return {std::unique_ptr {container.release()}}; } diff --git a/storage/dump/PropertyContainerDumpConstants.h b/storage/dump/PropertyContainerDumpConstants.h index d1511e8340..ff2d7ad5f8 100644 --- a/storage/dump/PropertyContainerDumpConstants.h +++ b/storage/dump/PropertyContainerDumpConstants.h @@ -12,17 +12,17 @@ namespace db { -// The dumpers bulk-write property IDs and string limits as raw object bytes, -// while the loaders read them back as fixed-width fields. Pin the layouts the +// The dumpers write property IDs, values and string limits as raw object bytes, +// and the loaders copy those bytes back into the objects. Pin the layouts the // on-disk format depends on. static_assert(sizeof(EntityID) == sizeof(EntityID::Type), "Dump format writes EntityID object bytes as plain uint64 values"); static_assert(std::is_trivially_copyable_v, "Dump format writes EntityID object bytes as plain uint64 values"); static_assert(sizeof(StringBucket::StringLimits) == 2 * sizeof(uint32_t), "Dump format writes StringLimits object bytes as packed uint32 pairs"); -static_assert(offsetof(StringBucket::StringLimits, _offset) == 0, "Loaders read StringLimits as offset then count"); -static_assert(offsetof(StringBucket::StringLimits, _count) == sizeof(uint32_t), "Loaders read StringLimits as offset then count"); +static_assert(offsetof(StringBucket::StringLimits, _offset) == 0, "Dump format stores StringLimits as offset then count"); +static_assert(offsetof(StringBucket::StringLimits, _count) == sizeof(uint32_t), "Dump format stores StringLimits as offset then count"); static_assert(sizeof(Point) == 4 * sizeof(double), "Dump format writes a Point as x, y, z, then its CRS ordinal and padding"); static_assert(std::is_trivially_copyable_v, "Dump format writes Point object bytes"); -static_assert(std::is_trivially_default_constructible_v, "Loaders read Point values straight out of the mapped page"); +static_assert(std::is_trivially_default_constructible_v, "Loaders copy Point bytes into value-initialised Points"); template class TrivialPropertyContainerDumpConstants { diff --git a/storage/dump/PropertyContainerLoader.h b/storage/dump/PropertyContainerLoader.h index ecbb0063fc..2dbdf222c0 100644 --- a/storage/dump/PropertyContainerLoader.h +++ b/storage/dump/PropertyContainerLoader.h @@ -15,8 +15,6 @@ namespace db { template class TrivialPropertyContainerLoader { public: - using Constants = TrivialPropertyContainerDumpConstants; - explicit TrivialPropertyContainerLoader(fs::FilePageReader& reader) : _reader(reader) { @@ -51,87 +49,22 @@ class TrivialPropertyContainerLoader { const uint64_t nullCount = it.get(); const uint64_t nullPageCount = it.get(); - auto* container = new TypedPropertyContainer; + std::unique_ptr> container = std::make_unique>(); container->_ids.resize(propCount); container->_values.resize(propCount); - // Loading ids - size_t offset = 0; - - for (size_t i = 0; i < idPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - // Check that we read a whole page - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_ids[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, idPageCount, std::span {container->_ids}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } - // Loading values - offset = 0; - - for (size_t i = 0; i < valuePageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - // Check that we read a whole page - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_values[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, valuePageCount, std::span {container->_values}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } - // Loading null ids container->_nullIds.resize(nullCount); - offset = 0; - - for (size_t i = 0; i < nullPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - // Check that we read a whole page - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - for (size_t j = 0; j < countInPage; j++) { - container->_nullIds[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, nullPageCount, std::span {container->_nullIds}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } // Reconstruct the entity ID -> index map; it is not dumped. Nulls go in after the @@ -150,7 +83,7 @@ class TrivialPropertyContainerLoader { container->_sorted = std::is_sorted(ids.begin(), ids.end()); - return {std::unique_ptr {container}}; + return {std::unique_ptr {container.release()}}; } private: @@ -196,7 +129,7 @@ class StringPropertyContainerLoader { const uint64_t nullCount = it.get(); const uint64_t nullPageCount = it.get(); - auto* container = new TypedPropertyContainer; + std::unique_ptr> container = std::make_unique>(); container->_ids.resize(propCount); auto& buckets = container->_values; buckets.clear(); @@ -204,30 +137,8 @@ class StringPropertyContainerLoader { std::vector> rawBuckets; std::vector> rawLimits; - // Loading ids - size_t offset = 0; - - for (size_t i = 0; i < idPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - - it = _reader.begin(); - - // Check that we read a whole page - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_ids[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, idPageCount, std::span {container->_ids}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } // Loading buckets @@ -247,7 +158,7 @@ class StringPropertyContainerLoader { const size_t countInPage = it.get(); - if (countInPage * Constants::BUCKET_STRIDE > Constants::BUCKET_PAGE_AVAIL) { + if (countInPage > Constants::BUCKET_COUNT_PER_PAGE) { return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); } @@ -280,6 +191,10 @@ class StringPropertyContainerLoader { const size_t blockCountInPage = it.get(); for (size_t j = 0; j < blockCountInPage; j++) { + if (it.remainingBytes() < Constants::LIMIT_BLOCK_HEADER_STRIDE) { + return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); + } + const uint64_t bucketIndex = it.get(); const uint32_t blockStrCount = it.get(); @@ -287,44 +202,25 @@ class StringPropertyContainerLoader { return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); } + const size_t blockBytes = blockStrCount * Constants::LIMIT_STRIDE; + if (blockBytes > it.remainingBytes()) { + return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); + } + auto& limits = rawLimits[bucketIndex]; const size_t prevSize = limits.size(); limits.resize(prevSize + blockStrCount); - for (size_t k = 0; k < blockStrCount; k++) { - auto& lim = limits[k + prevSize]; - lim._offset = it.get(); - lim._count = it.get(); - } + std::copy_n(it.data(), blockBytes, reinterpret_cast(limits.data() + prevSize)); + it.advance(blockBytes); } } - // Loading null ids container->_nullIds.resize(nullCount); - offset = 0; - - for (size_t i = 0; i < nullPageCount; i++) { - _reader.nextPage(); - - if (_reader.errorOccured()) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS, _reader.error().value()); - } - it = _reader.begin(); - - // Check that we read a whole page - if (it.remainingBytes() != DumpConfig::PAGE_SIZE) { - return DumpError::result(DumpErrorType::COULD_NOT_READ_PROPS); - } - - const size_t countInPage = it.get(); - - for (size_t j = 0; j < countInPage; j++) { - container->_nullIds[j + offset] = it.get(); - } - - offset += countInPage; + if (auto res = GraphDumpHelper::loadPages(_reader, nullPageCount, std::span {container->_nullIds}, DumpErrorType::COULD_NOT_READ_PROPS); !res) { + return res.get_unexpected(); } for (size_t i = 0; i < rawBuckets.size(); i++) { @@ -349,7 +245,7 @@ class StringPropertyContainerLoader { container->_sorted = std::is_sorted(ids.begin(), ids.end()); - return {std::unique_ptr {container}}; + return {std::unique_ptr {container.release()}}; } private: diff --git a/test/storage/CMakeLists.txt b/test/storage/CMakeLists.txt index f5072b8100..b788640307 100644 --- a/test/storage/CMakeLists.txt +++ b/test/storage/CMakeLists.txt @@ -136,6 +136,7 @@ add_turing_test(test_storage_dump_point_graph dump/PointGraphDumpLoadTest.cpp) add_turing_test(test_storage_dump_map_graph dump/MapGraphDumpLoadTest.cpp) add_turing_test(test_storage_dump_map_datetime_graph dump/MapDateTimeGraphDumpLoadTest.cpp) add_turing_test(test_storage_dump_celltagdumpformat dump/CellTagDumpFormatTest.cpp) +add_turing_test(test_storage_dump_propertyloaderpagecount dump/PropertyLoaderPageCountTest.cpp) add_turing_test(test_storage_columns_dispatcher ColumnDispatcherTest.cpp) add_turing_test(test_storage_column_set_element ColumnSetElementTest.cpp) diff --git a/test/storage/dump/PropertyLoaderPageCountTest.cpp b/test/storage/dump/PropertyLoaderPageCountTest.cpp new file mode 100644 index 0000000000..7b44a6dff9 --- /dev/null +++ b/test/storage/dump/PropertyLoaderPageCountTest.cpp @@ -0,0 +1,170 @@ +#include +#include +#include + +#include "TuringTest.h" + +#include "dump/DumpConfig.h" +#include "dump/GraphDumpHelper.h" +#include "dump/PropertyContainerLoader.h" +#include "FilePageReader.h" +#include "FilePageWriter.h" + +using namespace db; +using namespace turing::test; + +namespace { + +struct DumpPage { + uint64_t _count {0}; + size_t _byteCount {0}; +}; + +void writePropertyDump(const fs::Path& path, + ValueType valueType, + std::span metadata, + std::span pages) { + auto writer = fs::FilePageWriter::open(path); + ASSERT_TRUE(writer); + fs::FilePageWriter& pageWriter = writer.value(); + + GraphDumpHelper::writeFileHeader(pageWriter); + pageWriter.writeToCurrentPage(valueType); + for (const uint64_t field : metadata) { + pageWriter.writeToCurrentPage(field); + } + + for (const DumpPage& page : pages) { + pageWriter.nextPage(); + pageWriter.writeToCurrentPage(page._count); + + const std::vector payload(page._byteCount, 0); + pageWriter.writeToCurrentPage(std::span {payload}); + } + + pageWriter.finish(); +} + +template +bool loads(const fs::Path& path) { + auto reader = fs::FilePageReader::open(path, DumpConfig::PAGE_SIZE); + if (!reader) { + return false; + } + + Loader loader(reader.value()); + return loader.load().has_value(); +} + +} + +class PropertyLoaderPageCountTest : public TuringTest { +}; + +TEST_F(PropertyLoaderPageCountTest, trivialIDPageOverflowsPropCount) { + const fs::Path path = fs::Path(_outDir.c_str()) / "ints"; + + // propCount, idPageCount, valuePageCount, nullCount, nullPageCount + const std::vector metadata {1, 1, 0, 0, 0}; + const std::vector pages {{2, 2 * sizeof(EntityID)}}; + writePropertyDump(path, ValueType::Int64, metadata, pages); + + EXPECT_FALSE(loads>(path)); +} + +TEST_F(PropertyLoaderPageCountTest, trivialPagesShortOfPropCount) { + const fs::Path path = fs::Path(_outDir.c_str()) / "ints"; + + const std::vector metadata {3, 1, 1, 0, 0}; + const std::vector pages {{3, 3 * sizeof(EntityID)}, {1, sizeof(int64_t)}}; + writePropertyDump(path, ValueType::Int64, metadata, pages); + + EXPECT_FALSE(loads>(path)); +} + +TEST_F(PropertyLoaderPageCountTest, trivialPageCountBeyondPage) { + const fs::Path path = fs::Path(_outDir.c_str()) / "ints"; + + const uint64_t countBeyondPage = DumpConfig::PAGE_SIZE; + const std::vector metadata {0, 0, 0, countBeyondPage, 1}; + const std::vector pages {{countBeyondPage, 0}}; + writePropertyDump(path, ValueType::Int64, metadata, pages); + + EXPECT_FALSE(loads>(path)); +} + +TEST_F(PropertyLoaderPageCountTest, stringNullPageOverflowsNullCount) { + const fs::Path path = fs::Path(_outDir.c_str()) / "strings"; + + // propCount, idPageCount, bucketPageCount, limitsPageCount, nullCount, nullPageCount + const std::vector metadata {0, 0, 0, 0, 1, 1}; + const std::vector pages {{2, 2 * sizeof(EntityID)}}; + writePropertyDump(path, ValueType::String, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, embeddingFloatPageOverflowsPropCount) { + const fs::Path path = fs::Path(_outDir.c_str()) / "embeddings"; + + // propCount, dimension, idPageCount, floatPageCount, nullCount, nullPageCount + const std::vector metadata {1, 2, 1, 1, 0, 0}; + const std::vector pages {{1, sizeof(EntityID)}, {4, 4 * sizeof(float)}}; + writePropertyDump(path, ValueType::Embedding, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, listBytePageOverflowsTotalBytes) { + const fs::Path path = fs::Path(_outDir.c_str()) / "lists"; + + // propCount, totalBytes, idPageCount, bytePageCount, nullCount, nullPageCount + const std::vector metadata {0, 4, 0, 1, 0, 0}; + const std::vector pages {{16, 16}}; + writePropertyDump(path, ValueType::List, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, mapBytePageOverflowsTotalBytes) { + const fs::Path path = fs::Path(_outDir.c_str()) / "maps"; + + const std::vector metadata {0, 4, 0, 1, 0, 0}; + const std::vector pages {{16, 16}}; + writePropertyDump(path, ValueType::Map, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, stringBucketPageCountBeyondPage) { + const fs::Path path = fs::Path(_outDir.c_str()) / "strings"; + + const uint64_t countWrappingByteSize = uint64_t {1} << 46; + const std::vector metadata {0, 0, 1, 0, 0, 0}; + const std::vector pages {{countWrappingByteSize, 0}}; + writePropertyDump(path, ValueType::String, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, stringLimitBlocksBeyondPage) { + const fs::Path path = fs::Path(_outDir.c_str()) / "strings"; + + const uint64_t blockCountBeyondPage = DumpConfig::PAGE_SIZE; + const std::vector metadata {0, 0, 1, 1, 0, 0}; + const std::vector pages {{1, StringBucket::BUCKET_SIZE}, {blockCountBeyondPage, 0}}; + writePropertyDump(path, ValueType::String, metadata, pages); + + EXPECT_FALSE(loads(path)); +} + +TEST_F(PropertyLoaderPageCountTest, embeddingFloatCountWrapping) { + const fs::Path path = fs::Path(_outDir.c_str()) / "embeddings"; + + const uint64_t dimensionWrappingFloatCount = uint64_t {1} << 63; + const std::vector metadata {2, dimensionWrappingFloatCount, 1, 0, 0, 0}; + const std::vector pages {{2, 2 * sizeof(EntityID)}}; + writePropertyDump(path, ValueType::Embedding, metadata, pages); + + EXPECT_FALSE(loads(path)); +}