diff --git a/src/meta-srv/src/metasrv/builder.rs b/src/meta-srv/src/metasrv/builder.rs index 90ce54142f..8a6f3147bf 100644 --- a/src/meta-srv/src/metasrv/builder.rs +++ b/src/meta-srv/src/metasrv/builder.rs @@ -28,7 +28,9 @@ use common_meta::ddl::table_meta::{TableMetadataAllocator, TableMetadataAllocato use common_meta::ddl::{ DdlContext, NoopRegionFailureDetectorControl, RegionFailureDetectorControllerRef, }; -use common_meta::ddl_manager::{DdlManager, DdlManagerConfiguratorRef}; +use common_meta::ddl_manager::{ + DdlManager, DdlManagerConfiguratorRef, RepartitionProcedureFactoryRef, +}; use common_meta::distributed_time_constants::default_distributed_time_constants; use common_meta::key::TableMetadataManager; use common_meta::key::flow::FlowMetadataManager; @@ -70,7 +72,9 @@ use crate::metasrv::{ use crate::peer::MetasrvPeerAllocator; use crate::procedure::region_migration::DefaultContextFactory; use crate::procedure::region_migration::manager::RegionMigrationManager; -use crate::procedure::repartition::DefaultRepartitionProcedureFactory; +use crate::procedure::repartition::{ + DefaultRepartitionProcedureFactory, GcDisabledRepartitionProcedureFactory, +}; use crate::procedure::wal_prune::Context as WalPruneContext; use crate::procedure::wal_prune::manager::{WalPruneManager, WalPruneTicker}; use crate::region::flush_trigger::RegionFlushTrigger; @@ -428,10 +432,17 @@ impl MetasrvBuilder { soft_drop_enabled: ddl_soft_drop_enabled(&options), }; let procedure_manager_c = procedure_manager.clone(); - let repartition_procedure_factory = Arc::new(DefaultRepartitionProcedureFactory::new( - mailbox.clone(), - options.grpc.server_addr.clone(), - )); + let repartition_procedure_factory: RepartitionProcedureFactoryRef = if options.gc.enable { + Arc::new(DefaultRepartitionProcedureFactory::new( + mailbox.clone(), + options.grpc.server_addr.clone(), + )) + } else { + Arc::new(GcDisabledRepartitionProcedureFactory::new( + mailbox.clone(), + options.grpc.server_addr.clone(), + )) + }; let ddl_manager = DdlManager::try_new( ddl_context, procedure_manager_c, diff --git a/src/meta-srv/src/procedure/repartition.rs b/src/meta-srv/src/procedure/repartition.rs index 53e4bacf1e..2aa8f18799 100644 --- a/src/meta-srv/src/procedure/repartition.rs +++ b/src/meta-srv/src/procedure/repartition.rs @@ -808,6 +808,50 @@ impl DefaultRepartitionProcedureFactory { } } +/// Rejects new repartition requests when metasrv GC is disabled. +/// +/// Procedure loaders are still delegated to the enabled factory so procedures +/// persisted before a metasrv restart can be recovered. +pub struct GcDisabledRepartitionProcedureFactory { + enabled_factory: DefaultRepartitionProcedureFactory, +} + +impl GcDisabledRepartitionProcedureFactory { + pub fn new(mailbox: MailboxRef, server_addr: String) -> Self { + Self { + enabled_factory: DefaultRepartitionProcedureFactory::new(mailbox, server_addr), + } + } +} + +impl RepartitionProcedureFactory for GcDisabledRepartitionProcedureFactory { + fn create( + &self, + _ddl_ctx: &DdlContext, + _table_name: TableName, + _table_id: TableId, + _source: RepartitionSource, + _to_exprs: Vec, + _timeout: Option, + ) -> std::result::Result { + Err(BoxedError::new( + error::InvalidArgumentsSnafu { + err_msg: "Repartition requires metasrv GC to be enabled".to_string(), + } + .build(), + )) + } + + fn register_loaders( + &self, + ddl_ctx: &DdlContext, + procedure_manager: &ProcedureManagerRef, + ) -> std::result::Result<(), BoxedError> { + self.enabled_factory + .register_loaders(ddl_ctx, procedure_manager) + } +} + impl RepartitionProcedureFactory for DefaultRepartitionProcedureFactory { fn create( &self, @@ -919,7 +963,7 @@ mod tests { use std::sync::Arc; use std::sync::atomic::{AtomicBool, Ordering}; - use common_error::ext::BoxedError; + use common_error::ext::{BoxedError, ErrorExt}; use common_error::mock::MockError; use common_error::status_code::StatusCode; use common_meta::ddl::test_util::datanode_handler::{ @@ -1050,6 +1094,37 @@ mod tests { ) } + #[test] + fn test_gc_disabled_factory_rejects_repartition() { + let env = TestingEnv::new(); + let node_manager = Arc::new(MockDatanodeManager::new(UnexpectedErrorDatanodeHandler)); + let ddl_ctx = env.ddl_context(node_manager); + let factory = GcDisabledRepartitionProcedureFactory::new( + env.mailbox_ctx.mailbox().clone(), + env.server_addr.clone(), + ); + + let err = factory + .create( + &ddl_ctx, + TableName::new("test_catalog", "test_schema", "test_table"), + 1024, + RepartitionSource::Unpartitioned { + partition_columns: vec![], + }, + vec![], + None, + ) + .err() + .expect("GC-disabled factory must reject repartition"); + + assert_eq!(StatusCode::InvalidArguments, err.status_code()); + assert_eq!( + "Invalid arguments: Repartition requires metasrv GC to be enabled", + err.to_string() + ); + } + #[test] fn test_filter_allocated_region_routes() { let table_id = 1024; diff --git a/tests-integration/tests/repartition.rs b/tests-integration/tests/repartition.rs index 32f80a32cb..cc55cb1bfd 100644 --- a/tests-integration/tests/repartition.rs +++ b/tests-integration/tests/repartition.rs @@ -345,6 +345,17 @@ pub async fn test_partition_unpartitioned_mito(store_type: StorageType) { .with_datanodes(datanodes as u32) .with_store_config(store_config) .with_datanode_wal_config(DatanodeWalConfig::Noop) + .with_metasrv_gc_config(GcSchedulerOptions { + enable: true, + gc_cooldown_period: Duration::from_nanos(1), + ..Default::default() + }) + .with_datanode_gc_config(GcConfig { + enable: true, + lingering_time: Some(Duration::from_secs(0)), + unknown_file_lingering_time: Duration::from_secs(0), + ..Default::default() + }) .build(true) .await; @@ -472,6 +483,17 @@ pub async fn test_repartition_on_columns_metadata_mito(store_type: StorageType) .with_datanodes(datanodes as u32) .with_store_config(store_config) .with_datanode_wal_config(DatanodeWalConfig::Noop) + .with_metasrv_gc_config(GcSchedulerOptions { + enable: true, + gc_cooldown_period: Duration::from_nanos(1), + ..Default::default() + }) + .with_datanode_gc_config(GcConfig { + enable: true, + lingering_time: Some(Duration::from_secs(0)), + unknown_file_lingering_time: Duration::from_secs(0), + ..Default::default() + }) .build(true) .await; @@ -557,6 +579,17 @@ pub async fn test_repartition_on_columns_data_correctness_mito(store_type: Stora .with_datanodes(datanodes as u32) .with_store_config(store_config) .with_datanode_wal_config(DatanodeWalConfig::Noop) + .with_metasrv_gc_config(GcSchedulerOptions { + enable: true, + gc_cooldown_period: Duration::from_nanos(1), + ..Default::default() + }) + .with_datanode_gc_config(GcConfig { + enable: true, + lingering_time: Some(Duration::from_secs(0)), + unknown_file_lingering_time: Duration::from_secs(0), + ..Default::default() + }) .build(true) .await; @@ -612,6 +645,17 @@ pub async fn test_partition_unpartitioned_metric(store_type: StorageType) { .with_datanodes(datanodes as u32) .with_store_config(store_config) .with_datanode_wal_config(DatanodeWalConfig::Noop) + .with_metasrv_gc_config(GcSchedulerOptions { + enable: true, + gc_cooldown_period: Duration::from_nanos(1), + ..Default::default() + }) + .with_datanode_gc_config(GcConfig { + enable: true, + lingering_time: Some(Duration::from_secs(0)), + unknown_file_lingering_time: Duration::from_secs(0), + ..Default::default() + }) .build(true) .await; diff --git a/tests/cases/distributed/repartition/repartition.result b/tests/cases/distributed/repartition/repartition.result index 72212699af..7b25a13a06 100644 --- a/tests/cases/distributed/repartition/repartition.result +++ b/tests/cases/distributed/repartition/repartition.result @@ -12,6 +12,7 @@ CREATE TABLE alter_repartition_table( Affected Rows: 0 +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE alter_repartition_table REPARTITION ( device_id < 100 ) INTO ( @@ -19,7 +20,7 @@ ALTER TABLE alter_repartition_table REPARTITION ( device_id < 100 AND area >= 'South' ); -Affected Rows: 0 +Error: 1004(InvalidArguments), Invalid arguments: Repartition requires metasrv GC to be enabled SHOW CREATE TABLE alter_repartition_table; @@ -35,21 +36,21 @@ SHOW CREATE TABLE alter_repartition_table; | | PRIMARY KEY ("device_id") | | | ) | | | PARTITION ON COLUMNS ("device_id", "area") ( | -| | device_id < 100 AND area < 'South', | +| | device_id < 100, | | | device_id >= 100 AND device_id < 200, | -| | device_id >= 200, | -| | device_id < 100 AND area >= 'South' | +| | device_id >= 200 | | | ) | | | ENGINE=mito | | | | +-------------------------+--------------------------------------------------------+ +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE alter_repartition_table MERGE PARTITION ( - device_id < 100 AND area < 'South', - device_id < 100 AND area >= 'South' + device_id < 100, + device_id >= 100 AND device_id < 200, ); -Affected Rows: 0 +Error: 1004(InvalidArguments), Invalid arguments: Repartition requires metasrv GC to be enabled SHOW CREATE TABLE alter_repartition_table; @@ -124,11 +125,7 @@ ALTER TABLE alter_repartition_table_with_options REPARTITION ( WAIT = false ); -+--------------------------------------+ -| Procedure ID | -+--------------------------------------+ -| PROC_ID | -+--------------------------------------+ +Error: 1004(InvalidArguments), Invalid arguments: Repartition requires metasrv GC to be enabled DROP TABLE alter_repartition_table_with_options; @@ -177,7 +174,7 @@ WITH ( Affected Rows: 0 --- Split physical table partition +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE metric_physical_table SPLIT PARTITION ( host < 'h1' ) INTO ( @@ -185,7 +182,7 @@ ALTER TABLE metric_physical_table SPLIT PARTITION ( host >= 'h0' AND host < 'h1' ); -Affected Rows: 0 +Error: 1004(InvalidArguments), Invalid arguments: Repartition requires metasrv GC to be enabled SHOW CREATE TABLE metric_physical_table; @@ -200,10 +197,9 @@ SHOW CREATE TABLE metric_physical_table; | | PRIMARY KEY ("host") | | | ) | | | PARTITION ON COLUMNS ("host") ( | -| | host < 'h0', | +| | host < 'h1', | | | host >= 'h1' AND host < 'h2', | -| | host >= 'h2', | -| | host >= 'h0' AND host < 'h1' | +| | host >= 'h2' | | | ) | | | ENGINE=metric | | | WITH( | @@ -227,13 +223,13 @@ SELECT * FROM logical_table_v2; ++ ++ --- Merge physical table partition +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE metric_physical_table MERGE PARTITION ( - host < 'h0', - host >= 'h0' AND host < 'h1' + host < 'h1', + host >= 'h1' AND host < 'h2' ); -Affected Rows: 0 +Error: 1004(InvalidArguments), Invalid arguments: Repartition requires metasrv GC to be enabled SHOW CREATE TABLE metric_physical_table; diff --git a/tests/cases/distributed/repartition/repartition.sql b/tests/cases/distributed/repartition/repartition.sql index 92b1d6ad97..180969f96f 100644 --- a/tests/cases/distributed/repartition/repartition.sql +++ b/tests/cases/distributed/repartition/repartition.sql @@ -10,6 +10,7 @@ CREATE TABLE alter_repartition_table( device_id >= 200 ); +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE alter_repartition_table REPARTITION ( device_id < 100 ) INTO ( @@ -19,9 +20,10 @@ ALTER TABLE alter_repartition_table REPARTITION ( SHOW CREATE TABLE alter_repartition_table; +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE alter_repartition_table MERGE PARTITION ( - device_id < 100 AND area < 'South', - device_id < 100 AND area >= 'South' + device_id < 100, + device_id >= 100 AND device_id < 200, ); SHOW CREATE TABLE alter_repartition_table; @@ -109,7 +111,7 @@ WITH ( on_physical_table = "metric_physical_table" ); --- Split physical table partition +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE metric_physical_table SPLIT PARTITION ( host < 'h1' ) INTO ( @@ -126,10 +128,10 @@ SELECT * FROM logical_table_v1; SELECT * FROM logical_table_v2; --- Merge physical table partition +-- Repartition requires metasrv GC. Verify split requests are rejected when it is disabled. ALTER TABLE metric_physical_table MERGE PARTITION ( - host < 'h0', - host >= 'h0' AND host < 'h1' + host < 'h1', + host >= 'h1' AND host < 'h2' ); SHOW CREATE TABLE metric_physical_table;