diff --git a/generated_types/protos/influxdata/iox/namespace/v1/service.proto b/generated_types/protos/influxdata/iox/namespace/v1/service.proto index 2f8026a4e2..e586bece07 100644 --- a/generated_types/protos/influxdata/iox/namespace/v1/service.proto +++ b/generated_types/protos/influxdata/iox/namespace/v1/service.proto @@ -94,13 +94,9 @@ message Namespace { // NULL means "infinite retention". optional int64 retention_period_ns = 3; - // The service protection limits which the namespace operates within. - ServiceProtectionLimits service_protection_limits = 4; -} - -message ServiceProtectionLimits { // The maximum number of tables which this namespace is allowed to contain. - int32 max_tables = 1; + int32 max_tables = 4; + // The maximum number of columns a table belonging to this namespace may have. - int32 max_columns_per_table = 2; + int32 max_columns_per_table = 5; } diff --git a/ioxd_querier/src/rpc/namespace.rs b/ioxd_querier/src/rpc/namespace.rs index e65429bdec..313868df2d 100644 --- a/ioxd_querier/src/rpc/namespace.rs +++ b/ioxd_querier/src/rpc/namespace.rs @@ -36,10 +36,8 @@ fn namespace_to_proto(namespace: Namespace) -> proto::Namespace { id: namespace.id.get(), name: namespace.name, retention_period_ns: namespace.retention_period_ns, - service_protection_limits: Some(proto::ServiceProtectionLimits { - max_tables: namespace.max_tables, - max_columns_per_table: namespace.max_columns_per_table, - }), + max_tables: namespace.max_tables, + max_columns_per_table: namespace.max_columns_per_table, } } @@ -190,19 +188,15 @@ mod tests { id: 1, name: "namespace2".to_string(), retention_period_ns: TEST_RETENTION_PERIOD_NS, - service_protection_limits: Some(proto::ServiceProtectionLimits { - max_tables: TEST_MAX_TABLES, - max_columns_per_table: TEST_MAX_COLUMNS_PER_TABLE, - }), + max_tables: TEST_MAX_TABLES, + max_columns_per_table: TEST_MAX_COLUMNS_PER_TABLE, }, proto::Namespace { id: 2, name: "namespace1".to_string(), retention_period_ns: TEST_RETENTION_PERIOD_NS, - service_protection_limits: Some(proto::ServiceProtectionLimits { - max_tables: TEST_MAX_TABLES, - max_columns_per_table: TEST_MAX_COLUMNS_PER_TABLE, - }), + max_tables: TEST_MAX_TABLES, + max_columns_per_table: TEST_MAX_COLUMNS_PER_TABLE, }, ] } diff --git a/router/tests/grpc.rs b/router/tests/grpc.rs index 034928d9f7..fd940bae2d 100644 --- a/router/tests/grpc.rs +++ b/router/tests/grpc.rs @@ -565,22 +565,11 @@ async fn test_update_namespace_negative_retention_period() { #[tokio::test] async fn test_update_namespace_limit_max_tables() { - // Initialise a TestContext requiring explicit namespace creation. - let ctx = TestContextBuilder::default().build().await; - - // Explicitly create the namespace. - let create = ctx - .grpc_delegate() - .namespace_service() - .create_namespace(Request::new(CreateNamespaceRequest { - name: "bananas_test".to_string(), - retention_period_ns: Some(0), - })) - .await - .expect("failed to create namespace") - .into_inner() - .namespace - .expect("no namespace in response"); + // Initialise a TestContext with namespace autocreation. + let ctx = TestContextBuilder::default() + .with_autocreate_namespace(None) + .build() + .await; // Writing to two initial tables should succeed ctx.write_lp("bananas", "test", "ananas,tag1=A,tag2=B val=42i 42424242") @@ -608,13 +597,13 @@ async fn test_update_namespace_limit_max_tables() { .namespace .expect("no namespace in response"); - assert_eq!(got.name, create.name); - assert_eq!(got.id, create.id); - assert_eq!(got.retention_period_ns, create.retention_period_ns); - assert_matches!(&got.service_protection_limits, Some(got_limits) => { - assert_eq!(got_limits.max_tables, 1); - assert_eq!(got_limits.max_columns_per_table, create.service_protection_limits.expect("created namespace should have limits").max_columns_per_table); - }); + assert_eq!(got.name, "bananas_test"); + assert_eq!(got.id, 1); + assert_eq!(got.max_tables, 1); + assert_eq!( + got.max_columns_per_table, + iox_catalog::DEFAULT_MAX_COLUMNS_PER_TABLE + ); // The list namespace RPC should show the updated namespace { @@ -644,9 +633,8 @@ async fn test_update_namespace_limit_max_tables() { assert_eq!(ns.id.get(), got.id); assert_eq!(ns.name, got.name); assert_eq!(ns.retention_period_ns, got.retention_period_ns); - let got_limits = got.service_protection_limits.expect("created namespace should have limits"); - assert_eq!(ns.max_tables, got_limits.max_tables); - assert_eq!(ns.max_columns_per_table, got_limits.max_columns_per_table); + assert_eq!(ns.max_tables, got.max_tables); + assert_eq!(ns.max_columns_per_table, got.max_columns_per_table); assert!(ns.deleted_at.is_none()); }); } @@ -662,29 +650,19 @@ async fn test_update_namespace_limit_max_tables() { .expect_err("cached entry should be removed"); assert_matches!(err, router::server::http::Error::DmlHandler(DmlError::Schema(SchemaError::ServiceLimit(e))) => { let e: CatalogError = *e.downcast::().expect("error returned should be a table create limit error"); - assert_matches!(e,CatalogError::TableCreateLimitError { table_name, .. } => { + assert_matches!(&e, CatalogError::TableCreateLimitError { table_name, .. } => { assert_eq!(table_name, "arán_banana"); + assert_eq!(e.to_string(), "couldn't create table arán_banana; limit reached on namespace 1") }); }); } #[tokio::test] async fn test_update_namespace_limit_max_columns_per_table() { - // Initialise a TestContext requiring explicit namespace creation. - let ctx = TestContextBuilder::default().build().await; - - // Explicitly create the namespace. - let create = ctx - .grpc_delegate() - .namespace_service() - .create_namespace(Request::new(CreateNamespaceRequest { - name: "bananas_test".to_string(), - retention_period_ns: Some(0), - })) - .await - .expect("failed to create namespace") - .into_inner() - .namespace - .expect("no namespace in response"); + // Initialise a TestContext with namespace autocreation. + let ctx = TestContextBuilder::default() + .with_autocreate_namespace(None) + .build() + .await; // Initial write within limit should succeed ctx.write_lp("bananas", "test", "ananas,tag1=A,tag2=B val=42i 42424242") @@ -709,13 +687,10 @@ async fn test_update_namespace_limit_max_columns_per_table() { .namespace .expect("no namespace in response"); - assert_eq!(got.name, create.name); - assert_eq!(got.id, create.id); - assert_eq!(got.retention_period_ns, create.retention_period_ns); - assert_matches!(&got.service_protection_limits, Some(got_limits) => { - assert_eq!(got_limits.max_tables, create.service_protection_limits.expect("namespace should have limits").max_tables); - assert_eq!(got_limits.max_columns_per_table, 1); - }); + assert_eq!(got.name, "bananas_test"); + assert_eq!(got.id, 1); + assert_eq!(got.max_tables, iox_catalog::DEFAULT_MAX_TABLES); + assert_eq!(got.max_columns_per_table, 1); // The list namespace RPC should show the updated namespace { @@ -745,9 +720,8 @@ async fn test_update_namespace_limit_max_columns_per_table() { assert_eq!(ns.id.get(), got.id); assert_eq!(ns.name, got.name); assert_eq!(ns.retention_period_ns, got.retention_period_ns); - let got_limits = got.service_protection_limits.expect("created namespace should have limits"); - assert_eq!(ns.max_tables, got_limits.max_tables); - assert_eq!(ns.max_columns_per_table, got_limits.max_columns_per_table); + assert_eq!(ns.max_tables, got.max_tables); + assert_eq!(ns.max_columns_per_table, got.max_columns_per_table); assert!(ns.deleted_at.is_none()); }); } @@ -859,9 +833,8 @@ async fn test_update_namespace_limit_0_max_tables_max_columns() { assert_eq!(ns.id.get(), create.id); assert_eq!(ns.name, create.name); assert_eq!(ns.retention_period_ns, create.retention_period_ns); - let create_limits = create.service_protection_limits.expect("created namespace should have limits"); - assert_eq!(ns.max_tables, create_limits.max_tables); - assert_eq!(ns.max_columns_per_table, create_limits.max_columns_per_table); + assert_eq!(ns.max_tables, create.max_tables); + assert_eq!(ns.max_columns_per_table, create.max_columns_per_table); assert!(ns.deleted_at.is_none()); }); } diff --git a/service_grpc_namespace/src/lib.rs b/service_grpc_namespace/src/lib.rs index 4d63ec5404..26754f77b2 100644 --- a/service_grpc_namespace/src/lib.rs +++ b/service_grpc_namespace/src/lib.rs @@ -131,7 +131,11 @@ impl namespace_service_server::NamespaceService for NamespaceService { let retention_period_ns = map_retention_period(retention_period_ns)?; - debug!(%namespace_name, ?retention_period_ns, "Updating namespace retention"); + debug!( + %namespace_name, + ?retention_period_ns, + "Updating namespace retention", + ); let namespace = repos .namespaces() @@ -143,7 +147,7 @@ impl namespace_service_server::NamespaceService for NamespaceService { })?; info!( - namespace_name, + %namespace_name, retention_period_ns, namespace_id = %namespace.id, "updated namespace retention" @@ -165,7 +169,11 @@ impl namespace_service_server::NamespaceService for NamespaceService { limit_update, } = request.into_inner(); - debug!(%namespace_name, ?limit_update, "updating namespace service protection limit"); + debug!( + %namespace_name, + ?limit_update, + "updating namespace service protection limit", + ); let namespace = match limit_update { Some(LimitUpdate::MaxTables(n)) => { @@ -175,13 +183,18 @@ impl namespace_service_server::NamespaceService for NamespaceService { )); } repos - .namespaces() - .update_table_limit(&namespace_name, n) - .await - .map_err(|e| { - warn!(error=%e, %namespace_name, table_limit=%n, "failed to update table limit for namespace"); - status_from_catalog_namespace_error(e) - }) + .namespaces() + .update_table_limit(&namespace_name, n) + .await + .map_err(|e| { + warn!( + error = %e, + %namespace_name, + table_limit = %n, + "failed to update table limit for namespace", + ); + status_from_catalog_namespace_error(e) + }) } Some(LimitUpdate::MaxColumnsPerTable(n)) => { if n == 0 { @@ -190,20 +203,31 @@ impl namespace_service_server::NamespaceService for NamespaceService { )); } repos - .namespaces() - .update_column_limit(&namespace_name, n) - .await - .map_err(|e| { - warn!(error=%e, %namespace_name, per_table_column_limit=%n, "failed to update per table column limit for namespace"); - status_from_catalog_namespace_error(e) - }) + .namespaces() + .update_column_limit(&namespace_name, n) + .await + .map_err(|e| { + warn!( + error = %e, + %namespace_name, + per_table_column_limit = %n, + "failed to update per table column limit for namespace", + ); + status_from_catalog_namespace_error(e) + }) } None => Err(Status::invalid_argument( "unsupported service protection limit change requested", )), }?; - info!(namespace_name, namespace_id = %namespace.id, "updated namespace service protection limits"); + info!( + %namespace_name, + namespace_id = %namespace.id, + max_tables = %namespace.max_tables, + max_columns_per_table = %namespace.max_columns_per_table, + "updated namespace service protection limits", + ); Ok(Response::new( UpdateNamespaceServiceProtectionLimitResponse { @@ -218,10 +242,8 @@ fn namespace_to_proto(namespace: CatalogNamespace) -> Namespace { id: namespace.id.get(), name: namespace.name.clone(), retention_period_ns: namespace.retention_period_ns, - service_protection_limits: Some(ServiceProtectionLimits { - max_tables: namespace.max_tables, - max_columns_per_table: namespace.max_columns_per_table, - }), + max_tables: namespace.max_tables, + max_columns_per_table: namespace.max_columns_per_table, } } @@ -231,10 +253,8 @@ fn namespace_to_create_response_proto(namespace: CatalogNamespace) -> CreateName id: namespace.id.get(), name: namespace.name.clone(), retention_period_ns: namespace.retention_period_ns, - service_protection_limits: Some(ServiceProtectionLimits { - max_tables: namespace.max_tables, - max_columns_per_table: namespace.max_columns_per_table, - }), + max_tables: namespace.max_tables, + max_columns_per_table: namespace.max_columns_per_table, }), } } @@ -365,13 +385,6 @@ mod tests { .expect("no namespace in response"); assert_eq!(created_ns.name, NS_NAME); assert_eq!(created_ns.retention_period_ns, Some(RETENTION)); - let ServiceProtectionLimits { - max_tables: original_max_tables, - max_columns_per_table: original_max_columns_per_table, - } = created_ns - .service_protection_limits - .clone() - .expect("created namespace must have service protection limits"); // There should now be one namespace { @@ -416,7 +429,7 @@ mod tests { } // Update the max allowed tables - let want_max_tables = original_max_tables + 42; + let want_max_tables = created_ns.max_tables + 42; let updated_ns = handler .update_namespace_service_protection_limit(Request::new( UpdateNamespaceServiceProtectionLimitRequest { @@ -431,13 +444,14 @@ mod tests { .expect("no namespace in response"); assert_eq!(updated_ns.name, created_ns.name); assert_eq!(updated_ns.id, created_ns.id); - assert_matches!(updated_ns.service_protection_limits, Some(got_limits) => { - assert_eq!(got_limits.max_tables, want_max_tables); - assert_eq!(got_limits.max_columns_per_table, original_max_columns_per_table); - }); + assert_eq!(updated_ns.max_tables, want_max_tables); + assert_eq!( + updated_ns.max_columns_per_table, + created_ns.max_columns_per_table + ); // Update the max allowed columns per table - let want_max_columns_per_table = original_max_columns_per_table + 7; + let want_max_columns_per_table = created_ns.max_columns_per_table + 7; let updated_ns = handler .update_namespace_service_protection_limit(Request::new( UpdateNamespaceServiceProtectionLimitRequest { @@ -452,10 +466,8 @@ mod tests { .expect("no namespace in response"); assert_eq!(updated_ns.name, created_ns.name); assert_eq!(updated_ns.id, created_ns.id); - assert_matches!(updated_ns.service_protection_limits, Some(got_limits) => { - assert_eq!(got_limits.max_tables, want_max_tables); - assert_eq!(got_limits.max_columns_per_table, want_max_columns_per_table); - }); + assert_eq!(updated_ns.max_tables, want_max_tables); + assert_eq!(updated_ns.max_columns_per_table, want_max_columns_per_table); // Deleting the namespace should cause it to disappear handler @@ -511,10 +523,8 @@ mod tests { .expect("no namespace in response"); assert_eq!(created_ns.name, NS_NAME); assert_eq!(created_ns.retention_period_ns, Some(RETENTION)); - assert_matches!(created_ns.service_protection_limits, Some(limits) => { - assert_ne!(limits.max_tables, 0); - assert_ne!(limits.max_columns_per_table, 0); - }); + assert_ne!(created_ns.max_tables, 0); + assert_ne!(created_ns.max_columns_per_table, 0); // The handler should reject any attempt to set the table limit to zero. let status = handler