Influxdata
/
influxdb
mirror of https://github.com/influxdata/influxdb.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

refactor(namespace): Flatten service protection limits in Namespace proto definition 

This commit also cleans up the code formatting for the gRPC handler and
simplifies some of the gRPC handler tests for the new update service
limit API.
pull/24376/head
Fraser Savage 2023-04-05 14:46:14 +01:00
parent 3ad4cbe7a9
commit b53b8c7d76
No known key found for this signature in database
GPG Key ID: DE47C33CE8C5C446
4 changed files with 95 additions and 122 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
generated_types/protos/influxdata/iox/namespace/v1/service.proto
View File

@ -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;
}

18
ioxd_querier/src/rpc/namespace.rs
View File

@ -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,
},
]
}

85
router/tests/grpc.rs
View File

@ -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::<CatalogError>().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());
});
}

104
service_grpc_namespace/src/lib.rs
View File

@ -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