Skip to content

Commit

Permalink
Add new datum types to timeseries schema table (#4349)
Browse files Browse the repository at this point in the history
- Fixes #4336
- Adds regression test making sure a query selecting all datum types
from the schema table succeeds, even if it returns zero results. This
uses an iterator over the datum type enum, so it should catch future
changes to the type.
- Adds new datum types to schema table
- Bumps oximeter database version number
  • Loading branch information
bnaecker authored Oct 25, 2023
1 parent d450580 commit 6f4923e
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 3 deletions.
41 changes: 41 additions & 0 deletions oximeter/db/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2634,4 +2634,45 @@ mod tests {
db.cleanup().await.expect("Failed to cleanup ClickHouse server");
logctx.cleanup_successful();
}

// Regression test for https://github.com/oxidecomputer/omicron/issues/4336.
//
// This tests that we can successfully query all extant datum types from the
// schema table. There may be no such values, but the query itself should
// succeed.
#[tokio::test]
async fn test_select_all_datum_types() {
use strum::IntoEnumIterator;
usdt::register_probes().unwrap();
let logctx = test_setup_log("test_update_schema_cache_on_new_sample");
let log = &logctx.log;

// Let the OS assign a port and discover it after ClickHouse starts
let mut db = ClickHouseInstance::new_single_node(0)
.await
.expect("Failed to start ClickHouse");
let address = SocketAddr::new("::1".parse().unwrap(), db.port());

let client = Client::new(address, &log);
client
.init_single_node_db()
.await
.expect("Failed to initialize timeseries database");

// Attempt to select all schema with each datum type.
for ty in oximeter::DatumType::iter() {
let sql = format!(
"SELECT COUNT() \
FROM {}.timeseries_schema WHERE \
datum_type = '{:?}'",
crate::DATABASE_NAME,
crate::model::DbDatumType::from(ty),
);
let res = client.execute_with_body(sql).await.unwrap();
let count = res.trim().parse::<usize>().unwrap();
assert_eq!(count, 0);
}
db.cleanup().await.expect("Failed to cleanup ClickHouse server");
logctx.cleanup_successful();
}
}
20 changes: 19 additions & 1 deletion oximeter/db/src/db-replicated-init.sql
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,25 @@ CREATE TABLE IF NOT EXISTS oximeter.timeseries_schema ON CLUSTER oximeter_cluste
'CumulativeI64' = 6,
'CumulativeF64' = 7,
'HistogramI64' = 8,
'HistogramF64' = 9
'HistogramF64' = 9,
'I8' = 10,
'U8' = 11,
'I16' = 12,
'U16' = 13,
'I32' = 14,
'U32' = 15,
'U64' = 16,
'F32' = 17,
'CumulativeU64' = 18,
'CumulativeF32' = 19,
'HistogramI8' = 20,
'HistogramU8' = 21,
'HistogramI16' = 23,
'HistogramU16' = 23,
'HistogramI32' = 24,
'HistogramU32' = 25,
'HistogramU64' = 26,
'HistogramF32' = 27
),
created DateTime64(9, 'UTC')
)
Expand Down
20 changes: 19 additions & 1 deletion oximeter/db/src/db-single-node-init.sql
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,25 @@ CREATE TABLE IF NOT EXISTS oximeter.timeseries_schema
'CumulativeI64' = 6,
'CumulativeF64' = 7,
'HistogramI64' = 8,
'HistogramF64' = 9
'HistogramF64' = 9,
'I8' = 10,
'U8' = 11,
'I16' = 12,
'U16' = 13,
'I32' = 14,
'U32' = 15,
'U64' = 16,
'F32' = 17,
'CumulativeU64' = 18,
'CumulativeF32' = 19,
'HistogramI8' = 20,
'HistogramU8' = 21,
'HistogramI16' = 22,
'HistogramU16' = 23,
'HistogramI32' = 24,
'HistogramU32' = 25,
'HistogramU64' = 26,
'HistogramF32' = 27
),
created DateTime64(9, 'UTC')
)
Expand Down
2 changes: 1 addition & 1 deletion oximeter/db/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ use uuid::Uuid;
///
/// TODO(#4271): The current implementation of versioning will wipe the metrics
/// database if this number is incremented.
pub const OXIMETER_VERSION: u64 = 1;
pub const OXIMETER_VERSION: u64 = 2;

// Wrapper type to represent a boolean in the database.
//
Expand Down
1 change: 1 addition & 0 deletions oximeter/oximeter/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ pub struct Field {
JsonSchema,
Serialize,
Deserialize,
strum::EnumIter,
)]
#[serde(rename_all = "snake_case")]
pub enum DatumType {
Expand Down

0 comments on commit 6f4923e

Please sign in to comment.