From ecde0d3d2baad2b6efb36cca40d169f20581d2c0 Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Wed, 29 Jan 2025 02:34:08 +0000 Subject: [PATCH 01/13] messagePack draft --- .../clp/streaming_archive/ArchiveMetadata.cpp | 41 ++++++++------ .../clp/streaming_archive/ArchiveMetadata.hpp | 54 +++++++++++++++++-- .../clp/streaming_archive/reader/Archive.cpp | 2 +- 3 files changed, 74 insertions(+), 23 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 9cf578eb8..c987eb76f 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -1,4 +1,6 @@ #include "ArchiveMetadata.hpp" +#include +#include namespace clp::streaming_archive { ArchiveMetadata::ArchiveMetadata( @@ -22,15 +24,24 @@ ArchiveMetadata::ArchiveMetadata( + sizeof(m_end_timestamp) + sizeof(m_compressed_size); } -ArchiveMetadata::ArchiveMetadata(FileReader& file_reader) { - file_reader.read_numeric_value(m_archive_format_version, false); - file_reader.read_numeric_value(m_creator_id_len, false); - file_reader.read_string(m_creator_id_len, m_creator_id, false); - file_reader.read_numeric_value(m_creation_idx, false); - file_reader.read_numeric_value(m_uncompressed_size, false); - file_reader.read_numeric_value(m_compressed_size, false); - file_reader.read_numeric_value(m_begin_timestamp, false); - file_reader.read_numeric_value(m_end_timestamp, false); +auto ArchiveMetadata::create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata { + struct stat file_stat = {}; + auto clp_rc = file_reader.try_fstat(file_stat); + if (clp::ErrorCode::ErrorCode_Success != clp_rc) { + throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + } + + std::vector buf(file_stat.st_size); + clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); + if (clp::ErrorCode::ErrorCode_Success != clp_rc) { + throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + } + + ArchiveMetadata metadata; + msgpack::object_handle oh = msgpack::unpack(buf.data(), buf.size()); + msgpack::object obj = oh.get(); + obj.convert(metadata); + return metadata; } void ArchiveMetadata::expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp) { @@ -43,13 +54,9 @@ void ArchiveMetadata::expand_time_range(epochtime_t begin_timestamp, epochtime_t } void ArchiveMetadata::write_to_file(FileWriter& file_writer) const { - file_writer.write_numeric_value(m_archive_format_version); - file_writer.write_numeric_value(m_creator_id_len); - file_writer.write_string(m_creator_id); - file_writer.write_numeric_value(m_creation_idx); - file_writer.write_numeric_value(m_uncompressed_size + m_dynamic_uncompressed_size); - file_writer.write_numeric_value(m_compressed_size + m_dynamic_uncompressed_size); - file_writer.write_numeric_value(m_begin_timestamp); - file_writer.write_numeric_value(m_end_timestamp); + std::ostringstream buf; + msgpack::pack(buf, *this); + const auto& string_buf = buf.str(); + file_writer.write(string_buf.data(), string_buf.size()); } } // namespace clp::streaming_archive diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 1a1edc894..cc95fffcb 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -4,11 +4,16 @@ #include #include "../Defs.h" +#include "../ffi/encoding_methods.hpp" #include "../FileReader.hpp" #include "../FileWriter.hpp" #include "Constants.hpp" +#include "msgpack.hpp" namespace clp::streaming_archive { + +static constexpr std::string_view cCompressionTypeZstd = "ZSTD"; + /** * A class to encapsulate metadata directly relating to an archive. */ @@ -28,6 +33,7 @@ class ArchiveMetadata { }; // Constructors + ArchiveMetadata() = default; /** * Constructs a metadata object with the given parameters * @param archive_format_version @@ -40,13 +46,18 @@ class ArchiveMetadata { uint64_t creation_idx ); + // Methods /** - * Constructs a metadata object and initializes it from the given file reader - * @param file_reader - */ - explicit ArchiveMetadata(FileReader& file_reader); + * Reads serialized MessagePack data from `file_reader`, then unpacks it into an `ArchiveMetadata` instance. + * + * @param file_reader Reader for the file containing archive metadata. + * @return The created instance. + * @throw `ArchiveMetadata::OperationFailed` if stat or read for metadata file fails. + * @throw `msgpack::unpack_error` if data cannot be deserialized into MessagePack format. + * @throw `msgpack::type_error` if deserialized MessagePack data can't be converted to `ArchiveMetadata`. + */ + [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader)-> ArchiveMetadata; - // Methods [[nodiscard]] auto get_archive_format_version() const { return m_archive_format_version; } [[nodiscard]] auto get_creator_id() const -> std::string const& { return m_creator_id; } @@ -79,6 +90,18 @@ class ArchiveMetadata { [[nodiscard]] auto get_end_timestamp() const { return m_end_timestamp; } + [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string const& { + return m_variable_encoding_methods_version; + } + + [[nodiscard]] auto get_variables_schema_version() const -> std::string const& { + return m_variables_schema_version; + } + + [[nodiscard]] auto get_compression_type() const -> std::string const& { + return m_compression_type; + } + /** * Expands the archive's time range based to encompass the given time range * @param begin_timestamp @@ -86,8 +109,26 @@ class ArchiveMetadata { */ void expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp); + /** + * Serializes `ArchiveMetadata` instance to MessagePack and writes to `file_writer`. + * + * @param file_writer Writer for archive metadata file. + * @throw FileWriter::OperationFailed if failed to write. + */ void write_to_file(FileWriter& file_writer) const; + MSGPACK_DEFINE_MAP( + MSGPACK_NVP("archive_format_version", m_archive_format_version), + MSGPACK_NVP("variable_encoding_methods_version", m_variable_encoding_methods_version), + MSGPACK_NVP("variables_schema_version", m_variables_schema_version), + MSGPACK_NVP("compression_type", m_compression_type), + MSGPACK_NVP("creator_id", m_creator_id), + MSGPACK_NVP("begin_timestamp", m_begin_timestamp), + MSGPACK_NVP("end_timestamp", m_end_timestamp), + MSGPACK_NVP("uncompressed_size", m_uncompressed_size), + MSGPACK_NVP("compressed_size", m_compressed_size) + ); + private: // Variables archive_format_version_t m_archive_format_version{cArchiveFormatVersion}; @@ -102,6 +143,9 @@ class ArchiveMetadata { // The size of the archive uint64_t m_compressed_size{0}; uint64_t m_dynamic_compressed_size{0}; + std::string m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; + std::string m_variables_schema_version{ffi::cVariablesSchemaVersion}; + std::string m_compression_type{cCompressionTypeZstd}; }; } // namespace clp::streaming_archive diff --git a/components/core/src/clp/streaming_archive/reader/Archive.cpp b/components/core/src/clp/streaming_archive/reader/Archive.cpp index 6bfa16776..7b3b08dff 100644 --- a/components/core/src/clp/streaming_archive/reader/Archive.cpp +++ b/components/core/src/clp/streaming_archive/reader/Archive.cpp @@ -38,7 +38,7 @@ void Archive::open(string const& path) { archive_format_version_t format_version{}; try { FileReader file_reader{metadata_file_path}; - ArchiveMetadata const metadata{file_reader}; + auto const metadata = ArchiveMetadata::create_from_file_reader(file_reader); format_version = metadata.get_archive_format_version(); } catch (TraceableException& traceable_exception) { auto error_code = traceable_exception.get_error_code(); From a6974c7b976c724d8f16ab2439708ae524c9e879 Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Wed, 29 Jan 2025 02:35:35 +0000 Subject: [PATCH 02/13] lint --- .../clp/streaming_archive/ArchiveMetadata.cpp | 4 +++- .../clp/streaming_archive/ArchiveMetadata.hpp | 20 ++++++++++--------- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index c987eb76f..66be2bff7 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -1,5 +1,7 @@ #include "ArchiveMetadata.hpp" + #include + #include namespace clp::streaming_archive { @@ -56,7 +58,7 @@ void ArchiveMetadata::expand_time_range(epochtime_t begin_timestamp, epochtime_t void ArchiveMetadata::write_to_file(FileWriter& file_writer) const { std::ostringstream buf; msgpack::pack(buf, *this); - const auto& string_buf = buf.str(); + auto const& string_buf = buf.str(); file_writer.write(string_buf.data(), string_buf.size()); } } // namespace clp::streaming_archive diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index cc95fffcb..2fd2bf69d 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -48,15 +48,17 @@ class ArchiveMetadata { // Methods /** - * Reads serialized MessagePack data from `file_reader`, then unpacks it into an `ArchiveMetadata` instance. - * - * @param file_reader Reader for the file containing archive metadata. - * @return The created instance. - * @throw `ArchiveMetadata::OperationFailed` if stat or read for metadata file fails. - * @throw `msgpack::unpack_error` if data cannot be deserialized into MessagePack format. - * @throw `msgpack::type_error` if deserialized MessagePack data can't be converted to `ArchiveMetadata`. - */ - [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader)-> ArchiveMetadata; + * Reads serialized MessagePack data from `file_reader`, then unpacks it into an + * `ArchiveMetadata` instance. + * + * @param file_reader Reader for the file containing archive metadata. + * @return The created instance. + * @throw `ArchiveMetadata::OperationFailed` if stat or read for metadata file fails. + * @throw `msgpack::unpack_error` if data cannot be deserialized into MessagePack format. + * @throw `msgpack::type_error` if deserialized MessagePack data can't be converted to + * `ArchiveMetadata`. + */ + [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata; [[nodiscard]] auto get_archive_format_version() const { return m_archive_format_version; } From d3a52d4d55ce654a5b49daf432acdaa93730284e Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Wed, 29 Jan 2025 17:17:31 +0000 Subject: [PATCH 03/13] small changes? --- .../core/src/clp/streaming_archive/ArchiveMetadata.cpp | 2 +- .../core/src/clp/streaming_archive/ArchiveMetadata.hpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 66be2bff7..fe3b9f20d 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -27,7 +27,7 @@ ArchiveMetadata::ArchiveMetadata( } auto ArchiveMetadata::create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata { - struct stat file_stat = {}; + struct stat file_stat{}; auto clp_rc = file_reader.try_fstat(file_stat); if (clp::ErrorCode::ErrorCode_Success != clp_rc) { throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 2fd2bf69d..1a91e746e 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -48,14 +48,14 @@ class ArchiveMetadata { // Methods /** - * Reads serialized MessagePack data from `file_reader`, then unpacks it into an + * Reads serialized MessagePack data from open file, unpacks it into an * `ArchiveMetadata` instance. * * @param file_reader Reader for the file containing archive metadata. * @return The created instance. - * @throw `ArchiveMetadata::OperationFailed` if stat or read for metadata file fails. - * @throw `msgpack::unpack_error` if data cannot be deserialized into MessagePack format. - * @throw `msgpack::type_error` if deserialized MessagePack data can't be converted to + * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. + * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. + * @throw `msgpack::type_error` if MessagePack object can't be converted to * `ArchiveMetadata`. */ [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata; @@ -112,7 +112,7 @@ class ArchiveMetadata { void expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp); /** - * Serializes `ArchiveMetadata` instance to MessagePack and writes to `file_writer`. + * Packs `ArchiveMetadata` to MessagePack and writes to open file. * * @param file_writer Writer for archive metadata file. * @throw FileWriter::OperationFailed if failed to write. From 72452c1360624bae4ce12fea1b32709affcfcbee Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Thu, 30 Jan 2025 15:51:36 +0000 Subject: [PATCH 04/13] haiqi review --- .../clp/streaming_archive/ArchiveMetadata.cpp | 18 +++++++++------ .../clp/streaming_archive/ArchiveMetadata.hpp | 22 +++++++++++-------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index fe3b9f20d..eacead7a0 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -4,6 +4,8 @@ #include +#include "../Array.hpp" + namespace clp::streaming_archive { ArchiveMetadata::ArchiveMetadata( archive_format_version_t archive_format_version, @@ -28,15 +30,17 @@ ArchiveMetadata::ArchiveMetadata( auto ArchiveMetadata::create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata { struct stat file_stat{}; - auto clp_rc = file_reader.try_fstat(file_stat); - if (clp::ErrorCode::ErrorCode_Success != clp_rc) { - throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + if (auto const clp_rc = file_reader.try_fstat(file_stat); + clp::ErrorCode::ErrorCode_Success != clp_rc) + { + throw OperationFailed(clp_rc, __FILENAME__, __LINE__); } - std::vector buf(file_stat.st_size); - clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); - if (clp::ErrorCode::ErrorCode_Success != clp_rc) { - throw OperationFailed(ErrorCode_Failure, __FILENAME__, __LINE__); + clp::Array buf(file_stat.st_size); + if (auto const clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); + clp::ErrorCode::ErrorCode_Success != clp_rc) + { + throw OperationFailed(clp_rc, __FILENAME__, __LINE__); } ArchiveMetadata metadata; diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 1a91e746e..8ae16ebbf 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -2,6 +2,7 @@ #define CLP_STREAMING_ARCHIVE_ARCHIVEMETADATA_HPP #include +#include #include "../Defs.h" #include "../ffi/encoding_methods.hpp" @@ -33,7 +34,10 @@ class ArchiveMetadata { }; // Constructors + // The class must be constructible to convert from msgpack::object. + // https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_adaptor ArchiveMetadata() = default; + /** * Constructs a metadata object with the given parameters * @param archive_format_version @@ -55,8 +59,7 @@ class ArchiveMetadata { * @return The created instance. * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. - * @throw `msgpack::type_error` if MessagePack object can't be converted to - * `ArchiveMetadata`. + * @throw `msgpack::type_error` if MessagePack object can't be converted to `ArchiveMetadata`. */ [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata; @@ -92,15 +95,15 @@ class ArchiveMetadata { [[nodiscard]] auto get_end_timestamp() const { return m_end_timestamp; } - [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string const& { + [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string_view const& { return m_variable_encoding_methods_version; } - [[nodiscard]] auto get_variables_schema_version() const -> std::string const& { + [[nodiscard]] auto get_variables_schema_version() const -> std::string_view const& { return m_variables_schema_version; } - [[nodiscard]] auto get_compression_type() const -> std::string const& { + [[nodiscard]] auto get_compression_type() const -> std::string_view const& { return m_compression_type; } @@ -112,7 +115,7 @@ class ArchiveMetadata { void expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp); /** - * Packs `ArchiveMetadata` to MessagePack and writes to open file. + * Packs `ArchiveMetadata` to MessagePack and writes to the open file. * * @param file_writer Writer for archive metadata file. * @throw FileWriter::OperationFailed if failed to write. @@ -125,6 +128,7 @@ class ArchiveMetadata { MSGPACK_NVP("variables_schema_version", m_variables_schema_version), MSGPACK_NVP("compression_type", m_compression_type), MSGPACK_NVP("creator_id", m_creator_id), + MSGPACK_NVP("creation_idx", m_creation_idx), MSGPACK_NVP("begin_timestamp", m_begin_timestamp), MSGPACK_NVP("end_timestamp", m_end_timestamp), MSGPACK_NVP("uncompressed_size", m_uncompressed_size), @@ -145,9 +149,9 @@ class ArchiveMetadata { // The size of the archive uint64_t m_compressed_size{0}; uint64_t m_dynamic_compressed_size{0}; - std::string m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; - std::string m_variables_schema_version{ffi::cVariablesSchemaVersion}; - std::string m_compression_type{cCompressionTypeZstd}; + std::string_view m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; + std::string_view m_variables_schema_version{ffi::cVariablesSchemaVersion}; + std::string_view m_compression_type{cCompressionTypeZstd}; }; } // namespace clp::streaming_archive From aafee6e61f87512fa9339683bc989389f7f08c38 Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Thu, 30 Jan 2025 16:34:42 +0000 Subject: [PATCH 05/13] haiqi review --- .../core/src/clp/streaming_archive/ArchiveMetadata.cpp | 4 +++- .../core/src/clp/streaming_archive/ArchiveMetadata.hpp | 7 ++++--- .../core/src/clp/streaming_archive/reader/Archive.cpp | 3 +-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index eacead7a0..834c5adf3 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -5,6 +5,7 @@ #include #include "../Array.hpp" +#include "FileReader.hpp" namespace clp::streaming_archive { ArchiveMetadata::ArchiveMetadata( @@ -28,7 +29,8 @@ ArchiveMetadata::ArchiveMetadata( + sizeof(m_end_timestamp) + sizeof(m_compressed_size); } -auto ArchiveMetadata::create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata { +auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata { + auto file_reader = FileReader(std::string(path)); struct stat file_stat{}; if (auto const clp_rc = file_reader.try_fstat(file_stat); clp::ErrorCode::ErrorCode_Success != clp_rc) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 8ae16ebbf..7f8962bdb 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -34,7 +34,8 @@ class ArchiveMetadata { }; // Constructors - // The class must be constructible to convert from msgpack::object. + // The class must be default constructible to convert from msgpack::object in + // `create_from_file` method. // https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_adaptor ArchiveMetadata() = default; @@ -55,13 +56,13 @@ class ArchiveMetadata { * Reads serialized MessagePack data from open file, unpacks it into an * `ArchiveMetadata` instance. * - * @param file_reader Reader for the file containing archive metadata. + * @param path Path to archive metadata file. * @return The created instance. * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. * @throw `msgpack::type_error` if MessagePack object can't be converted to `ArchiveMetadata`. */ - [[nodiscard]] static auto create_from_file_reader(FileReader& file_reader) -> ArchiveMetadata; + [[nodiscard]] static auto create_from_file(std::string_view path) -> ArchiveMetadata; [[nodiscard]] auto get_archive_format_version() const { return m_archive_format_version; } diff --git a/components/core/src/clp/streaming_archive/reader/Archive.cpp b/components/core/src/clp/streaming_archive/reader/Archive.cpp index 7b3b08dff..2788b57ad 100644 --- a/components/core/src/clp/streaming_archive/reader/Archive.cpp +++ b/components/core/src/clp/streaming_archive/reader/Archive.cpp @@ -37,8 +37,7 @@ void Archive::open(string const& path) { string metadata_file_path = path + '/' + cMetadataFileName; archive_format_version_t format_version{}; try { - FileReader file_reader{metadata_file_path}; - auto const metadata = ArchiveMetadata::create_from_file_reader(file_reader); + auto const metadata = ArchiveMetadata::create_from_file(metadata_file_path); format_version = metadata.get_archive_format_version(); } catch (TraceableException& traceable_exception) { auto error_code = traceable_exception.get_error_code(); From f19a7ed8243e9e564d3f9bdbda355d82878ebcc5 Mon Sep 17 00:00:00 2001 From: davemarco <83603688+davemarco@users.noreply.github.com> Date: Mon, 3 Feb 2025 10:07:25 -0500 Subject: [PATCH 06/13] Update components/core/src/clp/streaming_archive/ArchiveMetadata.cpp Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- components/core/src/clp/streaming_archive/ArchiveMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 834c5adf3..595370179 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -30,7 +30,7 @@ ArchiveMetadata::ArchiveMetadata( } auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata { - auto file_reader = FileReader(std::string(path)); + FileReader file_reader{std::string(path)}; struct stat file_stat{}; if (auto const clp_rc = file_reader.try_fstat(file_stat); clp::ErrorCode::ErrorCode_Success != clp_rc) From 620ce228991387aae0580309362c599fa4389887 Mon Sep 17 00:00:00 2001 From: davemarco <83603688+davemarco@users.noreply.github.com> Date: Mon, 3 Feb 2025 10:13:12 -0500 Subject: [PATCH 07/13] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../clp/streaming_archive/ArchiveMetadata.hpp | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 7f8962bdb..0f827ff56 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -6,14 +6,12 @@ #include "../Defs.h" #include "../ffi/encoding_methods.hpp" -#include "../FileReader.hpp" #include "../FileWriter.hpp" #include "Constants.hpp" -#include "msgpack.hpp" +#include namespace clp::streaming_archive { - -static constexpr std::string_view cCompressionTypeZstd = "ZSTD"; +constexpr std::string_view cCompressionTypeZstd{"ZSTD"}; /** * A class to encapsulate metadata directly relating to an archive. @@ -34,8 +32,7 @@ class ArchiveMetadata { }; // Constructors - // The class must be default constructible to convert from msgpack::object in - // `create_from_file` method. + // We need a default constructor to convert from a msgpack::object in `create_from_file`. See // https://github.com/msgpack/msgpack-c/wiki/v2_0_cpp_adaptor ArchiveMetadata() = default; @@ -53,10 +50,10 @@ class ArchiveMetadata { // Methods /** - * Reads serialized MessagePack data from open file, unpacks it into an - * `ArchiveMetadata` instance. + * Reads serialized MessagePack data from a file and unpacks it into an `ArchiveMetadata` + * instance. * - * @param path Path to archive metadata file. + * @param path * @return The created instance. * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. @@ -116,9 +113,9 @@ class ArchiveMetadata { void expand_time_range(epochtime_t begin_timestamp, epochtime_t end_timestamp); /** - * Packs `ArchiveMetadata` to MessagePack and writes to the open file. + * Packs this instance into a MessagePack object and writes it to the open file. * - * @param file_writer Writer for archive metadata file. + * @param file_writer * @throw FileWriter::OperationFailed if failed to write. */ void write_to_file(FileWriter& file_writer) const; From fd7a4514b50f9dab982700216469009d0206e230 Mon Sep 17 00:00:00 2001 From: davemarco <83603688+davemarco@users.noreply.github.com> Date: Mon, 3 Feb 2025 10:14:49 -0500 Subject: [PATCH 08/13] Update components/core/src/clp/streaming_archive/ArchiveMetadata.cpp Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- components/core/src/clp/streaming_archive/ArchiveMetadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 595370179..73e5a4bc7 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -38,7 +38,7 @@ auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata throw OperationFailed(clp_rc, __FILENAME__, __LINE__); } - clp::Array buf(file_stat.st_size); + clp::Array buf{static_cast(file_stat.st_size)}; if (auto const clp_rc = file_reader.try_read_exact_length(buf.data(), buf.size()); clp::ErrorCode::ErrorCode_Success != clp_rc) { From fa67ad655dead48da05a6dd833c66cb9eadff72f Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Mon, 3 Feb 2025 15:26:23 +0000 Subject: [PATCH 09/13] small changes and remove new fields --- .../clp/streaming_archive/ArchiveMetadata.cpp | 6 ++--- .../clp/streaming_archive/ArchiveMetadata.hpp | 24 ++----------------- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 73e5a4bc7..cc5c8e484 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -30,7 +30,7 @@ ArchiveMetadata::ArchiveMetadata( } auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata { - FileReader file_reader{std::string(path)}; + FileReader file_reader{std::string(path)}; struct stat file_stat{}; if (auto const clp_rc = file_reader.try_fstat(file_stat); clp::ErrorCode::ErrorCode_Success != clp_rc) @@ -46,8 +46,8 @@ auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata } ArchiveMetadata metadata; - msgpack::object_handle oh = msgpack::unpack(buf.data(), buf.size()); - msgpack::object obj = oh.get(); + msgpack::object_handle const obj_handle = msgpack::unpack(buf.data(), buf.size()); + msgpack::object const obj = obj_handle.get(); obj.convert(metadata); return metadata; } diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 0f827ff56..4d6a847ab 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -4,14 +4,13 @@ #include #include +#include + #include "../Defs.h" -#include "../ffi/encoding_methods.hpp" #include "../FileWriter.hpp" #include "Constants.hpp" -#include namespace clp::streaming_archive { -constexpr std::string_view cCompressionTypeZstd{"ZSTD"}; /** * A class to encapsulate metadata directly relating to an archive. @@ -93,18 +92,6 @@ class ArchiveMetadata { [[nodiscard]] auto get_end_timestamp() const { return m_end_timestamp; } - [[nodiscard]] auto get_variable_encoding_methods_version() const -> std::string_view const& { - return m_variable_encoding_methods_version; - } - - [[nodiscard]] auto get_variables_schema_version() const -> std::string_view const& { - return m_variables_schema_version; - } - - [[nodiscard]] auto get_compression_type() const -> std::string_view const& { - return m_compression_type; - } - /** * Expands the archive's time range based to encompass the given time range * @param begin_timestamp @@ -116,15 +103,11 @@ class ArchiveMetadata { * Packs this instance into a MessagePack object and writes it to the open file. * * @param file_writer - * @throw FileWriter::OperationFailed if failed to write. */ void write_to_file(FileWriter& file_writer) const; MSGPACK_DEFINE_MAP( MSGPACK_NVP("archive_format_version", m_archive_format_version), - MSGPACK_NVP("variable_encoding_methods_version", m_variable_encoding_methods_version), - MSGPACK_NVP("variables_schema_version", m_variables_schema_version), - MSGPACK_NVP("compression_type", m_compression_type), MSGPACK_NVP("creator_id", m_creator_id), MSGPACK_NVP("creation_idx", m_creation_idx), MSGPACK_NVP("begin_timestamp", m_begin_timestamp), @@ -147,9 +130,6 @@ class ArchiveMetadata { // The size of the archive uint64_t m_compressed_size{0}; uint64_t m_dynamic_compressed_size{0}; - std::string_view m_variable_encoding_methods_version{ffi::cVariableEncodingMethodsVersion}; - std::string_view m_variables_schema_version{ffi::cVariablesSchemaVersion}; - std::string_view m_compression_type{cCompressionTypeZstd}; }; } // namespace clp::streaming_archive From 13d5fdfd6621597f30d43a4932b2f83c6a39f160 Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Mon, 3 Feb 2025 15:45:22 +0000 Subject: [PATCH 10/13] small changes --- components/core/src/clp/streaming_archive/ArchiveMetadata.cpp | 4 ++-- components/core/src/clp/streaming_archive/ArchiveMetadata.hpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index cc5c8e484..8f0941b03 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -29,8 +29,8 @@ ArchiveMetadata::ArchiveMetadata( + sizeof(m_end_timestamp) + sizeof(m_compressed_size); } -auto ArchiveMetadata::create_from_file(std::string_view path) -> ArchiveMetadata { - FileReader file_reader{std::string(path)}; +auto ArchiveMetadata::create_from_file(std::string_view file_path) -> ArchiveMetadata { + FileReader file_reader{std::string(file_path)}; struct stat file_stat{}; if (auto const clp_rc = file_reader.try_fstat(file_stat); clp::ErrorCode::ErrorCode_Success != clp_rc) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 4d6a847ab..94ccc0da3 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -52,13 +52,13 @@ class ArchiveMetadata { * Reads serialized MessagePack data from a file and unpacks it into an `ArchiveMetadata` * instance. * - * @param path + * @param file_path * @return The created instance. * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. * @throw `msgpack::type_error` if MessagePack object can't be converted to `ArchiveMetadata`. */ - [[nodiscard]] static auto create_from_file(std::string_view path) -> ArchiveMetadata; + [[nodiscard]] static auto create_from_file(std::string_view file_path) -> ArchiveMetadata; [[nodiscard]] auto get_archive_format_version() const { return m_archive_format_version; } From 202c4ee7189ec818eae4e90c843dd0b5139e016e Mon Sep 17 00:00:00 2001 From: davemarco <83603688+davemarco@users.noreply.github.com> Date: Tue, 4 Feb 2025 11:51:09 -0500 Subject: [PATCH 11/13] Apply suggestions from code review Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com> --- .../core/src/clp/streaming_archive/ArchiveMetadata.cpp | 2 +- .../core/src/clp/streaming_archive/ArchiveMetadata.hpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 8f0941b03..4527bcc87 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -5,7 +5,7 @@ #include #include "../Array.hpp" -#include "FileReader.hpp" +#include "../FileReader.hpp" namespace clp::streaming_archive { ArchiveMetadata::ArchiveMetadata( diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 31f75f597..44f3c47c9 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -11,7 +11,6 @@ #include "Constants.hpp" namespace clp::streaming_archive { - /** * A class to encapsulate metadata directly relating to an archive. */ @@ -54,9 +53,10 @@ class ArchiveMetadata { * * @param file_path * @return The created instance. - * @throw `ArchiveMetadata::OperationFailed` if stat or read operation on metadata file fails. - * @throw `msgpack::unpack_error` if data cannot be unpacked into MessagePack object. - * @throw `msgpack::type_error` if MessagePack object can't be converted to `ArchiveMetadata`. + * @throw `ArchiveMetadata::OperationFailed` if `stat` fails or the file couldn't be read. + * @throw `msgpack::unpack_error` if the data cannot be unpacked into a MessagePack object. + * @throw `msgpack::type_error` if the MessagePack object can't be converted to an + * `ArchiveMetadata` instance. */ [[nodiscard]] static auto create_from_file(std::string_view file_path) -> ArchiveMetadata; From a885dc80d1bef1ee09c882bc13e277427c210d8f Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Tue, 4 Feb 2025 22:10:16 +0000 Subject: [PATCH 12/13] fix metadata size --- .../clp/streaming_archive/ArchiveMetadata.cpp | 23 +++++++------------ .../clp/streaming_archive/ArchiveMetadata.hpp | 4 ++-- 2 files changed, 10 insertions(+), 17 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 4527bcc87..7b6e119c5 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -15,19 +15,7 @@ ArchiveMetadata::ArchiveMetadata( ) : m_archive_format_version(archive_format_version), m_creator_id(std::move(creator_id)), - m_creation_idx(creation_idx) { - if (m_creator_id.length() > UINT16_MAX) { - throw OperationFailed(ErrorCode_BadParam, __FILENAME__, __LINE__); - } - m_creator_id_len = m_creator_id.length(); - - // NOTE: We set this to the size of this metadata on disk; when adding new members that will be - // written to disk, you must update this - m_compressed_size += sizeof(m_archive_format_version) + sizeof(m_creator_id_len) - + m_creator_id.length() + sizeof(m_creation_idx) - + sizeof(m_uncompressed_size) + sizeof(m_begin_timestamp) - + sizeof(m_end_timestamp) + sizeof(m_compressed_size); -} + m_creation_idx(creation_idx) {} auto ArchiveMetadata::create_from_file(std::string_view file_path) -> ArchiveMetadata { FileReader file_reader{std::string(file_path)}; @@ -61,10 +49,15 @@ void ArchiveMetadata::expand_time_range(epochtime_t begin_timestamp, epochtime_t } } -void ArchiveMetadata::write_to_file(FileWriter& file_writer) const { +void ArchiveMetadata::write_to_file(FileWriter& file_writer) { std::ostringstream buf; msgpack::pack(buf, *this); auto const& string_buf = buf.str(); - file_writer.write(string_buf.data(), string_buf.size()); + + size_t previous_metadata_size = m_metadata_size; + m_metadata_size = string_buf.size(); + file_writer.write(string_buf.data(), m_metadata_size); + + m_compressed_size = m_compressed_size - previous_metadata_size + m_metadata_size; } } // namespace clp::streaming_archive diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 44f3c47c9..5b08cabf2 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -104,7 +104,7 @@ class ArchiveMetadata { * * @param file_writer */ - void write_to_file(FileWriter& file_writer) const; + void write_to_file(FileWriter& file_writer); MSGPACK_DEFINE_MAP( MSGPACK_NVP("archive_format_version", m_archive_format_version), @@ -120,7 +120,6 @@ class ArchiveMetadata { // Variables archive_format_version_t m_archive_format_version{cArchiveFormatVersion::Version}; std::string m_creator_id; - uint16_t m_creator_id_len{0}; uint64_t m_creation_idx{0}; epochtime_t m_begin_timestamp{cEpochTimeMax}; epochtime_t m_end_timestamp{cEpochTimeMin}; @@ -129,6 +128,7 @@ class ArchiveMetadata { uint64_t m_dynamic_uncompressed_size{0}; // The size of the archive uint64_t m_compressed_size{0}; + uint64_t m_metadata_size{0}; uint64_t m_dynamic_compressed_size{0}; }; } // namespace clp::streaming_archive From fc00b4aeb289fd217851a12c15faa30f6d0eb486 Mon Sep 17 00:00:00 2001 From: Dave Marco Date: Wed, 5 Feb 2025 18:39:05 +0000 Subject: [PATCH 13/13] remove metadata size --- .../core/src/clp/streaming_archive/ArchiveMetadata.cpp | 9 ++------- .../core/src/clp/streaming_archive/ArchiveMetadata.hpp | 3 +-- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp index 7b6e119c5..2e777b49e 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.cpp @@ -49,15 +49,10 @@ void ArchiveMetadata::expand_time_range(epochtime_t begin_timestamp, epochtime_t } } -void ArchiveMetadata::write_to_file(FileWriter& file_writer) { +void ArchiveMetadata::write_to_file(FileWriter& file_writer) const { std::ostringstream buf; msgpack::pack(buf, *this); auto const& string_buf = buf.str(); - - size_t previous_metadata_size = m_metadata_size; - m_metadata_size = string_buf.size(); - file_writer.write(string_buf.data(), m_metadata_size); - - m_compressed_size = m_compressed_size - previous_metadata_size + m_metadata_size; + file_writer.write(string_buf.data(), string_buf.size()); } } // namespace clp::streaming_archive diff --git a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp index 5b08cabf2..dc417432c 100644 --- a/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp +++ b/components/core/src/clp/streaming_archive/ArchiveMetadata.hpp @@ -104,7 +104,7 @@ class ArchiveMetadata { * * @param file_writer */ - void write_to_file(FileWriter& file_writer); + void write_to_file(FileWriter& file_writer) const; MSGPACK_DEFINE_MAP( MSGPACK_NVP("archive_format_version", m_archive_format_version), @@ -128,7 +128,6 @@ class ArchiveMetadata { uint64_t m_dynamic_uncompressed_size{0}; // The size of the archive uint64_t m_compressed_size{0}; - uint64_t m_metadata_size{0}; uint64_t m_dynamic_compressed_size{0}; }; } // namespace clp::streaming_archive