Skip to content

Commit

Permalink
refactor: DroppedId for listing db/tables for gc
Browse files Browse the repository at this point in the history
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`.
  • Loading branch information
drmingdrmer committed Sep 29, 2024
1 parent 4129816 commit 0d9a8b8
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 53 deletions.
44 changes: 18 additions & 26 deletions src/meta/api/src/schema_api_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2678,31 +2678,25 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?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(
Expand Down Expand Up @@ -2742,8 +2736,8 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?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![];

Expand All @@ -2766,11 +2760,9 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?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?
}
Expand Down Expand Up @@ -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<Error = MetaError> + ?Sized),
db_filter: (Range<Option<DateTime<Utc>>>, Arc<DatabaseInfo>),
drop_time_range: Range<Option<DateTime<Utc>>>,
db_info: Arc<DatabaseInfo>,
limit: usize,
) -> Result<Vec<(Arc<TableInfo>, u64)>, KVAppError> {
let (drop_time_range, db_info) = db_filter;
let db_id = db_info.database_id.db_id;

let dbid_tbname_idlist = TableIdHistoryIdent {
Expand Down
3 changes: 0 additions & 3 deletions src/meta/api/src/schema_api_test_suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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");
Expand Down
11 changes: 2 additions & 9 deletions src/meta/app/src/schema/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand Down

0 comments on commit 0d9a8b8

Please sign in to comment.