diff --git a/src/common/meta/src/ddl/drop_table/metadata.rs b/src/common/meta/src/ddl/drop_table/metadata.rs index c1a8a90d4e..6289b6c61a 100644 --- a/src/common/meta/src/ddl/drop_table/metadata.rs +++ b/src/common/meta/src/ddl/drop_table/metadata.rs @@ -13,12 +13,13 @@ // limitations under the License. use common_catalog::format_full_table_name; -use snafu::OptionExt; +use snafu::{OptionExt, ensure}; use store_api::metric_engine_consts::METRIC_ENGINE_NAME; use crate::ddl::drop_table::DropTableProcedure; -use crate::ddl::utils::extract_region_wal_options; +use crate::ddl::utils::{extract_region_wal_options, is_metric_engine_logical_table}; use crate::error::{self, Result}; +use crate::key::table_route::TableRouteValue; impl DropTableProcedure { /// Fetches the table info and physical table route. @@ -31,18 +32,34 @@ impl DropTableProcedure { .get_physical_table_route(task.table_id) .await?; - if physical_table_id == self.data.table_id() { - let table_info_value = self - .context - .table_metadata_manager - .table_info_manager() - .get(task.table_id) - .await? - .with_context(|| error::TableInfoNotFoundSnafu { - table: format_full_table_name(&task.catalog, &task.schema, &task.table), - })? - .into_inner(); + let table_info_value = self + .context + .table_metadata_manager + .table_info_manager() + .get(task.table_id) + .await? + .with_context(|| error::TableInfoNotFoundSnafu { + table: format_full_table_name(&task.catalog, &task.schema, &task.table), + })? + .into_inner(); + let table_route_value = TableRouteValue::new( + self.data.table_id(), + physical_table_id, + physical_table_route_value.region_routes.clone(), + ); + // TODO(hl): support soft-dropping logical tables. + ensure!( + !(self.context.soft_drop_enabled + && is_metric_engine_logical_table( + &table_info_value.table_info, + &table_route_value + )), + error::UnsupportedSnafu { + operation: "soft-dropping metric logical tables".to_string() + } + ); + if physical_table_id == self.data.table_id() { let engine = table_info_value.table_info.meta.engine; // rollback only if dropping the metric physical table fails self.data.allow_rollback = engine.as_str() == METRIC_ENGINE_NAME; diff --git a/src/common/meta/src/ddl/purge_dropped_table.rs b/src/common/meta/src/ddl/purge_dropped_table.rs index be73b02b04..4c35ecf2eb 100644 --- a/src/common/meta/src/ddl/purge_dropped_table.rs +++ b/src/common/meta/src/ddl/purge_dropped_table.rs @@ -21,7 +21,7 @@ use common_procedure::{ }; use common_wal::options::WalOptions; use serde::{Deserialize, Serialize}; -use snafu::ResultExt; +use snafu::{ResultExt, ensure}; use store_api::storage::{RegionNumber, TableId}; use strum::AsRefStr; use table::metadata::TableInfo; @@ -30,8 +30,8 @@ use table::table_name::TableName; use crate::ddl::DdlContext; use crate::ddl::drop_table::executor::DropTableExecutor; use crate::ddl::undrop_table::open_regions; -use crate::ddl::utils::map_to_procedure_error; -use crate::error::Result; +use crate::ddl::utils::{is_metric_engine_logical_table, map_to_procedure_error}; +use crate::error::{self, Result}; use crate::key::table_route::TableRouteValue; use crate::lock_key::{CatalogLock, SchemaLock, TableLock, TableNameLock}; use crate::rpc::ddl::PurgeDroppedTableTask; @@ -73,6 +73,15 @@ impl PurgeDroppedTableProcedure { let Some(dropped_table) = dropped_table else { return Ok(Status::done()); }; + ensure!( + !is_metric_engine_logical_table( + &dropped_table.table_info_value.table_info, + &dropped_table.table_route_value + ), + error::UnsupportedSnafu { + operation: "purging metric logical tables".to_string() + } + ); self.data.table_id = Some(dropped_table.table_id); self.data.table_name = Some(dropped_table.table_name); self.data.table_info = Some(dropped_table.table_info_value.table_info); diff --git a/src/common/meta/src/ddl/tests/drop_table.rs b/src/common/meta/src/ddl/tests/drop_table.rs index f4bcb29ef9..f9d72214dd 100644 --- a/src/common/meta/src/ddl/tests/drop_table.rs +++ b/src/common/meta/src/ddl/tests/drop_table.rs @@ -634,6 +634,59 @@ async fn test_undrop_logical_table_skips_datanode_open() { assert!(rx.try_recv().is_err()); } +#[tokio::test] +async fn test_soft_drop_metric_logical_table_fails() { + let node_manager = Arc::new(MockDatanodeManager::new(NaiveDatanodeHandler)); + let mut ddl_context = new_ddl_context(node_manager); + ddl_context.soft_drop_enabled = true; + let physical_table_id = create_physical_table(&ddl_context, "phy").await; + let logical_table_id = + create_logical_table(ddl_context.clone(), physical_table_id, "foo").await; + + let mut procedure = DropTableProcedure::new( + new_drop_table_task("foo", logical_table_id, false), + ddl_context, + ); + let err = procedure.on_prepare().await.unwrap_err(); + + assert_eq!(err.status_code(), StatusCode::Unsupported); +} + +#[tokio::test] +async fn test_undrop_metric_logical_table_fails() { + let node_manager = Arc::new(MockDatanodeManager::new(NaiveDatanodeHandler)); + let ddl_context = new_ddl_context(node_manager); + let physical_table_id = create_physical_table(&ddl_context, "phy").await; + let logical_table_id = + create_metric_logical_table_tombstone(&ddl_context, physical_table_id, "foo").await; + + let mut procedure = + UndropTableProcedure::new(new_undrop_table_task("foo", logical_table_id), ddl_context); + let err = procedure.on_prepare().await.unwrap_err(); + + assert_eq!(err.status_code(), StatusCode::Unsupported); +} + +#[tokio::test] +async fn test_purge_metric_logical_table_fails() { + let node_manager = Arc::new(MockDatanodeManager::new(NaiveDatanodeHandler)); + let ddl_context = new_ddl_context(node_manager); + let physical_table_id = create_physical_table(&ddl_context, "phy").await; + let logical_table_id = + create_metric_logical_table_tombstone(&ddl_context, physical_table_id, "foo").await; + + let mut procedure = PurgeDroppedTableProcedure::new( + new_purge_dropped_table_task("foo", Some(logical_table_id)), + ddl_context, + ); + let err = procedure + .execute(&new_test_procedure_context()) + .await + .unwrap_err(); + + assert_eq!(err.status_code(), StatusCode::Unsupported); +} + #[tokio::test] async fn test_undrop_table_fails_when_live_name_exists() { let node_manager = Arc::new(MockDatanodeManager::new(NaiveDatanodeHandler)); @@ -901,6 +954,28 @@ fn new_purge_dropped_table_task( } } +async fn create_metric_logical_table_tombstone( + ddl_context: &crate::ddl::DdlContext, + physical_table_id: TableId, + table_name: &str, +) -> TableId { + let logical_table_id = + create_logical_table(ddl_context.clone(), physical_table_id, table_name).await; + let mut task = test_create_logical_table_task(table_name); + task.set_table_id(logical_table_id); + ddl_context + .table_metadata_manager + .delete_table_metadata( + logical_table_id, + &task.table_name(), + &TableRouteValue::logical(physical_table_id), + &HashMap::new(), + ) + .await + .unwrap(); + logical_table_id +} + #[tokio::test] async fn test_memory_region_keeper_guard_dropped_on_procedure_done() { let node_manager = Arc::new(MockDatanodeManager::new(NaiveDatanodeHandler)); diff --git a/src/common/meta/src/ddl/undrop_table.rs b/src/common/meta/src/ddl/undrop_table.rs index 9d0ea734d7..0d9e8fa015 100644 --- a/src/common/meta/src/ddl/undrop_table.rs +++ b/src/common/meta/src/ddl/undrop_table.rs @@ -33,7 +33,10 @@ use table::metadata::TableId; use table::table_name::TableName; use table::table_reference::TableReference; -use crate::ddl::utils::{add_peer_context_if_needed, map_to_procedure_error, region_storage_path}; +use crate::ddl::utils::{ + add_peer_context_if_needed, is_metric_engine_logical_table, map_to_procedure_error, + region_storage_path, +}; use crate::ddl::{CreateRequestBuilder, DdlContext, build_template_from_raw_table_info}; use crate::error::{self, Result}; use crate::instruction::CacheIdent; @@ -92,6 +95,15 @@ impl UndropTableProcedure { self.data.table_name = Some(dropped_table.table_name.clone()); self.data.table_route_value = Some(dropped_table.table_route_value.clone()); self.data.region_wal_options = dropped_table.region_wal_options; + ensure!( + !is_metric_engine_logical_table( + &dropped_table.table_info_value.table_info, + self.data.table_route_value() + ), + error::UnsupportedSnafu { + operation: "undropping metric logical tables".to_string() + } + ); self.data.table_info = Some(dropped_table.table_info_value.table_info); self.data.state = UndropTableState::RestoreMetadata; Ok(Status::executing(true)) diff --git a/src/common/meta/src/ddl/utils.rs b/src/common/meta/src/ddl/utils.rs index e90f224c99..f5d18fbfad 100644 --- a/src/common/meta/src/ddl/utils.rs +++ b/src/common/meta/src/ddl/utils.rs @@ -37,8 +37,8 @@ use snafu::{OptionExt, ResultExt, ensure}; use store_api::metadata::ColumnMetadata; use store_api::metric_engine_consts::{LOGICAL_TABLE_METADATA_KEY, MANIFEST_INFO_EXTENSION_KEY}; use store_api::region_engine::RegionManifestInfo; -use store_api::storage::RegionId; -use table::metadata::TableId; +use store_api::storage::{RegionId, RegionNumber}; +use table::metadata::{TableId, TableInfo}; use table::table_reference::TableReference; use crate::ddl::{DdlContext, DetectingRegion}; @@ -68,6 +68,14 @@ pub fn add_peer_context_if_needed(datanode: Peer) -> impl FnOnce(Error) -> Error } } +pub(crate) fn is_metric_engine_logical_table( + table_info: &TableInfo, + table_route_value: &TableRouteValue, +) -> bool { + table_info.meta.engine == METRIC_ENGINE + && matches!(table_route_value, TableRouteValue::Logical(_)) +} + /// Maps the error to the corresponding procedure error. /// /// This function determines whether the error should be retried and if poison cleanup is needed,