From 0d9a8b84e749f8d11d9dee8cefcbbfddd0cfa2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Sun, 29 Sep 2024 08:36:27 +0800 Subject: [PATCH] refactor: `DroppedId` for listing db/tables for gc There is no need to store tables inside `DroppedId::Db`: the tables belonging to a DB for gc can still be stored in `DroppedId::Table`. --- src/meta/api/src/schema_api_impl.rs | 44 ++++++++----------- src/meta/api/src/schema_api_test_suite.rs | 3 -- src/meta/app/src/schema/table.rs | 11 +---- .../interpreter_vacuum_drop_tables.rs | 16 +------ 4 files changed, 21 insertions(+), 53 deletions(-) diff --git a/src/meta/api/src/schema_api_impl.rs b/src/meta/api/src/schema_api_impl.rs index 88304f4432a1..877ffdcb35f2 100644 --- a/src/meta/api/src/schema_api_impl.rs +++ b/src/meta/api/src/schema_api_impl.rs @@ -2678,31 +2678,25 @@ impl + ?Sized> SchemaApi for KV { drop_time_range.clone() }; - let db_filter = (table_drop_time_range, db_info.clone()); - let capacity = the_limit - vacuum_table_infos.len(); - let table_infos = do_get_table_history(self, db_filter, capacity).await?; + let table_infos = + do_get_table_history(self, table_drop_time_range, db_info.clone(), capacity) + .await?; + + for (table_info, db_id) in table_infos.iter() { + vacuum_ids.push(DroppedId::new_table( + *db_id, + table_info.ident.table_id, + table_info.name.clone(), + )); + } // A DB can be removed only when all its tables are removed. if vacuum_db && capacity > table_infos.len() { vacuum_ids.push(DroppedId::Db { db_id: db_info.database_id.db_id, db_name: db_info.name_ident.database_name().to_string(), - tables: table_infos - .iter() - .map(|(table_info, _)| { - (table_info.ident.table_id, table_info.name.clone()) - }) - .collect(), }); - } else { - for (table_info, db_id) in table_infos.iter().take(capacity) { - vacuum_ids.push(DroppedId::new_table( - *db_id, - table_info.ident.table_id, - table_info.name.clone(), - )); - } } vacuum_table_infos.extend( @@ -2742,8 +2736,8 @@ impl + ?Sized> SchemaApi for KV { name_ident: tenant_dbname.clone(), meta: db_meta, }); - let db_filter = (drop_time_range.clone(), db_info); - let table_infos = do_get_table_history(self, db_filter, the_limit).await?; + let table_infos = + do_get_table_history(self, drop_time_range.clone(), db_info, the_limit).await?; let mut drop_ids = vec![]; let mut drop_table_infos = vec![]; @@ -2766,11 +2760,9 @@ impl + ?Sized> SchemaApi for KV { async fn gc_drop_tables(&self, req: GcDroppedTableReq) -> Result<(), KVAppError> { for drop_id in req.drop_ids { match drop_id { - DroppedId::Db { - db_id, - db_name, - tables: _, - } => gc_dropped_db_by_id(self, db_id, &req.tenant, db_name).await?, + DroppedId::Db { db_id, db_name } => { + gc_dropped_db_by_id(self, db_id, &req.tenant, db_name).await? + } DroppedId::Table { name, id } => { gc_dropped_table_by_id(self, &req.tenant, &name, &id).await? } @@ -3532,10 +3524,10 @@ fn build_upsert_table_deduplicated_label(deduplicated_label: String) -> TxnOp { #[fastrace::trace] async fn do_get_table_history( kv_api: &(impl kvapi::KVApi + ?Sized), - db_filter: (Range>>, Arc), + drop_time_range: Range>>, + db_info: Arc, limit: usize, ) -> Result, u64)>, KVAppError> { - let (drop_time_range, db_info) = db_filter; let db_id = db_info.database_id.db_id; let dbid_tbname_idlist = TableIdHistoryIdent { diff --git a/src/meta/api/src/schema_api_test_suite.rs b/src/meta/api/src/schema_api_test_suite.rs index e17b6b752288..e61afa228ad9 100644 --- a/src/meta/api/src/schema_api_test_suite.rs +++ b/src/meta/api/src/schema_api_test_suite.rs @@ -4091,12 +4091,10 @@ impl SchemaApiTestSuite { drop_ids_1.push(DroppedId::Db { db_id: *res.db_id, db_name: db_name.database_name().to_string(), - tables: vec![], }); drop_ids_2.push(DroppedId::Db { db_id: *res.db_id, db_name: db_name.database_name().to_string(), - tables: vec![], }); let req = CreateTableReq { @@ -4136,7 +4134,6 @@ impl SchemaApiTestSuite { drop_ids_2.push(DroppedId::Db { db_id: *db_id, db_name: "db2".to_string(), - tables: vec![], }); info!("--- create and drop db2.tb1"); diff --git a/src/meta/app/src/schema/table.rs b/src/meta/app/src/schema/table.rs index 7542b8177ddd..0b4df049bbe5 100644 --- a/src/meta/app/src/schema/table.rs +++ b/src/meta/app/src/schema/table.rs @@ -984,15 +984,8 @@ impl ListDroppedTableReq { #[derive(Clone, Debug, PartialEq, Eq)] pub enum DroppedId { - Db { - db_id: u64, - db_name: String, - tables: Vec<(u64, String)>, - }, - Table { - name: DBIdTableName, - id: TableId, - }, + Db { db_id: u64, db_name: String }, + Table { name: DBIdTableName, id: TableId }, } impl DroppedId { diff --git a/src/query/service/src/interpreters/interpreter_vacuum_drop_tables.rs b/src/query/service/src/interpreters/interpreter_vacuum_drop_tables.rs index d1b87942fb07..bf6704d5fed4 100644 --- a/src/query/service/src/interpreters/interpreter_vacuum_drop_tables.rs +++ b/src/query/service/src/interpreters/interpreter_vacuum_drop_tables.rs @@ -172,23 +172,9 @@ impl Interpreter for VacuumDropTablesInterpreter { let mut success_dropped_ids = vec![]; for drop_id in drop_ids { match &drop_id { - DroppedId::Db { - db_id, - db_name, - tables, - } => { + DroppedId::Db { db_id: _, db_name } => { if !failed_dbs.contains(db_name) { success_dropped_ids.push(drop_id); - } else { - for (table_id, table_name) in tables.iter() { - if !failed_tables.contains(table_id) { - success_dropped_ids.push(DroppedId::new_table( - *db_id, - *table_id, - table_name.clone(), - )); - } - } } } DroppedId::Table { name: _, id } => {