From 6894a998c3672f25a6a91b08e84e6038d7820b32 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 2 Jul 2024 15:30:11 +0000 Subject: [PATCH 01/21] Implement compression side of table packing --- components/core/src/clp_s/ArchiveWriter.cpp | 73 ++++++++++++++++--- components/core/src/clp_s/ArchiveWriter.hpp | 2 + components/core/src/clp_s/ColumnWriter.cpp | 18 ++--- components/core/src/clp_s/ColumnWriter.hpp | 26 +++++-- .../core/src/clp_s/CommandLineArguments.cpp | 5 ++ .../core/src/clp_s/CommandLineArguments.hpp | 3 + components/core/src/clp_s/JsonParser.cpp | 1 + components/core/src/clp_s/JsonParser.hpp | 1 + components/core/src/clp_s/SchemaWriter.cpp | 8 +- components/core/src/clp_s/SchemaWriter.hpp | 13 ++-- components/core/src/clp_s/clp-s.cpp | 1 + 11 files changed, 111 insertions(+), 40 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index ba540a79d..eac6a2011 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -1,5 +1,8 @@ #include "ArchiveWriter.hpp" +#include +#include + #include #include "archive_constants.hpp" @@ -11,6 +14,7 @@ void ArchiveWriter::open(ArchiveWriterOption const& option) { m_id = boost::uuids::to_string(option.id); m_compression_level = option.compression_level; m_print_archive_stats = option.print_archive_stats; + m_min_table_size = option.min_table_size; auto archive_path = boost::filesystem::path(option.archives_dir) / m_id; boost::system::error_code boost_error_code; @@ -138,19 +142,70 @@ size_t ArchiveWriter::store_tables() { FileWriter::OpenMode::CreateForWriting ); m_table_metadata_compressor.open(m_table_metadata_file_writer, m_compression_level); - m_table_metadata_compressor.write_numeric_value(m_id_to_schema_writer.size()); - for (auto& i : m_id_to_schema_writer) { - m_table_metadata_compressor.write_numeric_value(i.first); - m_table_metadata_compressor.write_numeric_value(i.second->get_num_messages()); - m_table_metadata_compressor.write_numeric_value(m_tables_file_writer.get_pos()); - m_tables_compressor.open(m_tables_file_writer, m_compression_level); - size_t uncompressed_size = i.second->store(m_tables_compressor); - m_tables_compressor.close(); - delete i.second; + using schema_map_it = decltype(m_id_to_schema_writer)::iterator; + std::vector schemas; + std::vector> table_metadata; + std::vector> schema_metadata; + schema_metadata.reserve(m_id_to_schema_writer.size()); + schemas.reserve(m_id_to_schema_writer.size()); + auto it = m_id_to_schema_writer.begin(); + for (size_t i = 0; i < m_id_to_schema_writer.size(); ++i) { + schemas.push_back(it++); + } + auto comp = [](schema_map_it const& lhs, schema_map_it const& rhs) -> bool { + return lhs->second->get_total_uncompressed_size() + > rhs->second->get_total_uncompressed_size(); + }; + std::sort(schemas.begin(), schemas.end(), comp); + + size_t current_table_size = 0; + size_t current_table_id = 0; + size_t current_table_file_offset = 0; + m_tables_compressor.open(m_tables_file_writer, m_compression_level); + for (auto it : schemas) { + it->second->store(m_tables_compressor); + schema_metadata.emplace_back( + it->first, + it->second->get_num_messages(), + current_table_id, + current_table_size + ); + current_table_size += it->second->get_total_uncompressed_size(); + delete it->second; + + if (current_table_size > m_min_table_size || schemas.size() == schema_metadata.size()) { + table_metadata.emplace_back(current_table_file_offset, current_table_size); + m_tables_compressor.close(); + current_table_size = 0; + ++current_table_id; + current_table_file_offset = m_tables_file_writer.get_pos(); + + if (schemas.size() != schema_metadata.size()) { + m_tables_compressor.open(m_tables_file_writer, m_compression_level); + } + } + } + + // table metadata schema + // # num tables <64 bit> + // # [offset into file <64 bit> uncompressed size <64 bit>]+ + // # num schemas <64 bit> + // # [table id <64 bit> offset into table <64 bit> schema id <32 bit> num messages <64 bit>]+ + m_table_metadata_compressor.write_numeric_value(table_metadata.size()); + for (auto& [file_offset, uncompressed_size] : table_metadata) { + m_table_metadata_compressor.write_numeric_value(file_offset); m_table_metadata_compressor.write_numeric_value(uncompressed_size); } + + m_table_metadata_compressor.write_numeric_value(schema_metadata.size()); + for (auto& [schema_id, num_messages, table_id, table_offset] : schema_metadata) { + m_table_metadata_compressor.write_numeric_value(table_id); + m_table_metadata_compressor.write_numeric_value(table_offset); + m_table_metadata_compressor.write_numeric_value(schema_id); + m_table_metadata_compressor.write_numeric_value(num_messages); + } m_table_metadata_compressor.close(); compressed_size += m_table_metadata_file_writer.get_pos(); diff --git a/components/core/src/clp_s/ArchiveWriter.hpp b/components/core/src/clp_s/ArchiveWriter.hpp index 70eb5dc9b..489a27254 100644 --- a/components/core/src/clp_s/ArchiveWriter.hpp +++ b/components/core/src/clp_s/ArchiveWriter.hpp @@ -21,6 +21,7 @@ struct ArchiveWriterOption { std::string archives_dir; int compression_level; bool print_archive_stats; + size_t min_table_size; }; class ArchiveWriter { @@ -159,6 +160,7 @@ class ArchiveWriter { std::shared_ptr m_metadata_db; int m_compression_level{}; bool m_print_archive_stats{}; + size_t m_min_table_size{}; SchemaMap m_schema_map; SchemaTree m_schema_tree; diff --git a/components/core/src/clp_s/ColumnWriter.cpp b/components/core/src/clp_s/ColumnWriter.cpp index 183f17e57..9c40345b3 100644 --- a/components/core/src/clp_s/ColumnWriter.cpp +++ b/components/core/src/clp_s/ColumnWriter.cpp @@ -6,10 +6,9 @@ void Int64ColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size m_values.push_back(std::get(value)); } -size_t Int64ColumnWriter::store(ZstdCompressor& compressor) { +void Int64ColumnWriter::store(ZstdCompressor& compressor) { size_t size = m_values.size() * sizeof(int64_t); compressor.write(reinterpret_cast(m_values.data()), size); - return size; } void FloatColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { @@ -17,10 +16,9 @@ void FloatColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size m_values.push_back(std::get(value)); } -size_t FloatColumnWriter::store(ZstdCompressor& compressor) { +void FloatColumnWriter::store(ZstdCompressor& compressor) { size_t size = m_values.size() * sizeof(double); compressor.write(reinterpret_cast(m_values.data()), size); - return size; } void BooleanColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { @@ -28,10 +26,9 @@ void BooleanColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& si m_values.push_back(std::get(value) ? 1 : 0); } -size_t BooleanColumnWriter::store(ZstdCompressor& compressor) { +void BooleanColumnWriter::store(ZstdCompressor& compressor) { size_t size = m_values.size() * sizeof(uint8_t); compressor.write(reinterpret_cast(m_values.data()), size); - return size; } void ClpStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { @@ -51,14 +48,13 @@ void ClpStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size += sizeof(int64_t) * (m_encoded_vars.size() - offset); } -size_t ClpStringColumnWriter::store(ZstdCompressor& compressor) { +void ClpStringColumnWriter::store(ZstdCompressor& compressor) { size_t logtypes_size = m_logtypes.size() * sizeof(int64_t); compressor.write(reinterpret_cast(m_logtypes.data()), logtypes_size); size_t encoded_vars_size = m_encoded_vars.size() * sizeof(int64_t); size_t num_encoded_vars = m_encoded_vars.size(); compressor.write_numeric_value(num_encoded_vars); compressor.write(reinterpret_cast(m_encoded_vars.data()), encoded_vars_size); - return logtypes_size + sizeof(num_encoded_vars) + encoded_vars_size; } void VariableStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { @@ -69,10 +65,9 @@ void VariableStringColumnWriter::add_value(ParsedMessage::variable_t& value, siz m_variables.push_back(id); } -size_t VariableStringColumnWriter::store(ZstdCompressor& compressor) { +void VariableStringColumnWriter::store(ZstdCompressor& compressor) { size_t size = m_variables.size() * sizeof(int64_t); compressor.write(reinterpret_cast(m_variables.data()), size); - return size; } void DateStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { @@ -82,11 +77,10 @@ void DateStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& m_timestamp_encodings.push_back(encoded_timestamp.first); } -size_t DateStringColumnWriter::store(ZstdCompressor& compressor) { +void DateStringColumnWriter::store(ZstdCompressor& compressor) { size_t timestamps_size = m_timestamps.size() * sizeof(int64_t); compressor.write(reinterpret_cast(m_timestamps.data()), timestamps_size); size_t encodings_size = m_timestamp_encodings.size() * sizeof(int64_t); compressor.write(reinterpret_cast(m_timestamp_encodings.data()), encodings_size); - return timestamps_size + encodings_size; } } // namespace clp_s diff --git a/components/core/src/clp_s/ColumnWriter.hpp b/components/core/src/clp_s/ColumnWriter.hpp index ce458381e..37259b15b 100644 --- a/components/core/src/clp_s/ColumnWriter.hpp +++ b/components/core/src/clp_s/ColumnWriter.hpp @@ -34,9 +34,17 @@ class BaseColumnWriter { /** * Stores the column to a compressed file. * @param compressor - * @return the in-memory uncompressed size of the data written to the compressor */ - virtual size_t store(ZstdCompressor& compressor) = 0; + virtual void store(ZstdCompressor& compressor) = 0; + + /** + * Returns the total size of the header data that will be written to the compressor. This header + * size plus the sum of sizes returned by add_value is equal to the total size of data that will + * be written to the compressor in bytes. + * + * @return the total size of header data that will + */ + virtual size_t get_total_header_size() const { return 0; } protected: int32_t m_id; @@ -53,7 +61,7 @@ class Int64ColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; private: std::vector m_values; @@ -70,7 +78,7 @@ class FloatColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; private: std::vector m_values; @@ -87,7 +95,7 @@ class BooleanColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; private: std::vector m_values; @@ -111,7 +119,9 @@ class ClpStringColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; + + size_t get_total_header_size() const override { return sizeof(size_t); } /** * @param encoded_id @@ -165,7 +175,7 @@ class VariableStringColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; private: std::shared_ptr m_var_dict; @@ -183,7 +193,7 @@ class DateStringColumnWriter : public BaseColumnWriter { // Methods inherited from BaseColumnWriter void add_value(ParsedMessage::variable_t& value, size_t& size) override; - size_t store(ZstdCompressor& compressor) override; + void store(ZstdCompressor& compressor) override; private: std::vector m_timestamps; diff --git a/components/core/src/clp_s/CommandLineArguments.cpp b/components/core/src/clp_s/CommandLineArguments.cpp index 77e896160..31fe728ca 100644 --- a/components/core/src/clp_s/CommandLineArguments.cpp +++ b/components/core/src/clp_s/CommandLineArguments.cpp @@ -160,6 +160,11 @@ CommandLineArguments::parse_arguments(int argc, char const** argv) { default_value(m_target_encoded_size), "Target size (B) for the dictionaries and encoded messages before a new " "archive is created." + )( + "min-table-size", + po::value(&m_minimum_table_size)->value_name("MIN_TABLE_SIZE")-> + default_value(m_minimum_table_size), + "Minimum size (B) for a packed table before it gets compressed." )( "max-document-size", po::value(&m_max_document_size)->value_name("DOC_SIZE")-> diff --git a/components/core/src/clp_s/CommandLineArguments.hpp b/components/core/src/clp_s/CommandLineArguments.hpp index 0f3d8c556..d86c35303 100644 --- a/components/core/src/clp_s/CommandLineArguments.hpp +++ b/components/core/src/clp_s/CommandLineArguments.hpp @@ -108,6 +108,8 @@ class CommandLineArguments { size_t get_ordered_chunk_size() const { return m_ordered_chunk_size; } + size_t get_min_table_size() const { return m_minimum_table_size; } + private: // Methods /** @@ -173,6 +175,7 @@ class CommandLineArguments { bool m_structurize_arrays{false}; bool m_ordered_decompression{false}; size_t m_ordered_chunk_size{0}; + size_t m_minimum_table_size{1ULL * 1024 * 1024}; // 1 MB // Metadata db variables std::optional m_metadata_db_config; diff --git a/components/core/src/clp_s/JsonParser.cpp b/components/core/src/clp_s/JsonParser.cpp index d73643a64..d0b4dea45 100644 --- a/components/core/src/clp_s/JsonParser.cpp +++ b/components/core/src/clp_s/JsonParser.cpp @@ -31,6 +31,7 @@ JsonParser::JsonParser(JsonParserOption const& option) m_archive_options.archives_dir = option.archives_dir; m_archive_options.compression_level = option.compression_level; m_archive_options.print_archive_stats = option.print_archive_stats; + m_archive_options.min_table_size = option.min_table_size; m_archive_options.id = m_generator(); m_archive_writer = std::make_unique(option.metadata_db); diff --git a/components/core/src/clp_s/JsonParser.hpp b/components/core/src/clp_s/JsonParser.hpp index f65f4c4b1..08b3af86b 100644 --- a/components/core/src/clp_s/JsonParser.hpp +++ b/components/core/src/clp_s/JsonParser.hpp @@ -32,6 +32,7 @@ struct JsonParserOption { std::string archives_dir; size_t target_encoded_size; size_t max_document_size; + size_t min_table_size; int compression_level; bool print_archive_stats; bool structurize_arrays; diff --git a/components/core/src/clp_s/SchemaWriter.cpp b/components/core/src/clp_s/SchemaWriter.cpp index b6de15f2f..d3c5d8165 100644 --- a/components/core/src/clp_s/SchemaWriter.cpp +++ b/components/core/src/clp_s/SchemaWriter.cpp @@ -4,6 +4,7 @@ namespace clp_s { void SchemaWriter::append_column(BaseColumnWriter* column_writer) { + m_total_uncompressed_size += column_writer->get_total_header_size(); m_columns.push_back(column_writer); } @@ -24,15 +25,14 @@ size_t SchemaWriter::append_message(ParsedMessage& message) { } m_num_messages++; + m_total_uncompressed_size += total_size; return total_size; } -size_t SchemaWriter::store(ZstdCompressor& compressor) { - size_t total_size = 0; +void SchemaWriter::store(ZstdCompressor& compressor) { for (auto& writer : m_columns) { - total_size += writer->store(compressor); + writer->store(compressor); } - return total_size; } SchemaWriter::~SchemaWriter() { diff --git a/components/core/src/clp_s/SchemaWriter.hpp b/components/core/src/clp_s/SchemaWriter.hpp index 41b28600b..4f204d949 100644 --- a/components/core/src/clp_s/SchemaWriter.hpp +++ b/components/core/src/clp_s/SchemaWriter.hpp @@ -40,20 +40,19 @@ class SchemaWriter { /** * Stores the columns to disk. * @param compressor - * @return the uncompressed in-memory size of the table */ - [[nodiscard]] size_t store(ZstdCompressor& compressor); + void store(ZstdCompressor& compressor); + + uint64_t get_num_messages() const { return m_num_messages; } /** - * Closes the schema writer. - * @return the compressed size of the schema table in bytes + * @return the uncompressed in-memory size of the data that will be written to the compressor */ - [[nodiscard]] size_t close(); - - uint64_t get_num_messages() const { return m_num_messages; } + size_t get_total_uncompressed_size() const { return m_total_uncompressed_size; } private: uint64_t m_num_messages; + size_t m_total_uncompressed_size{}; std::vector m_columns; std::vector m_unordered_columns; diff --git a/components/core/src/clp_s/clp-s.cpp b/components/core/src/clp_s/clp-s.cpp index d01ed0fe0..a21add4a2 100644 --- a/components/core/src/clp_s/clp-s.cpp +++ b/components/core/src/clp_s/clp-s.cpp @@ -89,6 +89,7 @@ bool compress(CommandLineArguments const& command_line_arguments) { option.archives_dir = archives_dir.string(); option.target_encoded_size = command_line_arguments.get_target_encoded_size(); option.max_document_size = command_line_arguments.get_max_document_size(); + option.min_table_size = command_line_arguments.get_min_table_size(); option.compression_level = command_line_arguments.get_compression_level(); option.timestamp_key = command_line_arguments.get_timestamp_key(); option.print_archive_stats = command_line_arguments.print_archive_stats(); From 19590b17cac3cab59f9a068ab68e855a89cdb1b5 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 2 Jul 2024 15:34:30 +0000 Subject: [PATCH 02/21] Implement decompression side of table packing --- components/core/src/clp_s/ArchiveReader.cpp | 114 +++++++++++++------- components/core/src/clp_s/ArchiveReader.hpp | 24 ++++- components/core/src/clp_s/CMakeLists.txt | 2 + components/core/src/clp_s/SchemaReader.cpp | 18 ++-- components/core/src/clp_s/SchemaReader.hpp | 19 ++-- components/core/src/clp_s/TableReader.cpp | 108 +++++++++++++++++++ components/core/src/clp_s/TableReader.hpp | 89 +++++++++++++++ components/core/src/clp_s/search/Output.cpp | 2 +- 8 files changed, 312 insertions(+), 64 deletions(-) create mode 100644 components/core/src/clp_s/TableReader.cpp create mode 100644 components/core/src/clp_s/TableReader.hpp diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index f211a0707..3c62cc63b 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -27,8 +27,8 @@ void ArchiveReader::open(string_view archives_dir, string_view archive_id) { m_schema_tree = ReaderUtils::read_schema_tree(archive_path_str); m_schema_map = ReaderUtils::read_schemas(archive_path_str); - m_tables_file_reader.open(archive_path_str + constants::cArchiveTablesFile); m_table_metadata_file_reader.open(archive_path_str + constants::cArchiveTableMetadataFile); + m_table_reader.open_tables(archive_path_str + constants::cArchiveTablesFile); } void ArchiveReader::read_metadata() { @@ -38,6 +38,8 @@ void ArchiveReader::read_metadata() { cDecompressorFileReadBufferCapacity ); + m_table_reader.read_metadata(m_table_metadata_decompressor); + size_t num_schemas; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_schemas); ErrorCodeSuccess != error) @@ -45,39 +47,65 @@ void ArchiveReader::read_metadata() { throw OperationFailed(error, __FILENAME__, __LINE__); } - for (size_t i = 0; i < num_schemas; i++) { - int32_t schema_id; - uint64_t num_messages; + bool prev_metadata_initialized{false}; + SchemaReader::SchemaMetadata prev_metadata{}; + int32_t prev_schema_id{}; + for (size_t i = 0; i < num_schemas; ++i) { + size_t table_id; size_t table_offset; - size_t uncompressed_size; + int32_t schema_id; + size_t num_messages; - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); + if (table_offset > m_table_reader.get_uncompressed_table_size(table_id)) { + throw OperationFailed(ErrorCodeCorrupt, __FILENAME__, __LINE__); + } + + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(uncompressed_size); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - m_id_to_table_metadata[schema_id] = {num_messages, table_offset, uncompressed_size}; + if (prev_metadata_initialized) { + size_t uncompressed_size{0}; + if (table_id != prev_metadata.table_id) { + uncompressed_size + = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) + - prev_metadata.table_offset; + } else { + uncompressed_size = table_offset - prev_metadata.table_offset; + } + prev_metadata.uncompressed_size = uncompressed_size; + m_id_to_schema_metadata[prev_schema_id] = prev_metadata; + } else { + prev_metadata_initialized = true; + } + prev_metadata = {table_id, table_offset, num_messages, 0}; + prev_schema_id = schema_id; m_schema_ids.push_back(schema_id); } + prev_metadata.uncompressed_size + = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) + - prev_metadata.table_offset; + m_id_to_schema_metadata[prev_schema_id] = prev_metadata; m_table_metadata_decompressor.close(); } @@ -89,14 +117,12 @@ void ArchiveReader::read_dictionaries_and_metadata() { read_metadata(); } -SchemaReader& ArchiveReader::read_table( +SchemaReader& ArchiveReader::read_schema_table( int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records ) { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - - if (m_id_to_table_metadata.count(schema_id) == 0) { + if (m_id_to_schema_metadata.count(schema_id) == 0) { throw OperationFailed(ErrorCodeFileNotFound, __FILENAME__, __LINE__); } @@ -107,30 +133,26 @@ SchemaReader& ArchiveReader::read_table( should_marshal_records ); - m_tables_file_reader.try_seek_from_begin(m_id_to_table_metadata[schema_id].offset); - m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); - m_schema_reader.load( - m_tables_decompressor, - m_id_to_table_metadata[schema_id].uncompressed_size - ); - m_tables_decompressor.close_for_reuse(); + auto& schema_metadata = m_id_to_schema_metadata[schema_id]; + auto table_buffer = read_table(schema_metadata.table_id, true); + m_schema_reader + .load(table_buffer, schema_metadata.table_offset, schema_metadata.uncompressed_size); return m_schema_reader; } std::vector> ArchiveReader::read_all_tables() { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - std::vector> readers; - readers.reserve(m_id_to_table_metadata.size()); - for (auto const& [id, table_metadata] : m_id_to_table_metadata) { + readers.reserve(m_id_to_schema_metadata.size()); + for (auto schema_id : m_schema_ids) { auto schema_reader = std::make_shared(); - initialize_schema_reader(*schema_reader, id, true, true); - - m_tables_file_reader.try_seek_from_begin(table_metadata.offset); - m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); - schema_reader->load(m_tables_decompressor, table_metadata.uncompressed_size); - m_tables_decompressor.close_for_reuse(); - + initialize_schema_reader(*schema_reader, schema_id, true, true); + auto& schema_metadata = m_id_to_schema_metadata[schema_id]; + auto table_buffer = read_table(schema_metadata.table_id, false); + schema_reader->load( + table_buffer, + schema_metadata.table_offset, + schema_metadata.uncompressed_size + ); readers.push_back(std::move(schema_reader)); } return readers; @@ -237,7 +259,7 @@ void ArchiveReader::initialize_schema_reader( m_schema_tree, schema_id, schema.get_ordered_schema_view(), - m_id_to_table_metadata[schema_id].num_messages, + m_id_to_schema_metadata[schema_id].num_messages, should_marshal_records ); auto timestamp_column_ids = m_timestamp_dict->get_authoritative_timestamp_column_ids(); @@ -284,9 +306,8 @@ void ArchiveReader::initialize_schema_reader( void ArchiveReader::store(FileWriter& writer) { std::string message; - - for (auto& [id, table_metadata] : m_id_to_table_metadata) { - auto& schema_reader = read_table(id, false, true); + for (auto schema_id : m_schema_ids) { + auto& schema_reader = read_schema_table(schema_id, false, true); while (schema_reader.get_next_message(message)) { writer.write(message.c_str(), message.length()); } @@ -304,11 +325,28 @@ void ArchiveReader::close() { m_array_dict->close(); m_timestamp_dict->close(); - m_tables_file_reader.close(); + m_table_reader.close(); m_table_metadata_file_reader.close(); - m_id_to_table_metadata.clear(); + m_id_to_schema_metadata.clear(); m_schema_ids.clear(); + m_cur_table_id = 0; + m_table_buffer.reset(); + m_table_buffer_size = 0ULL; } +std::shared_ptr ArchiveReader::read_table(size_t table_id, bool reuse_buffer) { + if (nullptr != m_table_buffer && m_cur_table_id == table_id) { + return m_table_buffer; + } + + if (false == reuse_buffer) { + m_table_buffer.reset(); + m_table_buffer_size = 0; + } + + m_table_reader.read_table(table_id, m_table_buffer, m_table_buffer_size); + m_cur_table_id = table_id; + return m_table_buffer; +} } // namespace clp_s diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index 91fcc1a94..02755592a 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -12,6 +12,7 @@ #include "DictionaryReader.hpp" #include "ReaderUtils.hpp" #include "SchemaReader.hpp" +#include "TableReader.hpp" #include "TimestampDictionaryReader.hpp" #include "Utils.hpp" @@ -91,8 +92,11 @@ class ArchiveReader { * @param should_marshal_records * @return the schema reader */ - SchemaReader& - read_table(int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records); + SchemaReader& read_schema_table( + int32_t schema_id, + bool should_extract_timestamp, + bool should_marshal_records + ); /** * Loads all of the tables in the archive and returns SchemaReaders for them. @@ -171,6 +175,14 @@ class ArchiveReader { bool should_marshal_records ); + /** + * Reads a table with given ID from the table reader. If read_table is called multiple times in + * a row for the same table_id a cached buffer is returned. This function allows the caller to + * ask for the same buffer to be reused to read multiple different tables: this can save memory + * allocations, but can only be used when tables are read one at a time. + */ + std::shared_ptr read_table(size_t table_id, bool reuse_buffer); + bool m_is_open; std::string m_archive_id; std::shared_ptr m_var_dict; @@ -181,13 +193,15 @@ class ArchiveReader { std::shared_ptr m_schema_tree; std::shared_ptr m_schema_map; std::vector m_schema_ids; - std::map m_id_to_table_metadata; + std::map m_id_to_schema_metadata; - FileReader m_tables_file_reader; + TableReader m_table_reader; FileReader m_table_metadata_file_reader; - ZstdDecompressor m_tables_decompressor; ZstdDecompressor m_table_metadata_decompressor; SchemaReader m_schema_reader; + std::shared_ptr m_table_buffer{}; + size_t m_table_buffer_size{0ULL}; + size_t m_cur_table_id{0ULL}; }; } // namespace clp_s diff --git a/components/core/src/clp_s/CMakeLists.txt b/components/core/src/clp_s/CMakeLists.txt index c8cf08b22..cc1fd78cc 100644 --- a/components/core/src/clp_s/CMakeLists.txt +++ b/components/core/src/clp_s/CMakeLists.txt @@ -78,6 +78,8 @@ set( SchemaTree.hpp SchemaWriter.cpp SchemaWriter.hpp + TableReader.cpp + TableReader.hpp TimestampDictionaryReader.cpp TimestampDictionaryReader.hpp TimestampDictionaryWriter.cpp diff --git a/components/core/src/clp_s/SchemaReader.cpp b/components/core/src/clp_s/SchemaReader.cpp index 03edebf69..265e772d8 100644 --- a/components/core/src/clp_s/SchemaReader.cpp +++ b/components/core/src/clp_s/SchemaReader.cpp @@ -37,17 +37,13 @@ void SchemaReader::mark_column_as_timestamp(BaseColumnReader* column_reader) { } } -void SchemaReader::load(ZstdDecompressor& decompressor, size_t uncompressed_size) { - if (uncompressed_size > m_table_buffer_size) { - m_table_buffer = std::make_unique(uncompressed_size); - m_table_buffer_size = uncompressed_size; - } - auto error = decompressor.try_read_exact_length(m_table_buffer.get(), uncompressed_size); - if (ErrorCodeSuccess != error) { - throw OperationFailed(error, __FILENAME__, __LINE__); - } - - BufferViewReader buffer_reader{m_table_buffer.get(), uncompressed_size}; +void SchemaReader::load( + std::shared_ptr table_buffer, + size_t offset, + size_t uncompressed_size +) { + m_table_buffer = table_buffer; + BufferViewReader buffer_reader{m_table_buffer.get() + offset, uncompressed_size}; for (auto& reader : m_columns) { reader->load(buffer_reader, m_num_messages); } diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 3639560f6..300bc47c8 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -1,6 +1,7 @@ #ifndef CLP_S_SCHEMAREADER_HPP #define CLP_S_SCHEMAREADER_HPP +#include #include #include #include @@ -11,7 +12,6 @@ #include "FileReader.hpp" #include "JsonSerializer.hpp" #include "SchemaTree.hpp" -#include "ZstdDecompressor.hpp" namespace clp_s { class SchemaReader; @@ -47,9 +47,10 @@ class SchemaReader { : TraceableException(error_code, filename, line_number) {} }; - struct TableMetadata { - uint64_t num_messages; - size_t offset; + struct SchemaMetadata { + size_t table_id; + size_t table_offset; + size_t num_messages; size_t uncompressed_size; }; @@ -130,11 +131,12 @@ class SchemaReader { ); /** - * Loads the encoded messages - * @param decompressor + * Loads the encoded messages from a shared buffer starting at a given offset + * @param table_buffer + * @param offset * @param uncompressed_size */ - void load(ZstdDecompressor& decompressor, size_t uncompressed_size); + void load(std::shared_ptr table_buffer, size_t offset, size_t uncompressed_size); /** * Gets next message @@ -277,8 +279,7 @@ class SchemaReader { std::unordered_map m_column_map; std::vector m_columns; std::vector m_reordered_columns; - std::unique_ptr m_table_buffer; - size_t m_table_buffer_size{0}; + std::shared_ptr m_table_buffer; BaseColumnReader* m_timestamp_column; std::function m_get_timestamp; diff --git a/components/core/src/clp_s/TableReader.cpp b/components/core/src/clp_s/TableReader.cpp new file mode 100644 index 000000000..969824356 --- /dev/null +++ b/components/core/src/clp_s/TableReader.cpp @@ -0,0 +1,108 @@ +#include "TableReader.hpp" + +namespace clp_s { + +void TableReader::read_metadata(ZstdDecompressor& decompressor) { + switch (m_state) { + case TableReaderState::Uninitialized: + m_state = TableReaderState::MetadataRead; + break; + case TableReaderState::TablesOpened: + m_state = TableReaderState::TablesOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + + size_t num_tables; + if (auto error = decompressor.try_read_numeric_value(num_tables); ErrorCodeSuccess != error) { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_table_metadata.reserve(num_tables); + + for (size_t i = 0; i < num_tables; ++i) { + size_t file_offset; + size_t uncompressed_size; + + if (auto error = decompressor.try_read_numeric_value(file_offset); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + if (auto error = decompressor.try_read_numeric_value(uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + m_table_metadata.emplace_back(file_offset, uncompressed_size); + } +} + +void TableReader::open_tables(std::string const& tables_file_path) { + switch (m_state) { + case TableReaderState::Uninitialized: + m_state = TableReaderState::TablesOpened; + break; + case TableReaderState::MetadataRead: + m_state = TableReaderState::TablesOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_tables_reader.open(tables_file_path); +} + +void TableReader::close() { + switch (m_state) { + case TableReaderState::TablesOpened: + case TableReaderState::TablesOpenedAndMetadataRead: + case TableReaderState::ReadingTables: + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_tables_reader.close(); + m_previous_table_id = 0; + m_table_metadata.clear(); + m_state = TableReaderState::Uninitialized; +} + +void TableReader::read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size) { + constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB + if (table_id > m_table_metadata.size()) { + throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); + } + + switch (m_state) { + case TableReaderState::TablesOpenedAndMetadataRead: + m_state = TableReaderState::ReadingTables; + break; + case TableReaderState::ReadingTables: + if (m_previous_table_id >= table_id) { + throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); + } + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_previous_table_id = table_id; + + auto& [file_offset, uncompressed_size] = m_table_metadata[table_id]; + m_tables_reader.try_seek_from_begin(file_offset); + m_tables_decompressor.open(m_tables_reader, cDecompressorFileReadBufferCapacity); + if (buf_size < uncompressed_size) { + // make_shared is supposed to work here for c++20, but it seems like the compiler version + // we use doesn't support it, so we convert a unique_ptr to a shared_ptr instead. + buf = std::make_unique(uncompressed_size); + buf_size = uncompressed_size; + } + if (auto error = m_tables_decompressor.try_read_exact_length(buf.get(), uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_tables_decompressor.close_for_reuse(); +} +} // namespace clp_s diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp new file mode 100644 index 000000000..116d1b957 --- /dev/null +++ b/components/core/src/clp_s/TableReader.hpp @@ -0,0 +1,89 @@ +#ifndef CLP_S_TABLEREADER_HPP +#define CLP_S_TABLEREADER_HPP + +#include +#include +#include +#include + +#include "FileReader.hpp" +#include "ZstdDecompressor.hpp" + +namespace clp_s { +/** + * TableReader ensures that the tables section of an archive is read safely. Any attempt to read the + * tables section without loading the tables metadata, and any attempt to read tables section out of + * order will throw. As well, any incorrect usage of this class (e.g. closing without opening) will + * throw. + */ +class TableReader { +public: + class OperationFailed : public TraceableException { + public: + // Constructors + OperationFailed(ErrorCode error_code, char const* const filename, int line_number) + : TraceableException(error_code, filename, line_number) {} + }; + + struct TableMetadata { + size_t file_offset; + size_t uncompressed_size; + }; + + /** + * Reads table metadata from the provided compression stream. Must be invoked before reading + * tables. + */ + void read_metadata(ZstdDecompressor& decompressor); + + /** + * Opens a file reader for the tables section. Must be invoked before reading tables. + */ + void open_tables(std::string const& tables_file_path); + + /** + * Closes the file reader for the tables section. + */ + void close(); + + /** + * Decompresses a table with a given table_id and returns it. This function must be called + * strictly in ascending table_id order. If this function is called twice for the same table or + * if a table with lower id is requested after a table with higher id then an error is thrown. + * + * Note: the buffer and buffer size are returned by reference. This is to support the use case + * where the caller wants to re-use the same buffer for multiple tables to avoid allocations + * when they already have a sufficiently large buffer. If no buffer is provided or the provided + * buffer is too small calling read_table will create a buffer exactly as large as the table + * being decompressed. + * + * @param table_id + * @param buf + * @param buf_size + * @return a shared_ptr to a buffer containing the requested table + */ + void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); + + size_t get_uncompressed_table_size(size_t table_id) const { + return m_table_metadata.at(table_id).uncompressed_size; + } + +private: + enum TableReaderState { + Uninitialized, + MetadataRead, + TablesOpened, + TablesOpenedAndMetadataRead, + ReadingTables + }; + + std::vector m_table_metadata; + FileReader m_tables_reader; + ZstdDecompressor m_tables_decompressor; + TableReaderState m_state{TableReaderState::Uninitialized}; + size_t m_previous_table_id{0ULL}; +}; + +} // namespace clp_s + +#endif // CLP_S_TABLEREADER_HPP diff --git a/components/core/src/clp_s/search/Output.cpp b/components/core/src/clp_s/search/Output.cpp index b6a3b8fe0..4d36b4e29 100644 --- a/components/core/src/clp_s/search/Output.cpp +++ b/components/core/src/clp_s/search/Output.cpp @@ -84,7 +84,7 @@ bool Output::filter() { add_wildcard_columns_to_searched_columns(); - auto& reader = m_archive_reader->read_table( + auto& reader = m_archive_reader->read_schema_table( schema_id, m_output_handler->should_output_metadata(), m_should_marshal_records From 794a7327da1a0905bc2b3b0f2bff34259d7bc78b Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 2 Jul 2024 15:34:49 +0000 Subject: [PATCH 03/21] Revert "Implement decompression side of table packing" to make reviewing easie This reverts commit 19590b17cac3cab59f9a068ab68e855a89cdb1b5. --- components/core/src/clp_s/ArchiveReader.cpp | 114 +++++++------------- components/core/src/clp_s/ArchiveReader.hpp | 24 +---- components/core/src/clp_s/CMakeLists.txt | 2 - components/core/src/clp_s/SchemaReader.cpp | 18 ++-- components/core/src/clp_s/SchemaReader.hpp | 19 ++-- components/core/src/clp_s/TableReader.cpp | 108 ------------------- components/core/src/clp_s/TableReader.hpp | 89 --------------- components/core/src/clp_s/search/Output.cpp | 2 +- 8 files changed, 64 insertions(+), 312 deletions(-) delete mode 100644 components/core/src/clp_s/TableReader.cpp delete mode 100644 components/core/src/clp_s/TableReader.hpp diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index 3c62cc63b..f211a0707 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -27,8 +27,8 @@ void ArchiveReader::open(string_view archives_dir, string_view archive_id) { m_schema_tree = ReaderUtils::read_schema_tree(archive_path_str); m_schema_map = ReaderUtils::read_schemas(archive_path_str); + m_tables_file_reader.open(archive_path_str + constants::cArchiveTablesFile); m_table_metadata_file_reader.open(archive_path_str + constants::cArchiveTableMetadataFile); - m_table_reader.open_tables(archive_path_str + constants::cArchiveTablesFile); } void ArchiveReader::read_metadata() { @@ -38,8 +38,6 @@ void ArchiveReader::read_metadata() { cDecompressorFileReadBufferCapacity ); - m_table_reader.read_metadata(m_table_metadata_decompressor); - size_t num_schemas; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_schemas); ErrorCodeSuccess != error) @@ -47,65 +45,39 @@ void ArchiveReader::read_metadata() { throw OperationFailed(error, __FILENAME__, __LINE__); } - bool prev_metadata_initialized{false}; - SchemaReader::SchemaMetadata prev_metadata{}; - int32_t prev_schema_id{}; - for (size_t i = 0; i < num_schemas; ++i) { - size_t table_id; - size_t table_offset; + for (size_t i = 0; i < num_schemas; i++) { int32_t schema_id; - size_t num_messages; + uint64_t num_messages; + size_t table_offset; + size_t uncompressed_size; - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_id); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (table_offset > m_table_reader.get_uncompressed_table_size(table_id)) { - throw OperationFailed(ErrorCodeCorrupt, __FILENAME__, __LINE__); - } - - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(uncompressed_size); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (prev_metadata_initialized) { - size_t uncompressed_size{0}; - if (table_id != prev_metadata.table_id) { - uncompressed_size - = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) - - prev_metadata.table_offset; - } else { - uncompressed_size = table_offset - prev_metadata.table_offset; - } - prev_metadata.uncompressed_size = uncompressed_size; - m_id_to_schema_metadata[prev_schema_id] = prev_metadata; - } else { - prev_metadata_initialized = true; - } - prev_metadata = {table_id, table_offset, num_messages, 0}; - prev_schema_id = schema_id; + m_id_to_table_metadata[schema_id] = {num_messages, table_offset, uncompressed_size}; m_schema_ids.push_back(schema_id); } - prev_metadata.uncompressed_size - = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) - - prev_metadata.table_offset; - m_id_to_schema_metadata[prev_schema_id] = prev_metadata; m_table_metadata_decompressor.close(); } @@ -117,12 +89,14 @@ void ArchiveReader::read_dictionaries_and_metadata() { read_metadata(); } -SchemaReader& ArchiveReader::read_schema_table( +SchemaReader& ArchiveReader::read_table( int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records ) { - if (m_id_to_schema_metadata.count(schema_id) == 0) { + constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB + + if (m_id_to_table_metadata.count(schema_id) == 0) { throw OperationFailed(ErrorCodeFileNotFound, __FILENAME__, __LINE__); } @@ -133,26 +107,30 @@ SchemaReader& ArchiveReader::read_schema_table( should_marshal_records ); - auto& schema_metadata = m_id_to_schema_metadata[schema_id]; - auto table_buffer = read_table(schema_metadata.table_id, true); - m_schema_reader - .load(table_buffer, schema_metadata.table_offset, schema_metadata.uncompressed_size); + m_tables_file_reader.try_seek_from_begin(m_id_to_table_metadata[schema_id].offset); + m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); + m_schema_reader.load( + m_tables_decompressor, + m_id_to_table_metadata[schema_id].uncompressed_size + ); + m_tables_decompressor.close_for_reuse(); return m_schema_reader; } std::vector> ArchiveReader::read_all_tables() { + constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB + std::vector> readers; - readers.reserve(m_id_to_schema_metadata.size()); - for (auto schema_id : m_schema_ids) { + readers.reserve(m_id_to_table_metadata.size()); + for (auto const& [id, table_metadata] : m_id_to_table_metadata) { auto schema_reader = std::make_shared(); - initialize_schema_reader(*schema_reader, schema_id, true, true); - auto& schema_metadata = m_id_to_schema_metadata[schema_id]; - auto table_buffer = read_table(schema_metadata.table_id, false); - schema_reader->load( - table_buffer, - schema_metadata.table_offset, - schema_metadata.uncompressed_size - ); + initialize_schema_reader(*schema_reader, id, true, true); + + m_tables_file_reader.try_seek_from_begin(table_metadata.offset); + m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); + schema_reader->load(m_tables_decompressor, table_metadata.uncompressed_size); + m_tables_decompressor.close_for_reuse(); + readers.push_back(std::move(schema_reader)); } return readers; @@ -259,7 +237,7 @@ void ArchiveReader::initialize_schema_reader( m_schema_tree, schema_id, schema.get_ordered_schema_view(), - m_id_to_schema_metadata[schema_id].num_messages, + m_id_to_table_metadata[schema_id].num_messages, should_marshal_records ); auto timestamp_column_ids = m_timestamp_dict->get_authoritative_timestamp_column_ids(); @@ -306,8 +284,9 @@ void ArchiveReader::initialize_schema_reader( void ArchiveReader::store(FileWriter& writer) { std::string message; - for (auto schema_id : m_schema_ids) { - auto& schema_reader = read_schema_table(schema_id, false, true); + + for (auto& [id, table_metadata] : m_id_to_table_metadata) { + auto& schema_reader = read_table(id, false, true); while (schema_reader.get_next_message(message)) { writer.write(message.c_str(), message.length()); } @@ -325,28 +304,11 @@ void ArchiveReader::close() { m_array_dict->close(); m_timestamp_dict->close(); - m_table_reader.close(); + m_tables_file_reader.close(); m_table_metadata_file_reader.close(); - m_id_to_schema_metadata.clear(); + m_id_to_table_metadata.clear(); m_schema_ids.clear(); - m_cur_table_id = 0; - m_table_buffer.reset(); - m_table_buffer_size = 0ULL; } -std::shared_ptr ArchiveReader::read_table(size_t table_id, bool reuse_buffer) { - if (nullptr != m_table_buffer && m_cur_table_id == table_id) { - return m_table_buffer; - } - - if (false == reuse_buffer) { - m_table_buffer.reset(); - m_table_buffer_size = 0; - } - - m_table_reader.read_table(table_id, m_table_buffer, m_table_buffer_size); - m_cur_table_id = table_id; - return m_table_buffer; -} } // namespace clp_s diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index 02755592a..91fcc1a94 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -12,7 +12,6 @@ #include "DictionaryReader.hpp" #include "ReaderUtils.hpp" #include "SchemaReader.hpp" -#include "TableReader.hpp" #include "TimestampDictionaryReader.hpp" #include "Utils.hpp" @@ -92,11 +91,8 @@ class ArchiveReader { * @param should_marshal_records * @return the schema reader */ - SchemaReader& read_schema_table( - int32_t schema_id, - bool should_extract_timestamp, - bool should_marshal_records - ); + SchemaReader& + read_table(int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records); /** * Loads all of the tables in the archive and returns SchemaReaders for them. @@ -175,14 +171,6 @@ class ArchiveReader { bool should_marshal_records ); - /** - * Reads a table with given ID from the table reader. If read_table is called multiple times in - * a row for the same table_id a cached buffer is returned. This function allows the caller to - * ask for the same buffer to be reused to read multiple different tables: this can save memory - * allocations, but can only be used when tables are read one at a time. - */ - std::shared_ptr read_table(size_t table_id, bool reuse_buffer); - bool m_is_open; std::string m_archive_id; std::shared_ptr m_var_dict; @@ -193,15 +181,13 @@ class ArchiveReader { std::shared_ptr m_schema_tree; std::shared_ptr m_schema_map; std::vector m_schema_ids; - std::map m_id_to_schema_metadata; + std::map m_id_to_table_metadata; - TableReader m_table_reader; + FileReader m_tables_file_reader; FileReader m_table_metadata_file_reader; + ZstdDecompressor m_tables_decompressor; ZstdDecompressor m_table_metadata_decompressor; SchemaReader m_schema_reader; - std::shared_ptr m_table_buffer{}; - size_t m_table_buffer_size{0ULL}; - size_t m_cur_table_id{0ULL}; }; } // namespace clp_s diff --git a/components/core/src/clp_s/CMakeLists.txt b/components/core/src/clp_s/CMakeLists.txt index cc1fd78cc..c8cf08b22 100644 --- a/components/core/src/clp_s/CMakeLists.txt +++ b/components/core/src/clp_s/CMakeLists.txt @@ -78,8 +78,6 @@ set( SchemaTree.hpp SchemaWriter.cpp SchemaWriter.hpp - TableReader.cpp - TableReader.hpp TimestampDictionaryReader.cpp TimestampDictionaryReader.hpp TimestampDictionaryWriter.cpp diff --git a/components/core/src/clp_s/SchemaReader.cpp b/components/core/src/clp_s/SchemaReader.cpp index 265e772d8..03edebf69 100644 --- a/components/core/src/clp_s/SchemaReader.cpp +++ b/components/core/src/clp_s/SchemaReader.cpp @@ -37,13 +37,17 @@ void SchemaReader::mark_column_as_timestamp(BaseColumnReader* column_reader) { } } -void SchemaReader::load( - std::shared_ptr table_buffer, - size_t offset, - size_t uncompressed_size -) { - m_table_buffer = table_buffer; - BufferViewReader buffer_reader{m_table_buffer.get() + offset, uncompressed_size}; +void SchemaReader::load(ZstdDecompressor& decompressor, size_t uncompressed_size) { + if (uncompressed_size > m_table_buffer_size) { + m_table_buffer = std::make_unique(uncompressed_size); + m_table_buffer_size = uncompressed_size; + } + auto error = decompressor.try_read_exact_length(m_table_buffer.get(), uncompressed_size); + if (ErrorCodeSuccess != error) { + throw OperationFailed(error, __FILENAME__, __LINE__); + } + + BufferViewReader buffer_reader{m_table_buffer.get(), uncompressed_size}; for (auto& reader : m_columns) { reader->load(buffer_reader, m_num_messages); } diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 300bc47c8..3639560f6 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -1,7 +1,6 @@ #ifndef CLP_S_SCHEMAREADER_HPP #define CLP_S_SCHEMAREADER_HPP -#include #include #include #include @@ -12,6 +11,7 @@ #include "FileReader.hpp" #include "JsonSerializer.hpp" #include "SchemaTree.hpp" +#include "ZstdDecompressor.hpp" namespace clp_s { class SchemaReader; @@ -47,10 +47,9 @@ class SchemaReader { : TraceableException(error_code, filename, line_number) {} }; - struct SchemaMetadata { - size_t table_id; - size_t table_offset; - size_t num_messages; + struct TableMetadata { + uint64_t num_messages; + size_t offset; size_t uncompressed_size; }; @@ -131,12 +130,11 @@ class SchemaReader { ); /** - * Loads the encoded messages from a shared buffer starting at a given offset - * @param table_buffer - * @param offset + * Loads the encoded messages + * @param decompressor * @param uncompressed_size */ - void load(std::shared_ptr table_buffer, size_t offset, size_t uncompressed_size); + void load(ZstdDecompressor& decompressor, size_t uncompressed_size); /** * Gets next message @@ -279,7 +277,8 @@ class SchemaReader { std::unordered_map m_column_map; std::vector m_columns; std::vector m_reordered_columns; - std::shared_ptr m_table_buffer; + std::unique_ptr m_table_buffer; + size_t m_table_buffer_size{0}; BaseColumnReader* m_timestamp_column; std::function m_get_timestamp; diff --git a/components/core/src/clp_s/TableReader.cpp b/components/core/src/clp_s/TableReader.cpp deleted file mode 100644 index 969824356..000000000 --- a/components/core/src/clp_s/TableReader.cpp +++ /dev/null @@ -1,108 +0,0 @@ -#include "TableReader.hpp" - -namespace clp_s { - -void TableReader::read_metadata(ZstdDecompressor& decompressor) { - switch (m_state) { - case TableReaderState::Uninitialized: - m_state = TableReaderState::MetadataRead; - break; - case TableReaderState::TablesOpened: - m_state = TableReaderState::TablesOpenedAndMetadataRead; - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - - size_t num_tables; - if (auto error = decompressor.try_read_numeric_value(num_tables); ErrorCodeSuccess != error) { - throw OperationFailed(error, __FILE__, __LINE__); - } - m_table_metadata.reserve(num_tables); - - for (size_t i = 0; i < num_tables; ++i) { - size_t file_offset; - size_t uncompressed_size; - - if (auto error = decompressor.try_read_numeric_value(file_offset); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - - if (auto error = decompressor.try_read_numeric_value(uncompressed_size); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - - m_table_metadata.emplace_back(file_offset, uncompressed_size); - } -} - -void TableReader::open_tables(std::string const& tables_file_path) { - switch (m_state) { - case TableReaderState::Uninitialized: - m_state = TableReaderState::TablesOpened; - break; - case TableReaderState::MetadataRead: - m_state = TableReaderState::TablesOpenedAndMetadataRead; - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_tables_reader.open(tables_file_path); -} - -void TableReader::close() { - switch (m_state) { - case TableReaderState::TablesOpened: - case TableReaderState::TablesOpenedAndMetadataRead: - case TableReaderState::ReadingTables: - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_tables_reader.close(); - m_previous_table_id = 0; - m_table_metadata.clear(); - m_state = TableReaderState::Uninitialized; -} - -void TableReader::read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size) { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - if (table_id > m_table_metadata.size()) { - throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); - } - - switch (m_state) { - case TableReaderState::TablesOpenedAndMetadataRead: - m_state = TableReaderState::ReadingTables; - break; - case TableReaderState::ReadingTables: - if (m_previous_table_id >= table_id) { - throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); - } - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_previous_table_id = table_id; - - auto& [file_offset, uncompressed_size] = m_table_metadata[table_id]; - m_tables_reader.try_seek_from_begin(file_offset); - m_tables_decompressor.open(m_tables_reader, cDecompressorFileReadBufferCapacity); - if (buf_size < uncompressed_size) { - // make_shared is supposed to work here for c++20, but it seems like the compiler version - // we use doesn't support it, so we convert a unique_ptr to a shared_ptr instead. - buf = std::make_unique(uncompressed_size); - buf_size = uncompressed_size; - } - if (auto error = m_tables_decompressor.try_read_exact_length(buf.get(), uncompressed_size); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - m_tables_decompressor.close_for_reuse(); -} -} // namespace clp_s diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp deleted file mode 100644 index 116d1b957..000000000 --- a/components/core/src/clp_s/TableReader.hpp +++ /dev/null @@ -1,89 +0,0 @@ -#ifndef CLP_S_TABLEREADER_HPP -#define CLP_S_TABLEREADER_HPP - -#include -#include -#include -#include - -#include "FileReader.hpp" -#include "ZstdDecompressor.hpp" - -namespace clp_s { -/** - * TableReader ensures that the tables section of an archive is read safely. Any attempt to read the - * tables section without loading the tables metadata, and any attempt to read tables section out of - * order will throw. As well, any incorrect usage of this class (e.g. closing without opening) will - * throw. - */ -class TableReader { -public: - class OperationFailed : public TraceableException { - public: - // Constructors - OperationFailed(ErrorCode error_code, char const* const filename, int line_number) - : TraceableException(error_code, filename, line_number) {} - }; - - struct TableMetadata { - size_t file_offset; - size_t uncompressed_size; - }; - - /** - * Reads table metadata from the provided compression stream. Must be invoked before reading - * tables. - */ - void read_metadata(ZstdDecompressor& decompressor); - - /** - * Opens a file reader for the tables section. Must be invoked before reading tables. - */ - void open_tables(std::string const& tables_file_path); - - /** - * Closes the file reader for the tables section. - */ - void close(); - - /** - * Decompresses a table with a given table_id and returns it. This function must be called - * strictly in ascending table_id order. If this function is called twice for the same table or - * if a table with lower id is requested after a table with higher id then an error is thrown. - * - * Note: the buffer and buffer size are returned by reference. This is to support the use case - * where the caller wants to re-use the same buffer for multiple tables to avoid allocations - * when they already have a sufficiently large buffer. If no buffer is provided or the provided - * buffer is too small calling read_table will create a buffer exactly as large as the table - * being decompressed. - * - * @param table_id - * @param buf - * @param buf_size - * @return a shared_ptr to a buffer containing the requested table - */ - void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); - - size_t get_uncompressed_table_size(size_t table_id) const { - return m_table_metadata.at(table_id).uncompressed_size; - } - -private: - enum TableReaderState { - Uninitialized, - MetadataRead, - TablesOpened, - TablesOpenedAndMetadataRead, - ReadingTables - }; - - std::vector m_table_metadata; - FileReader m_tables_reader; - ZstdDecompressor m_tables_decompressor; - TableReaderState m_state{TableReaderState::Uninitialized}; - size_t m_previous_table_id{0ULL}; -}; - -} // namespace clp_s - -#endif // CLP_S_TABLEREADER_HPP diff --git a/components/core/src/clp_s/search/Output.cpp b/components/core/src/clp_s/search/Output.cpp index 4d36b4e29..b6a3b8fe0 100644 --- a/components/core/src/clp_s/search/Output.cpp +++ b/components/core/src/clp_s/search/Output.cpp @@ -84,7 +84,7 @@ bool Output::filter() { add_wildcard_columns_to_searched_columns(); - auto& reader = m_archive_reader->read_schema_table( + auto& reader = m_archive_reader->read_table( schema_id, m_output_handler->should_output_metadata(), m_should_marshal_records From 0225c9eff2207f9871df5cc69ce219b0316537bd Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Fri, 19 Jul 2024 12:08:25 -0400 Subject: [PATCH 04/21] Apply suggestions from code review Co-authored-by: wraymo <37269683+wraymo@users.noreply.github.com> --- components/core/src/clp_s/ArchiveWriter.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index eac6a2011..e64c87f45 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -150,9 +150,8 @@ size_t ArchiveWriter::store_tables() { schema_metadata.reserve(m_id_to_schema_writer.size()); schemas.reserve(m_id_to_schema_writer.size()); - auto it = m_id_to_schema_writer.begin(); - for (size_t i = 0; i < m_id_to_schema_writer.size(); ++i) { - schemas.push_back(it++); + for (auto it = m_id_to_schema_writer.begin(); it != m_id_to_schema_writer.end(); ++it) { + schemas.push_back(it); } auto comp = [](schema_map_it const& lhs, schema_map_it const& rhs) -> bool { return lhs->second->get_total_uncompressed_size() From b2e2c145c78ab1888acc6947d3493f1eff8cd232 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 19 Jul 2024 16:30:44 +0000 Subject: [PATCH 05/21] Address some review comments --- components/core/src/clp_s/ColumnWriter.cpp | 26 +++++++++---------- components/core/src/clp_s/ColumnWriter.hpp | 18 ++++++------- .../core/src/clp_s/CommandLineArguments.hpp | 2 +- components/core/src/clp_s/SchemaWriter.cpp | 13 ++++------ components/core/src/clp_s/clp-s.cpp | 2 +- 5 files changed, 29 insertions(+), 32 deletions(-) diff --git a/components/core/src/clp_s/ColumnWriter.cpp b/components/core/src/clp_s/ColumnWriter.cpp index 9c40345b3..77fa51f15 100644 --- a/components/core/src/clp_s/ColumnWriter.cpp +++ b/components/core/src/clp_s/ColumnWriter.cpp @@ -1,9 +1,9 @@ #include "ColumnWriter.hpp" namespace clp_s { -void Int64ColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = sizeof(int64_t); +size_t Int64ColumnWriter::add_value(ParsedMessage::variable_t& value) { m_values.push_back(std::get(value)); + return sizeof(int64_t); } void Int64ColumnWriter::store(ZstdCompressor& compressor) { @@ -11,9 +11,9 @@ void Int64ColumnWriter::store(ZstdCompressor& compressor) { compressor.write(reinterpret_cast(m_values.data()), size); } -void FloatColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = sizeof(double); +size_t FloatColumnWriter::add_value(ParsedMessage::variable_t& value) { m_values.push_back(std::get(value)); + return sizeof(double); } void FloatColumnWriter::store(ZstdCompressor& compressor) { @@ -21,9 +21,9 @@ void FloatColumnWriter::store(ZstdCompressor& compressor) { compressor.write(reinterpret_cast(m_values.data()), size); } -void BooleanColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = sizeof(uint8_t); +size_t BooleanColumnWriter::add_value(ParsedMessage::variable_t& value) { m_values.push_back(std::get(value) ? 1 : 0); + return sizeof(uint8_t); } void BooleanColumnWriter::store(ZstdCompressor& compressor) { @@ -31,8 +31,7 @@ void BooleanColumnWriter::store(ZstdCompressor& compressor) { compressor.write(reinterpret_cast(m_values.data()), size); } -void ClpStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = sizeof(int64_t); +size_t ClpStringColumnWriter::add_value(ParsedMessage::variable_t& value) { std::string string_var = std::get(value); uint64_t id; uint64_t offset = m_encoded_vars.size(); @@ -45,7 +44,7 @@ void ClpStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& m_log_dict->add_entry(m_logtype_entry, id); auto encoded_id = encode_log_dict_id(id, offset); m_logtypes.push_back(encoded_id); - size += sizeof(int64_t) * (m_encoded_vars.size() - offset); + return sizeof(int64_t) + sizeof(int64_t) * (m_encoded_vars.size() - offset); } void ClpStringColumnWriter::store(ZstdCompressor& compressor) { @@ -57,12 +56,12 @@ void ClpStringColumnWriter::store(ZstdCompressor& compressor) { compressor.write(reinterpret_cast(m_encoded_vars.data()), encoded_vars_size); } -void VariableStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = sizeof(int64_t); +size_t VariableStringColumnWriter::add_value(ParsedMessage::variable_t& value) { std::string string_var = std::get(value); uint64_t id; m_var_dict->add_entry(string_var, id); m_variables.push_back(id); + return sizeof(int64_t); } void VariableStringColumnWriter::store(ZstdCompressor& compressor) { @@ -70,11 +69,12 @@ void VariableStringColumnWriter::store(ZstdCompressor& compressor) { compressor.write(reinterpret_cast(m_variables.data()), size); } -void DateStringColumnWriter::add_value(ParsedMessage::variable_t& value, size_t& size) { - size = 2 * sizeof(int64_t); +size_t DateStringColumnWriter::add_value(ParsedMessage::variable_t& value) { auto encoded_timestamp = std::get>(value); m_timestamps.push_back(encoded_timestamp.second); m_timestamp_encodings.push_back(encoded_timestamp.first); + return 2 * sizeof(int64_t); + ; } void DateStringColumnWriter::store(ZstdCompressor& compressor) { diff --git a/components/core/src/clp_s/ColumnWriter.hpp b/components/core/src/clp_s/ColumnWriter.hpp index 37259b15b..7282cf7ea 100644 --- a/components/core/src/clp_s/ColumnWriter.hpp +++ b/components/core/src/clp_s/ColumnWriter.hpp @@ -27,9 +27,9 @@ class BaseColumnWriter { /** * Adds a value to the column * @param value - * @param size + * @return the size of the encoded data appended to this column in bytes */ - virtual void add_value(ParsedMessage::variable_t& value, size_t& size) = 0; + virtual size_t add_value(ParsedMessage::variable_t& value) = 0; /** * Stores the column to a compressed file. @@ -42,7 +42,7 @@ class BaseColumnWriter { * size plus the sum of sizes returned by add_value is equal to the total size of data that will * be written to the compressor in bytes. * - * @return the total size of header data that will + * @return the total size of header data that will be written to the compressor in bytes */ virtual size_t get_total_header_size() const { return 0; } @@ -59,7 +59,7 @@ class Int64ColumnWriter : public BaseColumnWriter { ~Int64ColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; @@ -76,7 +76,7 @@ class FloatColumnWriter : public BaseColumnWriter { ~FloatColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; @@ -93,7 +93,7 @@ class BooleanColumnWriter : public BaseColumnWriter { ~BooleanColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; @@ -117,7 +117,7 @@ class ClpStringColumnWriter : public BaseColumnWriter { ~ClpStringColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; @@ -173,7 +173,7 @@ class VariableStringColumnWriter : public BaseColumnWriter { ~VariableStringColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; @@ -191,7 +191,7 @@ class DateStringColumnWriter : public BaseColumnWriter { ~DateStringColumnWriter() override = default; // Methods inherited from BaseColumnWriter - void add_value(ParsedMessage::variable_t& value, size_t& size) override; + size_t add_value(ParsedMessage::variable_t& value) override; void store(ZstdCompressor& compressor) override; diff --git a/components/core/src/clp_s/CommandLineArguments.hpp b/components/core/src/clp_s/CommandLineArguments.hpp index d86c35303..0904f452a 100644 --- a/components/core/src/clp_s/CommandLineArguments.hpp +++ b/components/core/src/clp_s/CommandLineArguments.hpp @@ -108,7 +108,7 @@ class CommandLineArguments { size_t get_ordered_chunk_size() const { return m_ordered_chunk_size; } - size_t get_min_table_size() const { return m_minimum_table_size; } + size_t get_minimum_table_size() const { return m_minimum_table_size; } private: // Methods diff --git a/components/core/src/clp_s/SchemaWriter.cpp b/components/core/src/clp_s/SchemaWriter.cpp index d3c5d8165..ae3f16f15 100644 --- a/components/core/src/clp_s/SchemaWriter.cpp +++ b/components/core/src/clp_s/SchemaWriter.cpp @@ -9,18 +9,15 @@ void SchemaWriter::append_column(BaseColumnWriter* column_writer) { } size_t SchemaWriter::append_message(ParsedMessage& message) { - int count = 0; - size_t size, total_size; - size = total_size = 0; + int count{}; + size_t total_size{}; for (auto& i : message.get_content()) { - m_columns[count]->add_value(i.second, size); - total_size += size; - count++; + total_size += m_columns[count]->add_value(i.second); + ++count; } for (auto& i : message.get_unordered_content()) { - m_columns[count]->add_value(i, size); - total_size += size; + total_size += m_columns[count]->add_value(i); ++count; } diff --git a/components/core/src/clp_s/clp-s.cpp b/components/core/src/clp_s/clp-s.cpp index a21add4a2..044e6ead6 100644 --- a/components/core/src/clp_s/clp-s.cpp +++ b/components/core/src/clp_s/clp-s.cpp @@ -89,7 +89,7 @@ bool compress(CommandLineArguments const& command_line_arguments) { option.archives_dir = archives_dir.string(); option.target_encoded_size = command_line_arguments.get_target_encoded_size(); option.max_document_size = command_line_arguments.get_max_document_size(); - option.min_table_size = command_line_arguments.get_min_table_size(); + option.min_table_size = command_line_arguments.get_minimum_table_size(); option.compression_level = command_line_arguments.get_compression_level(); option.timestamp_key = command_line_arguments.get_timestamp_key(); option.print_archive_stats = command_line_arguments.print_archive_stats(); From a1dac04aa6d12b76a6e24adaed66fba557f5e81c Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 19 Jul 2024 16:45:22 +0000 Subject: [PATCH 06/21] Address remaining review comments --- components/core/src/clp_s/ArchiveWriter.cpp | 46 ++++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index e64c87f45..b535785fd 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -143,10 +143,29 @@ size_t ArchiveWriter::store_tables() { ); m_table_metadata_compressor.open(m_table_metadata_file_writer, m_compression_level); + /** + * Table metadata schema + * # num tables <64 bit> + * # [offset into file <64 bit> uncompressed size <64 bit>]+ + * # num schemas <64 bit> + * # [table id <64 bit> offset into stream <64 bit> schema id <32 bit> num messages <64 bit>]+ + * + * Schema tables are packed into a series of compression streams. Each of those compression + * streams is identified by a 64 bit table id. In the first half of the metadata we identify + * how many streams there are, and the offset into the file where each compression stream can + * be found. In the second half of the metadata we record how many schema tables there are, + * which compression stream they belong to, the offset into that compression stream where + * they can be found, and how many messages that schema table contains. + * + * We buffer the first half of the metadata in the "table_metadata" vector, and the second half + * of the metadata in the "schema_metadata" vector as we compress the tables. The metadata is + * flushed once all of the schema tables have been compressed. + */ + using schema_map_it = decltype(m_id_to_schema_writer)::iterator; std::vector schemas; std::vector> table_metadata; - std::vector> schema_metadata; + std::vector> schema_metadata; schema_metadata.reserve(m_id_to_schema_writer.size()); schemas.reserve(m_id_to_schema_writer.size()); @@ -159,25 +178,25 @@ size_t ArchiveWriter::store_tables() { }; std::sort(schemas.begin(), schemas.end(), comp); - size_t current_table_size = 0; + size_t current_stream_offset = 0; size_t current_table_id = 0; size_t current_table_file_offset = 0; m_tables_compressor.open(m_tables_file_writer, m_compression_level); for (auto it : schemas) { it->second->store(m_tables_compressor); schema_metadata.emplace_back( - it->first, - it->second->get_num_messages(), current_table_id, - current_table_size + current_stream_offset, + it->first, + it->second->get_num_messages() ); - current_table_size += it->second->get_total_uncompressed_size(); + current_stream_offset += it->second->get_total_uncompressed_size(); delete it->second; - if (current_table_size > m_min_table_size || schemas.size() == schema_metadata.size()) { - table_metadata.emplace_back(current_table_file_offset, current_table_size); + if (current_stream_offset > m_min_table_size || schemas.size() == schema_metadata.size()) { + table_metadata.emplace_back(current_table_file_offset, current_stream_offset); m_tables_compressor.close(); - current_table_size = 0; + current_stream_offset = 0; ++current_table_id; current_table_file_offset = m_tables_file_writer.get_pos(); @@ -187,11 +206,6 @@ size_t ArchiveWriter::store_tables() { } } - // table metadata schema - // # num tables <64 bit> - // # [offset into file <64 bit> uncompressed size <64 bit>]+ - // # num schemas <64 bit> - // # [table id <64 bit> offset into table <64 bit> schema id <32 bit> num messages <64 bit>]+ m_table_metadata_compressor.write_numeric_value(table_metadata.size()); for (auto& [file_offset, uncompressed_size] : table_metadata) { m_table_metadata_compressor.write_numeric_value(file_offset); @@ -199,9 +213,9 @@ size_t ArchiveWriter::store_tables() { } m_table_metadata_compressor.write_numeric_value(schema_metadata.size()); - for (auto& [schema_id, num_messages, table_id, table_offset] : schema_metadata) { + for (auto& [table_id, stream_offset, schema_id, num_messages] : schema_metadata) { m_table_metadata_compressor.write_numeric_value(table_id); - m_table_metadata_compressor.write_numeric_value(table_offset); + m_table_metadata_compressor.write_numeric_value(stream_offset); m_table_metadata_compressor.write_numeric_value(schema_id); m_table_metadata_compressor.write_numeric_value(num_messages); } From cd6c0b861eeb7a098b9f5966a2f80e23717c1fb8 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 23 Jul 2024 23:27:57 +0000 Subject: [PATCH 07/21] Add back decompression side of table packing This reverts commit 794a7327da1a0905bc2b3b0f2bff34259d7bc78b. --- components/core/src/clp_s/ArchiveReader.cpp | 114 +++++++++++++------- components/core/src/clp_s/ArchiveReader.hpp | 24 ++++- components/core/src/clp_s/CMakeLists.txt | 2 + components/core/src/clp_s/SchemaReader.cpp | 18 ++-- components/core/src/clp_s/SchemaReader.hpp | 19 ++-- components/core/src/clp_s/TableReader.cpp | 108 +++++++++++++++++++ components/core/src/clp_s/TableReader.hpp | 89 +++++++++++++++ components/core/src/clp_s/search/Output.cpp | 2 +- 8 files changed, 312 insertions(+), 64 deletions(-) create mode 100644 components/core/src/clp_s/TableReader.cpp create mode 100644 components/core/src/clp_s/TableReader.hpp diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index f211a0707..3c62cc63b 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -27,8 +27,8 @@ void ArchiveReader::open(string_view archives_dir, string_view archive_id) { m_schema_tree = ReaderUtils::read_schema_tree(archive_path_str); m_schema_map = ReaderUtils::read_schemas(archive_path_str); - m_tables_file_reader.open(archive_path_str + constants::cArchiveTablesFile); m_table_metadata_file_reader.open(archive_path_str + constants::cArchiveTableMetadataFile); + m_table_reader.open_tables(archive_path_str + constants::cArchiveTablesFile); } void ArchiveReader::read_metadata() { @@ -38,6 +38,8 @@ void ArchiveReader::read_metadata() { cDecompressorFileReadBufferCapacity ); + m_table_reader.read_metadata(m_table_metadata_decompressor); + size_t num_schemas; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_schemas); ErrorCodeSuccess != error) @@ -45,39 +47,65 @@ void ArchiveReader::read_metadata() { throw OperationFailed(error, __FILENAME__, __LINE__); } - for (size_t i = 0; i < num_schemas; i++) { - int32_t schema_id; - uint64_t num_messages; + bool prev_metadata_initialized{false}; + SchemaReader::SchemaMetadata prev_metadata{}; + int32_t prev_schema_id{}; + for (size_t i = 0; i < num_schemas; ++i) { + size_t table_id; size_t table_offset; - size_t uncompressed_size; + int32_t schema_id; + size_t num_messages; - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); + if (table_offset > m_table_reader.get_uncompressed_table_size(table_id)) { + throw OperationFailed(ErrorCodeCorrupt, __FILENAME__, __LINE__); + } + + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(schema_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(uncompressed_size); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_messages); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - m_id_to_table_metadata[schema_id] = {num_messages, table_offset, uncompressed_size}; + if (prev_metadata_initialized) { + size_t uncompressed_size{0}; + if (table_id != prev_metadata.table_id) { + uncompressed_size + = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) + - prev_metadata.table_offset; + } else { + uncompressed_size = table_offset - prev_metadata.table_offset; + } + prev_metadata.uncompressed_size = uncompressed_size; + m_id_to_schema_metadata[prev_schema_id] = prev_metadata; + } else { + prev_metadata_initialized = true; + } + prev_metadata = {table_id, table_offset, num_messages, 0}; + prev_schema_id = schema_id; m_schema_ids.push_back(schema_id); } + prev_metadata.uncompressed_size + = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) + - prev_metadata.table_offset; + m_id_to_schema_metadata[prev_schema_id] = prev_metadata; m_table_metadata_decompressor.close(); } @@ -89,14 +117,12 @@ void ArchiveReader::read_dictionaries_and_metadata() { read_metadata(); } -SchemaReader& ArchiveReader::read_table( +SchemaReader& ArchiveReader::read_schema_table( int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records ) { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - - if (m_id_to_table_metadata.count(schema_id) == 0) { + if (m_id_to_schema_metadata.count(schema_id) == 0) { throw OperationFailed(ErrorCodeFileNotFound, __FILENAME__, __LINE__); } @@ -107,30 +133,26 @@ SchemaReader& ArchiveReader::read_table( should_marshal_records ); - m_tables_file_reader.try_seek_from_begin(m_id_to_table_metadata[schema_id].offset); - m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); - m_schema_reader.load( - m_tables_decompressor, - m_id_to_table_metadata[schema_id].uncompressed_size - ); - m_tables_decompressor.close_for_reuse(); + auto& schema_metadata = m_id_to_schema_metadata[schema_id]; + auto table_buffer = read_table(schema_metadata.table_id, true); + m_schema_reader + .load(table_buffer, schema_metadata.table_offset, schema_metadata.uncompressed_size); return m_schema_reader; } std::vector> ArchiveReader::read_all_tables() { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - std::vector> readers; - readers.reserve(m_id_to_table_metadata.size()); - for (auto const& [id, table_metadata] : m_id_to_table_metadata) { + readers.reserve(m_id_to_schema_metadata.size()); + for (auto schema_id : m_schema_ids) { auto schema_reader = std::make_shared(); - initialize_schema_reader(*schema_reader, id, true, true); - - m_tables_file_reader.try_seek_from_begin(table_metadata.offset); - m_tables_decompressor.open(m_tables_file_reader, cDecompressorFileReadBufferCapacity); - schema_reader->load(m_tables_decompressor, table_metadata.uncompressed_size); - m_tables_decompressor.close_for_reuse(); - + initialize_schema_reader(*schema_reader, schema_id, true, true); + auto& schema_metadata = m_id_to_schema_metadata[schema_id]; + auto table_buffer = read_table(schema_metadata.table_id, false); + schema_reader->load( + table_buffer, + schema_metadata.table_offset, + schema_metadata.uncompressed_size + ); readers.push_back(std::move(schema_reader)); } return readers; @@ -237,7 +259,7 @@ void ArchiveReader::initialize_schema_reader( m_schema_tree, schema_id, schema.get_ordered_schema_view(), - m_id_to_table_metadata[schema_id].num_messages, + m_id_to_schema_metadata[schema_id].num_messages, should_marshal_records ); auto timestamp_column_ids = m_timestamp_dict->get_authoritative_timestamp_column_ids(); @@ -284,9 +306,8 @@ void ArchiveReader::initialize_schema_reader( void ArchiveReader::store(FileWriter& writer) { std::string message; - - for (auto& [id, table_metadata] : m_id_to_table_metadata) { - auto& schema_reader = read_table(id, false, true); + for (auto schema_id : m_schema_ids) { + auto& schema_reader = read_schema_table(schema_id, false, true); while (schema_reader.get_next_message(message)) { writer.write(message.c_str(), message.length()); } @@ -304,11 +325,28 @@ void ArchiveReader::close() { m_array_dict->close(); m_timestamp_dict->close(); - m_tables_file_reader.close(); + m_table_reader.close(); m_table_metadata_file_reader.close(); - m_id_to_table_metadata.clear(); + m_id_to_schema_metadata.clear(); m_schema_ids.clear(); + m_cur_table_id = 0; + m_table_buffer.reset(); + m_table_buffer_size = 0ULL; } +std::shared_ptr ArchiveReader::read_table(size_t table_id, bool reuse_buffer) { + if (nullptr != m_table_buffer && m_cur_table_id == table_id) { + return m_table_buffer; + } + + if (false == reuse_buffer) { + m_table_buffer.reset(); + m_table_buffer_size = 0; + } + + m_table_reader.read_table(table_id, m_table_buffer, m_table_buffer_size); + m_cur_table_id = table_id; + return m_table_buffer; +} } // namespace clp_s diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index 91fcc1a94..02755592a 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -12,6 +12,7 @@ #include "DictionaryReader.hpp" #include "ReaderUtils.hpp" #include "SchemaReader.hpp" +#include "TableReader.hpp" #include "TimestampDictionaryReader.hpp" #include "Utils.hpp" @@ -91,8 +92,11 @@ class ArchiveReader { * @param should_marshal_records * @return the schema reader */ - SchemaReader& - read_table(int32_t schema_id, bool should_extract_timestamp, bool should_marshal_records); + SchemaReader& read_schema_table( + int32_t schema_id, + bool should_extract_timestamp, + bool should_marshal_records + ); /** * Loads all of the tables in the archive and returns SchemaReaders for them. @@ -171,6 +175,14 @@ class ArchiveReader { bool should_marshal_records ); + /** + * Reads a table with given ID from the table reader. If read_table is called multiple times in + * a row for the same table_id a cached buffer is returned. This function allows the caller to + * ask for the same buffer to be reused to read multiple different tables: this can save memory + * allocations, but can only be used when tables are read one at a time. + */ + std::shared_ptr read_table(size_t table_id, bool reuse_buffer); + bool m_is_open; std::string m_archive_id; std::shared_ptr m_var_dict; @@ -181,13 +193,15 @@ class ArchiveReader { std::shared_ptr m_schema_tree; std::shared_ptr m_schema_map; std::vector m_schema_ids; - std::map m_id_to_table_metadata; + std::map m_id_to_schema_metadata; - FileReader m_tables_file_reader; + TableReader m_table_reader; FileReader m_table_metadata_file_reader; - ZstdDecompressor m_tables_decompressor; ZstdDecompressor m_table_metadata_decompressor; SchemaReader m_schema_reader; + std::shared_ptr m_table_buffer{}; + size_t m_table_buffer_size{0ULL}; + size_t m_cur_table_id{0ULL}; }; } // namespace clp_s diff --git a/components/core/src/clp_s/CMakeLists.txt b/components/core/src/clp_s/CMakeLists.txt index c8cf08b22..cc1fd78cc 100644 --- a/components/core/src/clp_s/CMakeLists.txt +++ b/components/core/src/clp_s/CMakeLists.txt @@ -78,6 +78,8 @@ set( SchemaTree.hpp SchemaWriter.cpp SchemaWriter.hpp + TableReader.cpp + TableReader.hpp TimestampDictionaryReader.cpp TimestampDictionaryReader.hpp TimestampDictionaryWriter.cpp diff --git a/components/core/src/clp_s/SchemaReader.cpp b/components/core/src/clp_s/SchemaReader.cpp index 03edebf69..265e772d8 100644 --- a/components/core/src/clp_s/SchemaReader.cpp +++ b/components/core/src/clp_s/SchemaReader.cpp @@ -37,17 +37,13 @@ void SchemaReader::mark_column_as_timestamp(BaseColumnReader* column_reader) { } } -void SchemaReader::load(ZstdDecompressor& decompressor, size_t uncompressed_size) { - if (uncompressed_size > m_table_buffer_size) { - m_table_buffer = std::make_unique(uncompressed_size); - m_table_buffer_size = uncompressed_size; - } - auto error = decompressor.try_read_exact_length(m_table_buffer.get(), uncompressed_size); - if (ErrorCodeSuccess != error) { - throw OperationFailed(error, __FILENAME__, __LINE__); - } - - BufferViewReader buffer_reader{m_table_buffer.get(), uncompressed_size}; +void SchemaReader::load( + std::shared_ptr table_buffer, + size_t offset, + size_t uncompressed_size +) { + m_table_buffer = table_buffer; + BufferViewReader buffer_reader{m_table_buffer.get() + offset, uncompressed_size}; for (auto& reader : m_columns) { reader->load(buffer_reader, m_num_messages); } diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 3639560f6..300bc47c8 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -1,6 +1,7 @@ #ifndef CLP_S_SCHEMAREADER_HPP #define CLP_S_SCHEMAREADER_HPP +#include #include #include #include @@ -11,7 +12,6 @@ #include "FileReader.hpp" #include "JsonSerializer.hpp" #include "SchemaTree.hpp" -#include "ZstdDecompressor.hpp" namespace clp_s { class SchemaReader; @@ -47,9 +47,10 @@ class SchemaReader { : TraceableException(error_code, filename, line_number) {} }; - struct TableMetadata { - uint64_t num_messages; - size_t offset; + struct SchemaMetadata { + size_t table_id; + size_t table_offset; + size_t num_messages; size_t uncompressed_size; }; @@ -130,11 +131,12 @@ class SchemaReader { ); /** - * Loads the encoded messages - * @param decompressor + * Loads the encoded messages from a shared buffer starting at a given offset + * @param table_buffer + * @param offset * @param uncompressed_size */ - void load(ZstdDecompressor& decompressor, size_t uncompressed_size); + void load(std::shared_ptr table_buffer, size_t offset, size_t uncompressed_size); /** * Gets next message @@ -277,8 +279,7 @@ class SchemaReader { std::unordered_map m_column_map; std::vector m_columns; std::vector m_reordered_columns; - std::unique_ptr m_table_buffer; - size_t m_table_buffer_size{0}; + std::shared_ptr m_table_buffer; BaseColumnReader* m_timestamp_column; std::function m_get_timestamp; diff --git a/components/core/src/clp_s/TableReader.cpp b/components/core/src/clp_s/TableReader.cpp new file mode 100644 index 000000000..969824356 --- /dev/null +++ b/components/core/src/clp_s/TableReader.cpp @@ -0,0 +1,108 @@ +#include "TableReader.hpp" + +namespace clp_s { + +void TableReader::read_metadata(ZstdDecompressor& decompressor) { + switch (m_state) { + case TableReaderState::Uninitialized: + m_state = TableReaderState::MetadataRead; + break; + case TableReaderState::TablesOpened: + m_state = TableReaderState::TablesOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + + size_t num_tables; + if (auto error = decompressor.try_read_numeric_value(num_tables); ErrorCodeSuccess != error) { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_table_metadata.reserve(num_tables); + + for (size_t i = 0; i < num_tables; ++i) { + size_t file_offset; + size_t uncompressed_size; + + if (auto error = decompressor.try_read_numeric_value(file_offset); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + if (auto error = decompressor.try_read_numeric_value(uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + m_table_metadata.emplace_back(file_offset, uncompressed_size); + } +} + +void TableReader::open_tables(std::string const& tables_file_path) { + switch (m_state) { + case TableReaderState::Uninitialized: + m_state = TableReaderState::TablesOpened; + break; + case TableReaderState::MetadataRead: + m_state = TableReaderState::TablesOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_tables_reader.open(tables_file_path); +} + +void TableReader::close() { + switch (m_state) { + case TableReaderState::TablesOpened: + case TableReaderState::TablesOpenedAndMetadataRead: + case TableReaderState::ReadingTables: + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_tables_reader.close(); + m_previous_table_id = 0; + m_table_metadata.clear(); + m_state = TableReaderState::Uninitialized; +} + +void TableReader::read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size) { + constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB + if (table_id > m_table_metadata.size()) { + throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); + } + + switch (m_state) { + case TableReaderState::TablesOpenedAndMetadataRead: + m_state = TableReaderState::ReadingTables; + break; + case TableReaderState::ReadingTables: + if (m_previous_table_id >= table_id) { + throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); + } + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_previous_table_id = table_id; + + auto& [file_offset, uncompressed_size] = m_table_metadata[table_id]; + m_tables_reader.try_seek_from_begin(file_offset); + m_tables_decompressor.open(m_tables_reader, cDecompressorFileReadBufferCapacity); + if (buf_size < uncompressed_size) { + // make_shared is supposed to work here for c++20, but it seems like the compiler version + // we use doesn't support it, so we convert a unique_ptr to a shared_ptr instead. + buf = std::make_unique(uncompressed_size); + buf_size = uncompressed_size; + } + if (auto error = m_tables_decompressor.try_read_exact_length(buf.get(), uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_tables_decompressor.close_for_reuse(); +} +} // namespace clp_s diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp new file mode 100644 index 000000000..116d1b957 --- /dev/null +++ b/components/core/src/clp_s/TableReader.hpp @@ -0,0 +1,89 @@ +#ifndef CLP_S_TABLEREADER_HPP +#define CLP_S_TABLEREADER_HPP + +#include +#include +#include +#include + +#include "FileReader.hpp" +#include "ZstdDecompressor.hpp" + +namespace clp_s { +/** + * TableReader ensures that the tables section of an archive is read safely. Any attempt to read the + * tables section without loading the tables metadata, and any attempt to read tables section out of + * order will throw. As well, any incorrect usage of this class (e.g. closing without opening) will + * throw. + */ +class TableReader { +public: + class OperationFailed : public TraceableException { + public: + // Constructors + OperationFailed(ErrorCode error_code, char const* const filename, int line_number) + : TraceableException(error_code, filename, line_number) {} + }; + + struct TableMetadata { + size_t file_offset; + size_t uncompressed_size; + }; + + /** + * Reads table metadata from the provided compression stream. Must be invoked before reading + * tables. + */ + void read_metadata(ZstdDecompressor& decompressor); + + /** + * Opens a file reader for the tables section. Must be invoked before reading tables. + */ + void open_tables(std::string const& tables_file_path); + + /** + * Closes the file reader for the tables section. + */ + void close(); + + /** + * Decompresses a table with a given table_id and returns it. This function must be called + * strictly in ascending table_id order. If this function is called twice for the same table or + * if a table with lower id is requested after a table with higher id then an error is thrown. + * + * Note: the buffer and buffer size are returned by reference. This is to support the use case + * where the caller wants to re-use the same buffer for multiple tables to avoid allocations + * when they already have a sufficiently large buffer. If no buffer is provided or the provided + * buffer is too small calling read_table will create a buffer exactly as large as the table + * being decompressed. + * + * @param table_id + * @param buf + * @param buf_size + * @return a shared_ptr to a buffer containing the requested table + */ + void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); + + size_t get_uncompressed_table_size(size_t table_id) const { + return m_table_metadata.at(table_id).uncompressed_size; + } + +private: + enum TableReaderState { + Uninitialized, + MetadataRead, + TablesOpened, + TablesOpenedAndMetadataRead, + ReadingTables + }; + + std::vector m_table_metadata; + FileReader m_tables_reader; + ZstdDecompressor m_tables_decompressor; + TableReaderState m_state{TableReaderState::Uninitialized}; + size_t m_previous_table_id{0ULL}; +}; + +} // namespace clp_s + +#endif // CLP_S_TABLEREADER_HPP diff --git a/components/core/src/clp_s/search/Output.cpp b/components/core/src/clp_s/search/Output.cpp index b6a3b8fe0..4d36b4e29 100644 --- a/components/core/src/clp_s/search/Output.cpp +++ b/components/core/src/clp_s/search/Output.cpp @@ -84,7 +84,7 @@ bool Output::filter() { add_wildcard_columns_to_searched_columns(); - auto& reader = m_archive_reader->read_table( + auto& reader = m_archive_reader->read_schema_table( schema_id, m_output_handler->should_output_metadata(), m_should_marshal_records From 0176ecb1dd5684cd58a7d1b34ec204ff2b6bdb65 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 26 Jul 2024 14:39:33 +0000 Subject: [PATCH 08/21] Fix build issue on MacOS --- components/core/src/clp_s/TableReader.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp index 116d1b957..b7a6c2a0e 100644 --- a/components/core/src/clp_s/TableReader.hpp +++ b/components/core/src/clp_s/TableReader.hpp @@ -26,6 +26,8 @@ class TableReader { }; struct TableMetadata { + TableMetadata(size_t offset, size_t size) : file_offset(offset), uncompressed_size(size) {} + size_t file_offset; size_t uncompressed_size; }; From 0f6b354ac5fe4028db116038cf4639c6df1161aa Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Thu, 1 Aug 2024 20:16:54 -0400 Subject: [PATCH 09/21] Apply suggestions from code review Co-authored-by: wraymo <37269683+wraymo@users.noreply.github.com> --- components/core/src/clp_s/ArchiveWriter.cpp | 1 - components/core/src/clp_s/TableReader.hpp | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index b535785fd..32e375353 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -161,7 +161,6 @@ size_t ArchiveWriter::store_tables() { * of the metadata in the "schema_metadata" vector as we compress the tables. The metadata is * flushed once all of the schema tables have been compressed. */ - using schema_map_it = decltype(m_id_to_schema_writer)::iterator; std::vector schemas; std::vector> table_metadata; diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp index b7a6c2a0e..579aef696 100644 --- a/components/core/src/clp_s/TableReader.hpp +++ b/components/core/src/clp_s/TableReader.hpp @@ -66,7 +66,7 @@ class TableReader { */ void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); - size_t get_uncompressed_table_size(size_t table_id) const { + [[nodiscard]] size_t get_uncompressed_table_size(size_t table_id) const { return m_table_metadata.at(table_id).uncompressed_size; } From 2c68328cbd60a9852f8715d16ed775272017fcf0 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 2 Aug 2024 00:37:32 +0000 Subject: [PATCH 10/21] Update docstrings and minor refactor --- components/core/src/clp_s/ArchiveReader.hpp | 4 ++++ components/core/src/clp_s/TableReader.cpp | 6 +++--- components/core/src/clp_s/TableReader.hpp | 11 +++++++---- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index 02755592a..b5f043385 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -180,6 +180,10 @@ class ArchiveReader { * a row for the same table_id a cached buffer is returned. This function allows the caller to * ask for the same buffer to be reused to read multiple different tables: this can save memory * allocations, but can only be used when tables are read one at a time. + * @param table_id + * @param reuse_buffer when true the same buffer is reused across invocations, overwriting data + * returned previous calls to read_table + * @return a buffer containing the decompressed table identified by table_id */ std::shared_ptr read_table(size_t table_id, bool reuse_buffer); diff --git a/components/core/src/clp_s/TableReader.cpp b/components/core/src/clp_s/TableReader.cpp index 969824356..f23858e7b 100644 --- a/components/core/src/clp_s/TableReader.cpp +++ b/components/core/src/clp_s/TableReader.cpp @@ -64,7 +64,7 @@ void TableReader::close() { throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); } m_tables_reader.close(); - m_previous_table_id = 0; + m_prev_table_id = 0; m_table_metadata.clear(); m_state = TableReaderState::Uninitialized; } @@ -80,14 +80,14 @@ void TableReader::read_table(size_t table_id, std::shared_ptr& buf, size m_state = TableReaderState::ReadingTables; break; case TableReaderState::ReadingTables: - if (m_previous_table_id >= table_id) { + if (m_prev_table_id >= table_id) { throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); } break; default: throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); } - m_previous_table_id = table_id; + m_prev_table_id = table_id; auto& [file_offset, uncompressed_size] = m_table_metadata[table_id]; m_tables_reader.try_seek_from_begin(file_offset); diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp index 579aef696..e0e42bf8d 100644 --- a/components/core/src/clp_s/TableReader.hpp +++ b/components/core/src/clp_s/TableReader.hpp @@ -35,11 +35,13 @@ class TableReader { /** * Reads table metadata from the provided compression stream. Must be invoked before reading * tables. + * @param decompressor an open ZstdDecompressor pointing to the table metadata */ void read_metadata(ZstdDecompressor& decompressor); /** * Opens a file reader for the tables section. Must be invoked before reading tables. + * @param tables_file_path the path to the tables file for the archive being read */ void open_tables(std::string const& tables_file_path); @@ -60,9 +62,10 @@ class TableReader { * being decompressed. * * @param table_id - * @param buf - * @param buf_size - * @return a shared_ptr to a buffer containing the requested table + * @param buf a shared ptr to the buffer where the table will be read. The buffer gets resized + * if it is too small to contain the requested table. + * @param buf_size the size of the underlying buffer owned by buf -- passed and updated by + * reference */ void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); @@ -83,7 +86,7 @@ class TableReader { FileReader m_tables_reader; ZstdDecompressor m_tables_decompressor; TableReaderState m_state{TableReaderState::Uninitialized}; - size_t m_previous_table_id{0ULL}; + size_t m_prev_table_id{0ULL}; }; } // namespace clp_s From 88337df0b1e7f898622ed8b14c4991eb13ab787a Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Tue, 13 Aug 2024 22:43:54 +0000 Subject: [PATCH 11/21] Rename TableReader -> PackedStreamReader and do associated variable renaming --- components/core/src/clp_s/ArchiveReader.cpp | 62 +++++----- components/core/src/clp_s/ArchiveReader.hpp | 22 ++-- components/core/src/clp_s/ArchiveWriter.cpp | 28 ++--- components/core/src/clp_s/CMakeLists.txt | 4 +- .../core/src/clp_s/PackedStreamReader.cpp | 113 ++++++++++++++++++ .../core/src/clp_s/PackedStreamReader.hpp | 97 +++++++++++++++ components/core/src/clp_s/SchemaReader.cpp | 6 +- components/core/src/clp_s/SchemaReader.hpp | 10 +- components/core/src/clp_s/TableReader.cpp | 108 ----------------- components/core/src/clp_s/TableReader.hpp | 94 --------------- 10 files changed, 276 insertions(+), 268 deletions(-) create mode 100644 components/core/src/clp_s/PackedStreamReader.cpp create mode 100644 components/core/src/clp_s/PackedStreamReader.hpp delete mode 100644 components/core/src/clp_s/TableReader.cpp delete mode 100644 components/core/src/clp_s/TableReader.hpp diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index 3c62cc63b..0a1d0850b 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -28,7 +28,7 @@ void ArchiveReader::open(string_view archives_dir, string_view archive_id) { m_schema_map = ReaderUtils::read_schemas(archive_path_str); m_table_metadata_file_reader.open(archive_path_str + constants::cArchiveTableMetadataFile); - m_table_reader.open_tables(archive_path_str + constants::cArchiveTablesFile); + m_stream_reader.open_packed_streams(archive_path_str + constants::cArchiveTablesFile); } void ArchiveReader::read_metadata() { @@ -38,7 +38,7 @@ void ArchiveReader::read_metadata() { cDecompressorFileReadBufferCapacity ); - m_table_reader.read_metadata(m_table_metadata_decompressor); + m_stream_reader.read_metadata(m_table_metadata_decompressor); size_t num_schemas; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_schemas); @@ -51,24 +51,24 @@ void ArchiveReader::read_metadata() { SchemaReader::SchemaMetadata prev_metadata{}; int32_t prev_schema_id{}; for (size_t i = 0; i < num_schemas; ++i) { - size_t table_id; - size_t table_offset; + size_t stream_id; + size_t stream_offset; int32_t schema_id; size_t num_messages; - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_id); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(stream_id); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (auto error = m_table_metadata_decompressor.try_read_numeric_value(table_offset); + if (auto error = m_table_metadata_decompressor.try_read_numeric_value(stream_offset); ErrorCodeSuccess != error) { throw OperationFailed(error, __FILENAME__, __LINE__); } - if (table_offset > m_table_reader.get_uncompressed_table_size(table_id)) { + if (stream_offset > m_stream_reader.get_uncompressed_stream_size(stream_id)) { throw OperationFailed(ErrorCodeCorrupt, __FILENAME__, __LINE__); } @@ -86,25 +86,25 @@ void ArchiveReader::read_metadata() { if (prev_metadata_initialized) { size_t uncompressed_size{0}; - if (table_id != prev_metadata.table_id) { + if (stream_id != prev_metadata.stream_id) { uncompressed_size - = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) - - prev_metadata.table_offset; + = m_stream_reader.get_uncompressed_stream_size(prev_metadata.stream_id) + - prev_metadata.stream_offset; } else { - uncompressed_size = table_offset - prev_metadata.table_offset; + uncompressed_size = stream_offset - prev_metadata.stream_offset; } prev_metadata.uncompressed_size = uncompressed_size; m_id_to_schema_metadata[prev_schema_id] = prev_metadata; } else { prev_metadata_initialized = true; } - prev_metadata = {table_id, table_offset, num_messages, 0}; + prev_metadata = {stream_id, stream_offset, num_messages, 0}; prev_schema_id = schema_id; m_schema_ids.push_back(schema_id); } prev_metadata.uncompressed_size - = m_table_reader.get_uncompressed_table_size(prev_metadata.table_id) - - prev_metadata.table_offset; + = m_stream_reader.get_uncompressed_stream_size(prev_metadata.stream_id) + - prev_metadata.stream_offset; m_id_to_schema_metadata[prev_schema_id] = prev_metadata; m_table_metadata_decompressor.close(); } @@ -134,9 +134,9 @@ SchemaReader& ArchiveReader::read_schema_table( ); auto& schema_metadata = m_id_to_schema_metadata[schema_id]; - auto table_buffer = read_table(schema_metadata.table_id, true); + auto stream_buffer = read_stream(schema_metadata.stream_id, true); m_schema_reader - .load(table_buffer, schema_metadata.table_offset, schema_metadata.uncompressed_size); + .load(stream_buffer, schema_metadata.stream_offset, schema_metadata.uncompressed_size); return m_schema_reader; } @@ -147,10 +147,10 @@ std::vector> ArchiveReader::read_all_tables() { auto schema_reader = std::make_shared(); initialize_schema_reader(*schema_reader, schema_id, true, true); auto& schema_metadata = m_id_to_schema_metadata[schema_id]; - auto table_buffer = read_table(schema_metadata.table_id, false); + auto stream_buffer = read_stream(schema_metadata.stream_id, false); schema_reader->load( - table_buffer, - schema_metadata.table_offset, + stream_buffer, + schema_metadata.stream_offset, schema_metadata.uncompressed_size ); readers.push_back(std::move(schema_reader)); @@ -325,28 +325,28 @@ void ArchiveReader::close() { m_array_dict->close(); m_timestamp_dict->close(); - m_table_reader.close(); + m_stream_reader.close(); m_table_metadata_file_reader.close(); m_id_to_schema_metadata.clear(); m_schema_ids.clear(); - m_cur_table_id = 0; - m_table_buffer.reset(); - m_table_buffer_size = 0ULL; + m_cur_stream_id = 0; + m_stream_buffer.reset(); + m_stream_buffer_size = 0ULL; } -std::shared_ptr ArchiveReader::read_table(size_t table_id, bool reuse_buffer) { - if (nullptr != m_table_buffer && m_cur_table_id == table_id) { - return m_table_buffer; +std::shared_ptr ArchiveReader::read_stream(size_t stream_id, bool reuse_buffer) { + if (nullptr != m_stream_buffer && m_cur_stream_id == stream_id) { + return m_stream_buffer; } if (false == reuse_buffer) { - m_table_buffer.reset(); - m_table_buffer_size = 0; + m_stream_buffer.reset(); + m_stream_buffer_size = 0; } - m_table_reader.read_table(table_id, m_table_buffer, m_table_buffer_size); - m_cur_table_id = table_id; - return m_table_buffer; + m_stream_reader.read_stream(stream_id, m_stream_buffer, m_stream_buffer_size); + m_cur_stream_id = stream_id; + return m_stream_buffer; } } // namespace clp_s diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index b5f043385..99a24b9d5 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -10,9 +10,9 @@ #include #include "DictionaryReader.hpp" +#include "PackedStreamReader.hpp" #include "ReaderUtils.hpp" #include "SchemaReader.hpp" -#include "TableReader.hpp" #include "TimestampDictionaryReader.hpp" #include "Utils.hpp" @@ -176,16 +176,16 @@ class ArchiveReader { ); /** - * Reads a table with given ID from the table reader. If read_table is called multiple times in - * a row for the same table_id a cached buffer is returned. This function allows the caller to + * Reads a table with given ID from the table reader. If read_stream is called multiple times in + * a row for the same stream_id a cached buffer is returned. This function allows the caller to * ask for the same buffer to be reused to read multiple different tables: this can save memory * allocations, but can only be used when tables are read one at a time. - * @param table_id + * @param stream_id * @param reuse_buffer when true the same buffer is reused across invocations, overwriting data - * returned previous calls to read_table - * @return a buffer containing the decompressed table identified by table_id + * returned previous calls to read_stream + * @return a buffer containing the decompressed stream identified by stream_id */ - std::shared_ptr read_table(size_t table_id, bool reuse_buffer); + std::shared_ptr read_stream(size_t stream_id, bool reuse_buffer); bool m_is_open; std::string m_archive_id; @@ -199,13 +199,13 @@ class ArchiveReader { std::vector m_schema_ids; std::map m_id_to_schema_metadata; - TableReader m_table_reader; + PackedStreamReader m_stream_reader; FileReader m_table_metadata_file_reader; ZstdDecompressor m_table_metadata_decompressor; SchemaReader m_schema_reader; - std::shared_ptr m_table_buffer{}; - size_t m_table_buffer_size{0ULL}; - size_t m_cur_table_id{0ULL}; + std::shared_ptr m_stream_buffer{}; + size_t m_stream_buffer_size{0ULL}; + size_t m_cur_stream_id{0ULL}; }; } // namespace clp_s diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index 32e375353..084102828 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -144,26 +144,26 @@ size_t ArchiveWriter::store_tables() { m_table_metadata_compressor.open(m_table_metadata_file_writer, m_compression_level); /** - * Table metadata schema - * # num tables <64 bit> + * Packed stream metadata schema + * # num packed streams <64 bit> * # [offset into file <64 bit> uncompressed size <64 bit>]+ * # num schemas <64 bit> - * # [table id <64 bit> offset into stream <64 bit> schema id <32 bit> num messages <64 bit>]+ + * # [stream id <64 bit> offset into stream <64 bit> schema id <32 bit> num messages <64 bit>]+ * * Schema tables are packed into a series of compression streams. Each of those compression - * streams is identified by a 64 bit table id. In the first half of the metadata we identify + * streams is identified by a 64 bit stream id. In the first half of the metadata we identify * how many streams there are, and the offset into the file where each compression stream can * be found. In the second half of the metadata we record how many schema tables there are, * which compression stream they belong to, the offset into that compression stream where * they can be found, and how many messages that schema table contains. * - * We buffer the first half of the metadata in the "table_metadata" vector, and the second half + * We buffer the first half of the metadata in the "stream_metadata" vector, and the second half * of the metadata in the "schema_metadata" vector as we compress the tables. The metadata is * flushed once all of the schema tables have been compressed. */ using schema_map_it = decltype(m_id_to_schema_writer)::iterator; std::vector schemas; - std::vector> table_metadata; + std::vector> stream_metadata; std::vector> schema_metadata; schema_metadata.reserve(m_id_to_schema_writer.size()); @@ -178,13 +178,13 @@ size_t ArchiveWriter::store_tables() { std::sort(schemas.begin(), schemas.end(), comp); size_t current_stream_offset = 0; - size_t current_table_id = 0; + size_t current_stream_id = 0; size_t current_table_file_offset = 0; m_tables_compressor.open(m_tables_file_writer, m_compression_level); for (auto it : schemas) { it->second->store(m_tables_compressor); schema_metadata.emplace_back( - current_table_id, + current_stream_id, current_stream_offset, it->first, it->second->get_num_messages() @@ -193,10 +193,10 @@ size_t ArchiveWriter::store_tables() { delete it->second; if (current_stream_offset > m_min_table_size || schemas.size() == schema_metadata.size()) { - table_metadata.emplace_back(current_table_file_offset, current_stream_offset); + stream_metadata.emplace_back(current_table_file_offset, current_stream_offset); m_tables_compressor.close(); current_stream_offset = 0; - ++current_table_id; + ++current_stream_id; current_table_file_offset = m_tables_file_writer.get_pos(); if (schemas.size() != schema_metadata.size()) { @@ -205,15 +205,15 @@ size_t ArchiveWriter::store_tables() { } } - m_table_metadata_compressor.write_numeric_value(table_metadata.size()); - for (auto& [file_offset, uncompressed_size] : table_metadata) { + m_table_metadata_compressor.write_numeric_value(stream_metadata.size()); + for (auto& [file_offset, uncompressed_size] : stream_metadata) { m_table_metadata_compressor.write_numeric_value(file_offset); m_table_metadata_compressor.write_numeric_value(uncompressed_size); } m_table_metadata_compressor.write_numeric_value(schema_metadata.size()); - for (auto& [table_id, stream_offset, schema_id, num_messages] : schema_metadata) { - m_table_metadata_compressor.write_numeric_value(table_id); + for (auto& [stream_id, stream_offset, schema_id, num_messages] : schema_metadata) { + m_table_metadata_compressor.write_numeric_value(stream_id); m_table_metadata_compressor.write_numeric_value(stream_offset); m_table_metadata_compressor.write_numeric_value(schema_id); m_table_metadata_compressor.write_numeric_value(num_messages); diff --git a/components/core/src/clp_s/CMakeLists.txt b/components/core/src/clp_s/CMakeLists.txt index cc1fd78cc..b3efff232 100644 --- a/components/core/src/clp_s/CMakeLists.txt +++ b/components/core/src/clp_s/CMakeLists.txt @@ -65,6 +65,8 @@ set( JsonParser.cpp JsonParser.hpp JsonSerializer.hpp + PackedStreamReader.cpp + PackedStreamReader.hpp ParsedMessage.hpp ReaderUtils.cpp ReaderUtils.hpp @@ -78,8 +80,6 @@ set( SchemaTree.hpp SchemaWriter.cpp SchemaWriter.hpp - TableReader.cpp - TableReader.hpp TimestampDictionaryReader.cpp TimestampDictionaryReader.hpp TimestampDictionaryWriter.cpp diff --git a/components/core/src/clp_s/PackedStreamReader.cpp b/components/core/src/clp_s/PackedStreamReader.cpp new file mode 100644 index 000000000..24ba02050 --- /dev/null +++ b/components/core/src/clp_s/PackedStreamReader.cpp @@ -0,0 +1,113 @@ +#include "PackedStreamReader.hpp" + +namespace clp_s { + +void PackedStreamReader::read_metadata(ZstdDecompressor& decompressor) { + switch (m_state) { + case PackedStreamReaderState::Uninitialized: + m_state = PackedStreamReaderState::MetadataRead; + break; + case PackedStreamReaderState::PackedStreamsOpened: + m_state = PackedStreamReaderState::PackedStreamsOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + + size_t num_streams; + if (auto error = decompressor.try_read_numeric_value(num_streams); ErrorCodeSuccess != error) { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_stream_metadata.reserve(num_streams); + + for (size_t i = 0; i < num_streams; ++i) { + size_t file_offset; + size_t uncompressed_size; + + if (auto error = decompressor.try_read_numeric_value(file_offset); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + if (auto error = decompressor.try_read_numeric_value(uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + + m_stream_metadata.emplace_back(file_offset, uncompressed_size); + } +} + +void PackedStreamReader::open_packed_streams(std::string const& tables_file_path) { + switch (m_state) { + case PackedStreamReaderState::Uninitialized: + m_state = PackedStreamReaderState::PackedStreamsOpened; + break; + case PackedStreamReaderState::MetadataRead: + m_state = PackedStreamReaderState::PackedStreamsOpenedAndMetadataRead; + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_packed_stream_reader.open(tables_file_path); +} + +void PackedStreamReader::close() { + switch (m_state) { + case PackedStreamReaderState::PackedStreamsOpened: + case PackedStreamReaderState::PackedStreamsOpenedAndMetadataRead: + case PackedStreamReaderState::ReadingPackedStreams: + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_packed_stream_reader.close(); + m_prev_stream_id = 0; + m_stream_metadata.clear(); + m_state = PackedStreamReaderState::Uninitialized; +} + +void PackedStreamReader::read_stream( + size_t stream_id, + std::shared_ptr& buf, + size_t& buf_size +) { + constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB + if (stream_id > m_stream_metadata.size()) { + throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); + } + + switch (m_state) { + case PackedStreamReaderState::PackedStreamsOpenedAndMetadataRead: + m_state = PackedStreamReaderState::ReadingPackedStreams; + break; + case PackedStreamReaderState::ReadingPackedStreams: + if (m_prev_stream_id >= stream_id) { + throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); + } + break; + default: + throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); + } + m_prev_stream_id = stream_id; + + auto& [file_offset, uncompressed_size] = m_stream_metadata[stream_id]; + m_packed_stream_reader.try_seek_from_begin(file_offset); + m_packed_stream_decompressor.open(m_packed_stream_reader, cDecompressorFileReadBufferCapacity); + if (buf_size < uncompressed_size) { + // make_shared is supposed to work here for c++20, but it seems like the compiler version + // we use doesn't support it, so we convert a unique_ptr to a shared_ptr instead. + buf = std::make_unique(uncompressed_size); + buf_size = uncompressed_size; + } + if (auto error + = m_packed_stream_decompressor.try_read_exact_length(buf.get(), uncompressed_size); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } + m_packed_stream_decompressor.close_for_reuse(); +} +} // namespace clp_s diff --git a/components/core/src/clp_s/PackedStreamReader.hpp b/components/core/src/clp_s/PackedStreamReader.hpp new file mode 100644 index 000000000..492e8039a --- /dev/null +++ b/components/core/src/clp_s/PackedStreamReader.hpp @@ -0,0 +1,97 @@ +#ifndef CLP_S_TABLEREADER_HPP +#define CLP_S_TABLEREADER_HPP + +#include +#include +#include +#include + +#include "FileReader.hpp" +#include "ZstdDecompressor.hpp" + +namespace clp_s { +/** + * PackedStreamReader ensures that the tables section of an archive is read safely. Any attempt to + * read the tables section without loading the tables metadata, and any attempt to read tables + * section out of order will throw. As well, any incorrect usage of this class (e.g. closing without + * opening) will throw. + */ +class PackedStreamReader { +public: + class OperationFailed : public TraceableException { + public: + // Constructors + OperationFailed(ErrorCode error_code, char const* const filename, int line_number) + : TraceableException(error_code, filename, line_number) {} + }; + + struct PackedStreamMetadata { + PackedStreamMetadata(size_t offset, size_t size) + : file_offset(offset), + uncompressed_size(size) {} + + size_t file_offset; + size_t uncompressed_size; + }; + + /** + * Reads packed stream metadata from the provided compression stream. Must be invoked before + * reading packed streams. + * @param decompressor an open ZstdDecompressor pointing to the packed stream metadata + */ + void read_metadata(ZstdDecompressor& decompressor); + + /** + * Opens a file reader for the tables section. Must be invoked before reading packed streams. + * @param tables_file_path the path to the tables file for the archive being read + */ + void open_packed_streams(std::string const& tables_file_path); + + /** + * Closes the file reader for the tables section. + */ + void close(); + + /** + * Decompresses a stream with a given stream_id and returns it. This function must be called + * strictly in ascending stream_id order. If this function is called twice for the same stream + * or if a stream with lower id is requested after a stream with higher id then an error is + * thrown. + * + * Note: the buffer and buffer size are returned by reference. This is to support the use case + * where the caller wants to re-use the same buffer for multiple streams to avoid allocations + * when they already have a sufficiently large buffer. If no buffer is provided or the provided + * buffer is too small calling read_stream will create a buffer exactly as large as the stream + * being decompressed. + * + * @param stream_id + * @param buf a shared ptr to the buffer where the stream will be read. The buffer gets resized + * if it is too small to contain the requested stream. + * @param buf_size the size of the underlying buffer owned by buf -- passed and updated by + * reference + */ + void read_stream(size_t stream_id, std::shared_ptr& buf, size_t& buf_size); + + [[nodiscard]] size_t get_uncompressed_stream_size(size_t stream_id) const { + return m_stream_metadata.at(stream_id).uncompressed_size; + } + +private: + enum PackedStreamReaderState { + Uninitialized, + MetadataRead, + PackedStreamsOpened, + PackedStreamsOpenedAndMetadataRead, + ReadingPackedStreams + }; + + std::vector m_stream_metadata; + FileReader m_packed_stream_reader; + ZstdDecompressor m_packed_stream_decompressor; + PackedStreamReaderState m_state{PackedStreamReaderState::Uninitialized}; + size_t m_prev_stream_id{0ULL}; +}; + +} // namespace clp_s + +#endif // CLP_S_TABLEREADER_HPP diff --git a/components/core/src/clp_s/SchemaReader.cpp b/components/core/src/clp_s/SchemaReader.cpp index 265e772d8..763e120a4 100644 --- a/components/core/src/clp_s/SchemaReader.cpp +++ b/components/core/src/clp_s/SchemaReader.cpp @@ -38,12 +38,12 @@ void SchemaReader::mark_column_as_timestamp(BaseColumnReader* column_reader) { } void SchemaReader::load( - std::shared_ptr table_buffer, + std::shared_ptr stream_buffer, size_t offset, size_t uncompressed_size ) { - m_table_buffer = table_buffer; - BufferViewReader buffer_reader{m_table_buffer.get() + offset, uncompressed_size}; + m_stream_buffer = stream_buffer; + BufferViewReader buffer_reader{m_stream_buffer.get() + offset, uncompressed_size}; for (auto& reader : m_columns) { reader->load(buffer_reader, m_num_messages); } diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 300bc47c8..5f5cb10ea 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -48,8 +48,8 @@ class SchemaReader { }; struct SchemaMetadata { - size_t table_id; - size_t table_offset; + size_t stream_id; + size_t stream_offset; size_t num_messages; size_t uncompressed_size; }; @@ -132,11 +132,11 @@ class SchemaReader { /** * Loads the encoded messages from a shared buffer starting at a given offset - * @param table_buffer + * @param stream_buffer * @param offset * @param uncompressed_size */ - void load(std::shared_ptr table_buffer, size_t offset, size_t uncompressed_size); + void load(std::shared_ptr stream_buffer, size_t offset, size_t uncompressed_size); /** * Gets next message @@ -279,7 +279,7 @@ class SchemaReader { std::unordered_map m_column_map; std::vector m_columns; std::vector m_reordered_columns; - std::shared_ptr m_table_buffer; + std::shared_ptr m_stream_buffer; BaseColumnReader* m_timestamp_column; std::function m_get_timestamp; diff --git a/components/core/src/clp_s/TableReader.cpp b/components/core/src/clp_s/TableReader.cpp deleted file mode 100644 index f23858e7b..000000000 --- a/components/core/src/clp_s/TableReader.cpp +++ /dev/null @@ -1,108 +0,0 @@ -#include "TableReader.hpp" - -namespace clp_s { - -void TableReader::read_metadata(ZstdDecompressor& decompressor) { - switch (m_state) { - case TableReaderState::Uninitialized: - m_state = TableReaderState::MetadataRead; - break; - case TableReaderState::TablesOpened: - m_state = TableReaderState::TablesOpenedAndMetadataRead; - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - - size_t num_tables; - if (auto error = decompressor.try_read_numeric_value(num_tables); ErrorCodeSuccess != error) { - throw OperationFailed(error, __FILE__, __LINE__); - } - m_table_metadata.reserve(num_tables); - - for (size_t i = 0; i < num_tables; ++i) { - size_t file_offset; - size_t uncompressed_size; - - if (auto error = decompressor.try_read_numeric_value(file_offset); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - - if (auto error = decompressor.try_read_numeric_value(uncompressed_size); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - - m_table_metadata.emplace_back(file_offset, uncompressed_size); - } -} - -void TableReader::open_tables(std::string const& tables_file_path) { - switch (m_state) { - case TableReaderState::Uninitialized: - m_state = TableReaderState::TablesOpened; - break; - case TableReaderState::MetadataRead: - m_state = TableReaderState::TablesOpenedAndMetadataRead; - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_tables_reader.open(tables_file_path); -} - -void TableReader::close() { - switch (m_state) { - case TableReaderState::TablesOpened: - case TableReaderState::TablesOpenedAndMetadataRead: - case TableReaderState::ReadingTables: - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_tables_reader.close(); - m_prev_table_id = 0; - m_table_metadata.clear(); - m_state = TableReaderState::Uninitialized; -} - -void TableReader::read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size) { - constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - if (table_id > m_table_metadata.size()) { - throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); - } - - switch (m_state) { - case TableReaderState::TablesOpenedAndMetadataRead: - m_state = TableReaderState::ReadingTables; - break; - case TableReaderState::ReadingTables: - if (m_prev_table_id >= table_id) { - throw OperationFailed(ErrorCodeBadParam, __FILE__, __LINE__); - } - break; - default: - throw OperationFailed(ErrorCodeNotReady, __FILE__, __LINE__); - } - m_prev_table_id = table_id; - - auto& [file_offset, uncompressed_size] = m_table_metadata[table_id]; - m_tables_reader.try_seek_from_begin(file_offset); - m_tables_decompressor.open(m_tables_reader, cDecompressorFileReadBufferCapacity); - if (buf_size < uncompressed_size) { - // make_shared is supposed to work here for c++20, but it seems like the compiler version - // we use doesn't support it, so we convert a unique_ptr to a shared_ptr instead. - buf = std::make_unique(uncompressed_size); - buf_size = uncompressed_size; - } - if (auto error = m_tables_decompressor.try_read_exact_length(buf.get(), uncompressed_size); - ErrorCodeSuccess != error) - { - throw OperationFailed(error, __FILE__, __LINE__); - } - m_tables_decompressor.close_for_reuse(); -} -} // namespace clp_s diff --git a/components/core/src/clp_s/TableReader.hpp b/components/core/src/clp_s/TableReader.hpp deleted file mode 100644 index e0e42bf8d..000000000 --- a/components/core/src/clp_s/TableReader.hpp +++ /dev/null @@ -1,94 +0,0 @@ -#ifndef CLP_S_TABLEREADER_HPP -#define CLP_S_TABLEREADER_HPP - -#include -#include -#include -#include - -#include "FileReader.hpp" -#include "ZstdDecompressor.hpp" - -namespace clp_s { -/** - * TableReader ensures that the tables section of an archive is read safely. Any attempt to read the - * tables section without loading the tables metadata, and any attempt to read tables section out of - * order will throw. As well, any incorrect usage of this class (e.g. closing without opening) will - * throw. - */ -class TableReader { -public: - class OperationFailed : public TraceableException { - public: - // Constructors - OperationFailed(ErrorCode error_code, char const* const filename, int line_number) - : TraceableException(error_code, filename, line_number) {} - }; - - struct TableMetadata { - TableMetadata(size_t offset, size_t size) : file_offset(offset), uncompressed_size(size) {} - - size_t file_offset; - size_t uncompressed_size; - }; - - /** - * Reads table metadata from the provided compression stream. Must be invoked before reading - * tables. - * @param decompressor an open ZstdDecompressor pointing to the table metadata - */ - void read_metadata(ZstdDecompressor& decompressor); - - /** - * Opens a file reader for the tables section. Must be invoked before reading tables. - * @param tables_file_path the path to the tables file for the archive being read - */ - void open_tables(std::string const& tables_file_path); - - /** - * Closes the file reader for the tables section. - */ - void close(); - - /** - * Decompresses a table with a given table_id and returns it. This function must be called - * strictly in ascending table_id order. If this function is called twice for the same table or - * if a table with lower id is requested after a table with higher id then an error is thrown. - * - * Note: the buffer and buffer size are returned by reference. This is to support the use case - * where the caller wants to re-use the same buffer for multiple tables to avoid allocations - * when they already have a sufficiently large buffer. If no buffer is provided or the provided - * buffer is too small calling read_table will create a buffer exactly as large as the table - * being decompressed. - * - * @param table_id - * @param buf a shared ptr to the buffer where the table will be read. The buffer gets resized - * if it is too small to contain the requested table. - * @param buf_size the size of the underlying buffer owned by buf -- passed and updated by - * reference - */ - void read_table(size_t table_id, std::shared_ptr& buf, size_t& buf_size); - - [[nodiscard]] size_t get_uncompressed_table_size(size_t table_id) const { - return m_table_metadata.at(table_id).uncompressed_size; - } - -private: - enum TableReaderState { - Uninitialized, - MetadataRead, - TablesOpened, - TablesOpenedAndMetadataRead, - ReadingTables - }; - - std::vector m_table_metadata; - FileReader m_tables_reader; - ZstdDecompressor m_tables_decompressor; - TableReaderState m_state{TableReaderState::Uninitialized}; - size_t m_prev_table_id{0ULL}; -}; - -} // namespace clp_s - -#endif // CLP_S_TABLEREADER_HPP From 237dddf4c9ac75d579542efd65e77f21b553f691 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 19 Sep 2024 17:19:34 +0000 Subject: [PATCH 12/21] Future-proof table-packing metadata --- components/core/src/clp_s/ArchiveReader.cpp | 12 ++++++++++++ components/core/src/clp_s/ArchiveWriter.cpp | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index 0a1d0850b..c35e1904d 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -40,6 +40,18 @@ void ArchiveReader::read_metadata() { m_stream_reader.read_metadata(m_table_metadata_decompressor); + size_t num_separate_column_schemas; + if (auto error + = m_table_metadata_decompressor.try_read_numeric_value(num_separate_column_schemas); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILENAME__, __LINE__); + } + + if (0 != num_separate_column_schemas) { + throw OperationFailed(ErrorCode::ErrorCodeUnsupported, __FILENAME__, __LINE__); + } + size_t num_schemas; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(num_schemas); ErrorCodeSuccess != error) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index 084102828..f11a3c527 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -147,6 +147,8 @@ size_t ArchiveWriter::store_tables() { * Packed stream metadata schema * # num packed streams <64 bit> * # [offset into file <64 bit> uncompressed size <64 bit>]+ + * # num_separate_column_schemas (always 0 in current implementation) + * # [undefined]+ (will be defined once supported) * # num schemas <64 bit> * # [stream id <64 bit> offset into stream <64 bit> schema id <32 bit> num messages <64 bit>]+ * @@ -211,6 +213,11 @@ size_t ArchiveWriter::store_tables() { m_table_metadata_compressor.write_numeric_value(uncompressed_size); } + // The current implementation doesn't store large tables as separate columns, so this is always + // zero. + size_t const num_separate_column_schemas{0}; + m_table_metadata_compressor.write_numeric_value(num_separate_column_schemas); + m_table_metadata_compressor.write_numeric_value(schema_metadata.size()); for (auto& [stream_id, stream_offset, schema_id, num_messages] : schema_metadata) { m_table_metadata_compressor.write_numeric_value(stream_id); From 0cab4cee552ce0b44a71ee3fc0b42109c9de4ddf Mon Sep 17 00:00:00 2001 From: Devin Gibson Date: Thu, 26 Sep 2024 10:34:50 -0400 Subject: [PATCH 13/21] Update components/core/src/clp_s/ArchiveWriter.cpp Co-authored-by: wraymo <37269683+wraymo@users.noreply.github.com> --- components/core/src/clp_s/ArchiveWriter.cpp | 27 ++++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index f11a3c527..c08ed08cb 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -145,12 +145,27 @@ size_t ArchiveWriter::store_tables() { /** * Packed stream metadata schema - * # num packed streams <64 bit> - * # [offset into file <64 bit> uncompressed size <64 bit>]+ - * # num_separate_column_schemas (always 0 in current implementation) - * # [undefined]+ (will be defined once supported) - * # num schemas <64 bit> - * # [stream id <64 bit> offset into stream <64 bit> schema id <32 bit> num messages <64 bit>]+ +* This metadata schema organizes information about packed compression streams and associated + * schema tables within a file. The schema is divided into two primary sections: + * Section 1: Compression Streams Metadata + * - Contains metadata about each compression stream. + * - Structure: + * - Number of packed streams: <64-bit integer> + * - For each stream: + * - Offset into the file: <64-bit integer> + * - Uncompressed size: <64-bit integer> + * - `num_separate_column_schemas` is always 0 in the current implementation. + * - Undefined section for separate column schemas, reserved for future support. + * + * Section 2: Schema Tables Metadata + * - Contains metadata about schema tables associated with each compression stream. + * - Structure: + * - Number of schema tables: <64-bit integer> + * - For each schema table: + * - Stream ID: <64-bit integer> + * - Offset into the stream: <64-bit integer> + * - Schema ID: <32-bit integer> + * - Number of messages: <64-bit integer> * * Schema tables are packed into a series of compression streams. Each of those compression * streams is identified by a 64 bit stream id. In the first half of the metadata we identify From 36c4ef6893979b6e36f953a553caf93f9e197525 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 26 Sep 2024 14:41:41 +0000 Subject: [PATCH 14/21] Address review comments --- components/core/src/clp_s/ArchiveReader.hpp | 8 ++-- components/core/src/clp_s/ArchiveWriter.cpp | 42 ++++++++++----------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/components/core/src/clp_s/ArchiveReader.hpp b/components/core/src/clp_s/ArchiveReader.hpp index 99a24b9d5..b3f23993d 100644 --- a/components/core/src/clp_s/ArchiveReader.hpp +++ b/components/core/src/clp_s/ArchiveReader.hpp @@ -176,10 +176,10 @@ class ArchiveReader { ); /** - * Reads a table with given ID from the table reader. If read_stream is called multiple times in - * a row for the same stream_id a cached buffer is returned. This function allows the caller to - * ask for the same buffer to be reused to read multiple different tables: this can save memory - * allocations, but can only be used when tables are read one at a time. + * Reads a table with given ID from the packed stream reader. If read_stream is called multiple + * times in a row for the same stream_id a cached buffer is returned. This function allows the + * caller to ask for the same buffer to be reused to read multiple different tables: this can + * save memory allocations, but can only be used when tables are read one at a time. * @param stream_id * @param reuse_buffer when true the same buffer is reused across invocations, overwriting data * returned previous calls to read_stream diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index c08ed08cb..7242a236f 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -145,27 +145,27 @@ size_t ArchiveWriter::store_tables() { /** * Packed stream metadata schema -* This metadata schema organizes information about packed compression streams and associated - * schema tables within a file. The schema is divided into two primary sections: - * Section 1: Compression Streams Metadata - * - Contains metadata about each compression stream. - * - Structure: - * - Number of packed streams: <64-bit integer> - * - For each stream: - * - Offset into the file: <64-bit integer> - * - Uncompressed size: <64-bit integer> - * - `num_separate_column_schemas` is always 0 in the current implementation. - * - Undefined section for separate column schemas, reserved for future support. - * - * Section 2: Schema Tables Metadata - * - Contains metadata about schema tables associated with each compression stream. - * - Structure: - * - Number of schema tables: <64-bit integer> - * - For each schema table: - * - Stream ID: <64-bit integer> - * - Offset into the stream: <64-bit integer> - * - Schema ID: <32-bit integer> - * - Number of messages: <64-bit integer> + * This metadata schema organizes information about packed compression streams and associated + * schema tables within a file. The schema is divided into two primary sections: + * Section 1: Compression Streams Metadata + * - Contains metadata about each compression stream. + * - Structure: + * - Number of packed streams: <64-bit integer> + * - For each stream: + * - Offset into the file: <64-bit integer> + * - Uncompressed size: <64-bit integer> + * - `num_separate_column_schemas` is always 0 in the current implementation. + * - Undefined section for separate column schemas, reserved for future support. + * + * Section 2: Schema Tables Metadata + * - Contains metadata about schema tables associated with each compression stream. + * - Structure: + * - Number of schema tables: <64-bit integer> + * - For each schema table: + * - Stream ID: <64-bit integer> + * - Offset into the stream: <64-bit integer> + * - Schema ID: <32-bit integer> + * - Number of messages: <64-bit integer> * * Schema tables are packed into a series of compression streams. Each of those compression * streams is identified by a 64 bit stream id. In the first half of the metadata we identify From 43c923152d4b765c501721b38ea15aa47fb680c8 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 26 Sep 2024 14:44:40 +0000 Subject: [PATCH 15/21] Lint fix --- components/core/src/clp_s/ArchiveWriter.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index 7242a236f..dce3ed1e4 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -145,18 +145,18 @@ size_t ArchiveWriter::store_tables() { /** * Packed stream metadata schema - * This metadata schema organizes information about packed compression streams and associated + * This metadata schema organizes information about packed compression streams and associated * schema tables within a file. The schema is divided into two primary sections: * Section 1: Compression Streams Metadata * - Contains metadata about each compression stream. * - Structure: * - Number of packed streams: <64-bit integer> - * - For each stream: + * - For each stream: * - Offset into the file: <64-bit integer> * - Uncompressed size: <64-bit integer> * - `num_separate_column_schemas` is always 0 in the current implementation. * - Undefined section for separate column schemas, reserved for future support. - * + * * Section 2: Schema Tables Metadata * - Contains metadata about schema tables associated with each compression stream. * - Structure: From 402d62ec4a425645be17146e20587339e71a1a11 Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Fri, 27 Sep 2024 13:12:52 +0000 Subject: [PATCH 16/21] Address rabbit comments --- components/core/src/clp_s/PackedStreamReader.cpp | 8 ++++++-- components/core/src/clp_s/PackedStreamReader.hpp | 6 +++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/components/core/src/clp_s/PackedStreamReader.cpp b/components/core/src/clp_s/PackedStreamReader.cpp index 24ba02050..44eb94e96 100644 --- a/components/core/src/clp_s/PackedStreamReader.cpp +++ b/components/core/src/clp_s/PackedStreamReader.cpp @@ -75,7 +75,7 @@ void PackedStreamReader::read_stream( size_t& buf_size ) { constexpr size_t cDecompressorFileReadBufferCapacity = 64 * 1024; // 64 KB - if (stream_id > m_stream_metadata.size()) { + if (stream_id >= m_stream_metadata.size()) { throw OperationFailed(ErrorCodeCorrupt, __FILE__, __LINE__); } @@ -94,7 +94,11 @@ void PackedStreamReader::read_stream( m_prev_stream_id = stream_id; auto& [file_offset, uncompressed_size] = m_stream_metadata[stream_id]; - m_packed_stream_reader.try_seek_from_begin(file_offset); + if (auto error = m_packed_stream_reader.try_seek_from_begin(file_offset); + ErrorCodeSuccess != error) + { + throw OperationFailed(error, __FILE__, __LINE__); + } m_packed_stream_decompressor.open(m_packed_stream_reader, cDecompressorFileReadBufferCapacity); if (buf_size < uncompressed_size) { // make_shared is supposed to work here for c++20, but it seems like the compiler version diff --git a/components/core/src/clp_s/PackedStreamReader.hpp b/components/core/src/clp_s/PackedStreamReader.hpp index 492e8039a..d9f9af58f 100644 --- a/components/core/src/clp_s/PackedStreamReader.hpp +++ b/components/core/src/clp_s/PackedStreamReader.hpp @@ -1,5 +1,5 @@ -#ifndef CLP_S_TABLEREADER_HPP -#define CLP_S_TABLEREADER_HPP +#ifndef CLP_S_PACKEDSTREAMREADER_HPP +#define CLP_S_PACKEDSTREAMREADER_HPP #include #include @@ -94,4 +94,4 @@ class PackedStreamReader { } // namespace clp_s -#endif // CLP_S_TABLEREADER_HPP +#endif // CLP_S_PACKEDSTREAMREADER_HPP From 45b693ffbc4eee8db4f216d3b1d41b55da90912c Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Thu, 3 Oct 2024 22:26:14 -0400 Subject: [PATCH 17/21] Benign change. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 4a5c4a81d..8e2584557 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ Join us on [Zulip][zulip] to chat with developers and other community members. # Next Steps This is our open-source release which we will be constantly updating with bug fixes, features, etc. -If you would like a feature or want to report a bug, please file an issue and we'll be happy to engage. +If you'd like a feature or want to report a bug, please file an issue and we'll be happy to engage. [bug-report]: https://github.com/y-scope/clp/issues/new?assignees=&labels=bug&template=bug-report.yml [build-package]: http://docs.yscope.com/clp/main/dev-guide/building-package From 0311077b0edc4511ed33527cbe77cc7754d40e76 Mon Sep 17 00:00:00 2001 From: Kirk Rodrigues <2454684+kirkrodrigues@users.noreply.github.com> Date: Thu, 3 Oct 2024 22:26:17 -0400 Subject: [PATCH 18/21] Revert "Benign change." This reverts commit 45b693ffbc4eee8db4f216d3b1d41b55da90912c. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 8e2584557..4a5c4a81d 100644 --- a/README.md +++ b/README.md @@ -87,7 +87,7 @@ Join us on [Zulip][zulip] to chat with developers and other community members. # Next Steps This is our open-source release which we will be constantly updating with bug fixes, features, etc. -If you'd like a feature or want to report a bug, please file an issue and we'll be happy to engage. +If you would like a feature or want to report a bug, please file an issue and we'll be happy to engage. [bug-report]: https://github.com/y-scope/clp/issues/new?assignees=&labels=bug&template=bug-report.yml [build-package]: http://docs.yscope.com/clp/main/dev-guide/building-package From 88fb17ffb87cffbc838253411e6c09a7a755371d Mon Sep 17 00:00:00 2001 From: wraymo Date: Thu, 3 Oct 2024 22:15:36 -0400 Subject: [PATCH 19/21] modify comments about the schema of packed streams in ArchiveWriter --- components/core/src/clp_s/ArchiveWriter.cpp | 22 ++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index dce3ed1e4..dc82d22a7 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -145,8 +145,14 @@ size_t ArchiveWriter::store_tables() { /** * Packed stream metadata schema - * This metadata schema organizes information about packed compression streams and associated - * schema tables within a file. The schema is divided into two primary sections: + * ------------------------------ + * Schema tables are packed into a series of compression streams. Each of those compression + * streams is identified by a 64 bit stream id. In the first half of the metadata we identify + * how many streams there are, and the offset into the file where each compression stream can + * be found. In the second half of the metadata we record how many schema tables there are, + * which compression stream they belong to, the offset into that compression stream where + * they can be found, and how many messages that schema table contains. + * * Section 1: Compression Streams Metadata * - Contains metadata about each compression stream. * - Structure: @@ -154,8 +160,9 @@ size_t ArchiveWriter::store_tables() { * - For each stream: * - Offset into the file: <64-bit integer> * - Uncompressed size: <64-bit integer> - * - `num_separate_column_schemas` is always 0 in the current implementation. - * - Undefined section for separate column schemas, reserved for future support. + * - Number of separate column schemas: <64-bit integer> + * It is always 0 in the current implementation. + * - Undefined section for separate column schemas, reserved for future support. * * Section 2: Schema Tables Metadata * - Contains metadata about schema tables associated with each compression stream. @@ -167,13 +174,6 @@ size_t ArchiveWriter::store_tables() { * - Schema ID: <32-bit integer> * - Number of messages: <64-bit integer> * - * Schema tables are packed into a series of compression streams. Each of those compression - * streams is identified by a 64 bit stream id. In the first half of the metadata we identify - * how many streams there are, and the offset into the file where each compression stream can - * be found. In the second half of the metadata we record how many schema tables there are, - * which compression stream they belong to, the offset into that compression stream where - * they can be found, and how many messages that schema table contains. - * * We buffer the first half of the metadata in the "stream_metadata" vector, and the second half * of the metadata in the "schema_metadata" vector as we compress the tables. The metadata is * flushed once all of the schema tables have been compressed. From efc2260434c357e35f0dfbeb1dbc6ac634a1c64a Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 24 Oct 2024 14:54:34 +0000 Subject: [PATCH 20/21] Clean up suggested by coderabbit --- components/core/src/clp_s/ArchiveReader.cpp | 8 ++++---- components/core/src/clp_s/ArchiveWriter.cpp | 10 +++++----- components/core/src/clp_s/SchemaReader.hpp | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/components/core/src/clp_s/ArchiveReader.cpp b/components/core/src/clp_s/ArchiveReader.cpp index 8c91ef89c..5362d32cc 100644 --- a/components/core/src/clp_s/ArchiveReader.cpp +++ b/components/core/src/clp_s/ArchiveReader.cpp @@ -63,10 +63,10 @@ void ArchiveReader::read_metadata() { SchemaReader::SchemaMetadata prev_metadata{}; int32_t prev_schema_id{}; for (size_t i = 0; i < num_schemas; ++i) { - size_t stream_id; - size_t stream_offset; + uint64_t stream_id; + uint64_t stream_offset; int32_t schema_id; - size_t num_messages; + uint64_t num_messages; if (auto error = m_table_metadata_decompressor.try_read_numeric_value(stream_id); ErrorCodeSuccess != error) @@ -97,7 +97,7 @@ void ArchiveReader::read_metadata() { } if (prev_metadata_initialized) { - size_t uncompressed_size{0}; + uint64_t uncompressed_size{0}; if (stream_id != prev_metadata.stream_id) { uncompressed_size = m_stream_reader.get_uncompressed_stream_size(prev_metadata.stream_id) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index dc82d22a7..ef10d116d 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -180,8 +180,8 @@ size_t ArchiveWriter::store_tables() { */ using schema_map_it = decltype(m_id_to_schema_writer)::iterator; std::vector schemas; - std::vector> stream_metadata; - std::vector> schema_metadata; + std::vector> stream_metadata; + std::vector> schema_metadata; schema_metadata.reserve(m_id_to_schema_writer.size()); schemas.reserve(m_id_to_schema_writer.size()); @@ -194,9 +194,9 @@ size_t ArchiveWriter::store_tables() { }; std::sort(schemas.begin(), schemas.end(), comp); - size_t current_stream_offset = 0; - size_t current_stream_id = 0; - size_t current_table_file_offset = 0; + uint64_t current_stream_offset = 0; + uint64_t current_stream_id = 0; + uint64_t current_table_file_offset = 0; m_tables_compressor.open(m_tables_file_writer, m_compression_level); for (auto it : schemas) { it->second->store(m_tables_compressor); diff --git a/components/core/src/clp_s/SchemaReader.hpp b/components/core/src/clp_s/SchemaReader.hpp index 1dbf36b1d..08651cc39 100644 --- a/components/core/src/clp_s/SchemaReader.hpp +++ b/components/core/src/clp_s/SchemaReader.hpp @@ -50,10 +50,10 @@ class SchemaReader { }; struct SchemaMetadata { - size_t stream_id; - size_t stream_offset; - size_t num_messages; - size_t uncompressed_size; + uint64_t stream_id; + uint64_t stream_offset; + uint64_t num_messages; + uint64_t uncompressed_size; }; // Constructor From 6130155339b98f9e13a9f60e980564552adc988b Mon Sep 17 00:00:00 2001 From: gibber9809 Date: Thu, 24 Oct 2024 15:46:37 +0000 Subject: [PATCH 21/21] Use structs on write side for packed streams --- components/core/src/clp_s/ArchiveWriter.cpp | 21 ++++++++-------- components/core/src/clp_s/ArchiveWriter.hpp | 27 +++++++++++++++++++++ 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/components/core/src/clp_s/ArchiveWriter.cpp b/components/core/src/clp_s/ArchiveWriter.cpp index ef10d116d..369fd79d2 100644 --- a/components/core/src/clp_s/ArchiveWriter.cpp +++ b/components/core/src/clp_s/ArchiveWriter.cpp @@ -1,7 +1,6 @@ #include "ArchiveWriter.hpp" #include -#include #include @@ -180,8 +179,8 @@ size_t ArchiveWriter::store_tables() { */ using schema_map_it = decltype(m_id_to_schema_writer)::iterator; std::vector schemas; - std::vector> stream_metadata; - std::vector> schema_metadata; + std::vector stream_metadata; + std::vector schema_metadata; schema_metadata.reserve(m_id_to_schema_writer.size()); schemas.reserve(m_id_to_schema_writer.size()); @@ -223,9 +222,9 @@ size_t ArchiveWriter::store_tables() { } m_table_metadata_compressor.write_numeric_value(stream_metadata.size()); - for (auto& [file_offset, uncompressed_size] : stream_metadata) { - m_table_metadata_compressor.write_numeric_value(file_offset); - m_table_metadata_compressor.write_numeric_value(uncompressed_size); + for (auto& stream : stream_metadata) { + m_table_metadata_compressor.write_numeric_value(stream.file_offset); + m_table_metadata_compressor.write_numeric_value(stream.uncompressed_size); } // The current implementation doesn't store large tables as separate columns, so this is always @@ -234,11 +233,11 @@ size_t ArchiveWriter::store_tables() { m_table_metadata_compressor.write_numeric_value(num_separate_column_schemas); m_table_metadata_compressor.write_numeric_value(schema_metadata.size()); - for (auto& [stream_id, stream_offset, schema_id, num_messages] : schema_metadata) { - m_table_metadata_compressor.write_numeric_value(stream_id); - m_table_metadata_compressor.write_numeric_value(stream_offset); - m_table_metadata_compressor.write_numeric_value(schema_id); - m_table_metadata_compressor.write_numeric_value(num_messages); + for (auto& schema : schema_metadata) { + m_table_metadata_compressor.write_numeric_value(schema.stream_id); + m_table_metadata_compressor.write_numeric_value(schema.stream_offset); + m_table_metadata_compressor.write_numeric_value(schema.schema_id); + m_table_metadata_compressor.write_numeric_value(schema.num_messages); } m_table_metadata_compressor.close(); diff --git a/components/core/src/clp_s/ArchiveWriter.hpp b/components/core/src/clp_s/ArchiveWriter.hpp index 489a27254..7edfe4491 100644 --- a/components/core/src/clp_s/ArchiveWriter.hpp +++ b/components/core/src/clp_s/ArchiveWriter.hpp @@ -33,6 +33,33 @@ class ArchiveWriter { : TraceableException(error_code, filename, line_number) {} }; + struct StreamMetadata { + StreamMetadata(uint64_t file_offset, uint64_t uncompressed_size) + : file_offset(file_offset), + uncompressed_size(uncompressed_size) {} + + uint64_t file_offset{}; + uint64_t uncompressed_size{}; + }; + + struct SchemaMetadata { + SchemaMetadata( + uint64_t stream_id, + uint64_t stream_offset, + int32_t schema_id, + uint64_t num_messages + ) + : stream_id(stream_id), + stream_offset(stream_offset), + schema_id(schema_id), + num_messages(num_messages) {} + + uint64_t stream_id{}; + uint64_t stream_offset{}; + int32_t schema_id{}; + uint64_t num_messages{}; + }; + // Constructor explicit ArchiveWriter(std::shared_ptr metadata_db) : m_metadata_db(std::move(metadata_db)) {}