Use Protocol Buffers for the Apache Arrow Flight interface - #335
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR replaces custom Arrow-based encoding in the Flight interface with Protocol Buffers across the server, manager, and embedded layers, centralizing definitions in modelardb_types. It also updates build and CI setup to include the Protobuf compiler.
- Replaced record-batch serialization with protobuf messages for all Flight RPC actions.
- Moved
TimeSeriesTableMetadataand related types intomodelardb_typesand updated serde calls. - Updated Dockerfile and CI workflow to install and configure
protoc; addedprostworkspace deps.
Reviewed Changes
Copilot reviewed 52 out of 52 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/modelardb_server/src/configuration.rs | Added encode_and_serialize to emit server configuration proto. |
| crates/modelardb_manager/src/remote.rs | Switched action payloads to use protobuf encode/decode. |
| crates/modelardb_embedded/src/operations/mod.rs | Updated lossless bound and generated-column parsing for protos. |
| crates/modelardb_common/src/arguments.rs | Removed legacy connection-info utilities. |
| Dockerfile | Installed protobuf-compiler for Protobuf codegen. |
| .github/workflows/build-lint-and-test.yml | Added CI step to set up protoc. |
Comments suppressed due to low confidence (2)
.github/workflows/build-lint-and-test.yml:82
- The added workflow step is indented inconsistently and may break YAML parsing; ensure
- name: Install Protocaligns with other steps understeps:and does not carry an extra hyphen or spaces.
- name: Install Protoc
crates/modelardb_server/Cargo.toml:44
- [nitpick] Ensure workspace dependency syntax matches other crates; the shorthand
prost.workspace = trueis valid but double-check thatprostis declared under[dependencies]and that this matches your workspace's version consolidation.
prost.workspace = true
skejserjensen
requested changes
Jun 24, 2025
skejserjensen
approved these changes
Jun 30, 2025
chrthomsen
approved these changes
Jul 4, 2025
aabduvakhobov
pushed a commit
to aabduvakhobov/ModelarDB-RS
that referenced
this pull request
Aug 4, 2025
…ta#335) * Install dependencies for protocol buffers * Add build file and basic proto file * Mention in user doc that protobuf needs to be installed * Create module in modelardb_types for flight types * Add datafusion error to modelardb_types * Move GeneratedColumn to modelardb_types * Move TimeSeriesTableMetadata to modelardb_types * Move tests for time_series_table_metadata to modelardb_types * Remove time_series_table_metadata from modelardb_storage * Add ModelarDbTypesError to ModelarDbServerError * Add ModelarDbTypesError to ModelarDbEmbeddedError * Fix imports after moving TimeSeriesTableMetadata and GeneratedColumn * Fix issue with Expr.to_string() not providing being consistent * Add function to convert sql expression to generated column * Use sql_expr_to_generated_column() when parsing SQL expressions * Add ArrowError to ModelarDbTypesError * Add schema conversion methods to modelardb_types * Remove old schema conversion functions * Add method to encode normal table metadata as protocol buffer message * Add function to encode error bounds in time series table metadata * Add function to encode time series table metadata as protocol buffer message * Add convenience functions to encode and serialize in one function * Update documentation for CreateTables actions * Convert protocol buffer to protobuf * Use new encoding and serialization when creating tables in client and manager * Use new encoding and serialization when returning from InitializeDatabase * Use new encoding and serialization in create table tests * Remove deprecated functions to convert table metadata to record batches * Add Prost decode error to modelardb types errors * Remove original_expr from GeneratedColumn and add try_from_expr * Install datafusion-proto for serializing datafusion expression * Use bytes to encode generated column expression instead of strings * Remove sql_expr_to_generated_column * Saving generated column expressions as bytes in metadata deltalake * Add method to deserialize and extract table metadata * Use new function to deserialize and extract table metadata * Remove TABLE_METADATA_SCHEMA and table_metadata_from_record_batch * Use Rustfmt * Add protobuf message definition for configuration * Add method to encode and serialize configuration * Remove deprecated configuration schema * Fix configuration related integration tests after change to GetConfiguration * Use protobuf message for UpdateConfiguration request body * Use enum for the updateable setting in UpdateConfigurationRequest * Rename CreateTablesRequest to TableMetadata * Rename GetConfigurationResponse to Configuration * Rename UpdateConfigurationRequest to UpdateConfiguration * Use DatabaseMetadata protobuf message for InitializeDatabase requests * Use DatabaseMetadata protobuf message when initializing nodes * Add StorageConfiguration protobuf message * Add environment variable error to ModelarDbTypesError * Add argument_to_storage_configuration * Add unit tests for argument_to_storage_configuration * Add ModelarDbTypesError to ModelarDbManagerError * Saving StorageConfiguration instead of connection info in RemoteDataFolder * Changed DeltaLake::try_remote_from_connection_info to DeltaLake::try_remote_from_storage_configuration * Changed TableMetadataManager::try_from_connection_info to TableMetadataManager::try_from_storage_configuration * Changed MetadataManager::try_from_connection_info to MetadataManager::try_from_storage_configuration * Changed DataFolder::try_from_connection_info to DataFolder::try_from_storage_configuration * Add NodeMetadata protobuf message * Move Node struct to modelardb_types * Add functions to encode nodes and decode node metadata * Use NodeMetadata in RegisterNode request body * Add ProstDecode to ModelarDbServerError * Add ManagerConfiguration protobuf message and use it in RegisterNode request response * Use NodeMetadata for RemoveNode request body * Remove utility functions related to encoding and decoding arguments using custom encoding * Add unit test for encode and decode node * Add unit test for encoding and decoding table metadata * Move storage configuration up into ManagerMetadata protobuf message * Use new storage configuration protobuf message that is part of ManagerMetadata * Fix doc issues * Remove unused dependencies * Fix clippy issues * Fix doc issues * Add documentation for each message in the proto definition * Remove deprecated integration test * Install protobuf compiler in dockerfile before running cargo build * Make start up process more robust in cluster docker * Add environment variable to fix object store connection issue * Add step to setup protobuf compiler * Fix import error after merge * Disable large error variant clippy warning in workspace * Minor refactoring * Remove unused record batch conversion functions * Remove unused time series table metadata method * Use modelardb_storage util in modelardb_types tests * Use Rustfmt * Pass github token to avoid rate limiting * Using workspace lints in all crates * Add rust-cache to GitHub workflow * Use cargo::rerun-if-changed to avoid unnecessary rebuilds * Update based on comments from @skejserjensen * Fix Clippy issues from new Rust version * Fix Clippy issue in client * Update based on comment from @chrthomsen
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements #328 by removing the custom encoding for data in the Apache Arrow Flight interface and replacing it with Protocol Buffers. Since the Protobuf definition is in
modelardb_types, multiple other types have also been moved to this crate.Finally, the PR also makes other small changes such as encoding generated column expressions as bytes both when transferring them in the table metadata and when saving them to the metadata database. The Docker setup has also been updated to install the Protocol Buffer compiler and make the startup process more robust.
Note that
TimeSeriesTableMetadataand the corresponding tests have just been moved and have not been changed.