feat: disable soft-drop operations for metric logical tables

Prevent soft-dropping, undropping, and purging of metric engine logical tables by explicitly returning unsupported errors. This introduces `is_metric_engine_logical_table` to identify metric logical tables and adds corresponding test cases.

Files:
- `src/common/meta/src/ddl/drop_table/metadata.rs`
- `src/common/meta/src/ddl/purge_dropped_table.rs`
- `src/common/meta/src/ddl/tests/drop_table.rs`
- `src/common/meta/src/ddl/undrop_table.rs`
- `src/common/meta/src/ddl/utils.rs`

Signed-off-by: Lei, HUANG <mrsatangel@gmail.com>
This commit is contained in:
Lei, HUANG
2026-05-03 16:28:47 +08:00
committed by Lei, HUANG
parent 91900ada8b
commit 1df0c58269
5 changed files with 140 additions and 19 deletions

View File

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

View File

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

View File

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

View File

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

View File

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