Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read / write timeseries schema with the native client #6943

Merged
merged 3 commits into from
Nov 7, 2024

Conversation

bnaecker
Copy link
Collaborator

  • Add methods for converting an array of objects into a data block, or deserializing an array out of one.
  • Use new to/from block methods to read and write timeseries schema in the native format
  • Add native Connection method for inserting data in a block.
  • Implement deserialization of the TableColumns server message type, which is sent to client when it tries to insert data, and is used to check that the block to be inserted matches the structure of the table into which the insertion is directed.
  • Add a build script, which reads the db-init.sql file to extract the enum data type definitions into constants. This is used in the serialization of timeseries schema rows into a block.
  • Closes Use native ClickHouse protocol to read / write timeseries schema #6942

@bnaecker bnaecker force-pushed the manipulate-schema-with-native-client branch 9 times, most recently from b95c5bf to fa3b05f Compare October 31, 2024 19:23
- Add methods for converting an array of objects into a data block, or
  deserializing an array out of one.
- Use new to/from block methods to read and write timeseries schema in
  the native format
- Add native `Connection` method for inserting data in a block.
- Implement deserialization of the `TableColumns` server message type,
  which is sent to client when it tries to insert data, and is used to
  check that the block to be inserted matches the structure of the table
  into which the insertion is directed.
- Add a build script, which reads the `db-init.sql` file to extract the
  enum data type definitions into constants. This is used in the
  serialization of timeseries schema rows into a block.
- Add better handling of HTTP / native addresses in Nexus setup
- Closes #6942
@bnaecker bnaecker force-pushed the manipulate-schema-with-native-client branch from fa3b05f to 6c8b927 Compare October 31, 2024 19:42
@bnaecker
Copy link
Collaborator Author

bnaecker commented Nov 6, 2024

Friendly ping on this @andrewjstone and @jgallagher :)

Copy link
Contributor

@andrewjstone andrewjstone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just minor comments from me.

oximeter/db/build.rs Show resolved Hide resolved
//! As is helpfully noted in the ClickHouse sources at
//! <https://github.com/ClickHouse/ClickHouse/blob/424ecb86e0f04a9422dd01f9216cdbe3c0e314e2/src/Storages/ColumnsDescription.cpp#L132>,
//!
//! > NOTE: Serialization format is insane.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤣

oximeter/db/src/native/io/table_columns.rs Outdated Show resolved Hide resolved
oximeter/db/src/native/io/table_columns.rs Outdated Show resolved Hide resolved
oximeter/db/src/native/block.rs Show resolved Hide resolved
@@ -221,12 +248,109 @@ impl Connection {
self.writer.send(ClientPacket::Query(query)).await?;
probes::packet__sent!(|| "Query");
self.outstanding_query = true;

// If we have data to send, wait for the server to send an empty block
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is nice and easy to understand! Thanks!

- Add comment to new build.rs
- Add test for parsing bad data types with `nom`
- Add message and context to `InvalidPacket` err variant
@bnaecker bnaecker enabled auto-merge (squash) November 6, 2024 23:12
@bnaecker bnaecker merged commit eb8791c into main Nov 7, 2024
17 checks passed
@bnaecker bnaecker deleted the manipulate-schema-with-native-client branch November 7, 2024 00:45
bnaecker added a commit that referenced this pull request Nov 8, 2024
- Most of the work to use the native client during schema updates was
  already done in #6943. One annoyingly small exception is the code that
  lists the tables in the database, which we use for expunging old
  timeseries schema from all the relevant tables. This switches to using
  the native client to list tables too.
- Fixes #7015
bnaecker added a commit that referenced this pull request Nov 8, 2024
- Most of the work to use the native client during schema updates was
  already done in #6943. One annoyingly small exception is the code that
  lists the tables in the database, which we use for expunging old
  timeseries schema from all the relevant tables. This switches to using
  the native client to list tables too.
- Fixes #7015
bnaecker added a commit that referenced this pull request Nov 8, 2024
- Most of the work to use the native client during schema updates was
already done in #6943. One annoyingly small exception is the code that
lists the tables in the database, which we use for expunging old
timeseries schema from all the relevant tables. This switches to using
the native client to list tables too.
- Fixes #7015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use native ClickHouse protocol to read / write timeseries schema
2 participants