Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: define DbIdListKey with TIdent, and rename it to DatabaseIdHistoryIdent #15287

Merged
merged 1 commit into from
Apr 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 22 additions & 39 deletions src/meta/api/src/schema_api_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,14 @@ use databend_common_meta_app::schema::CreateVirtualColumnReply;
use databend_common_meta_app::schema::CreateVirtualColumnReq;
use databend_common_meta_app::schema::DBIdTableName;
use databend_common_meta_app::schema::DatabaseId;
use databend_common_meta_app::schema::DatabaseIdHistoryIdent;
use databend_common_meta_app::schema::DatabaseIdToName;
use databend_common_meta_app::schema::DatabaseIdent;
use databend_common_meta_app::schema::DatabaseInfo;
use databend_common_meta_app::schema::DatabaseInfoFilter;
use databend_common_meta_app::schema::DatabaseMeta;
use databend_common_meta_app::schema::DatabaseType;
use databend_common_meta_app::schema::DbIdList;
use databend_common_meta_app::schema::DbIdListKey;
use databend_common_meta_app::schema::DeleteLockRevReq;
use databend_common_meta_app::schema::DropCatalogReply;
use databend_common_meta_app::schema::DropCatalogReq;
Expand Down Expand Up @@ -318,10 +318,8 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
};

// get db id list from _fd_db_id_list/db_id
let dbid_idlist = DbIdListKey {
tenant: name_key.tenant().clone(),
db_name: name_key.database_name().to_string(),
};
let dbid_idlist =
DatabaseIdHistoryIdent::new(name_key.tenant(), name_key.database_name());
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(self, &dbid_idlist).await?;

Expand Down Expand Up @@ -461,7 +459,8 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
}

// get db id list from _fd_db_id_list/<tenant>/<db_name>
let dbid_idlist = DbIdListKey::new(name_key.tenant(), name_key.database_name());
let dbid_idlist =
DatabaseIdHistoryIdent::new(name_key.tenant(), name_key.database_name());
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(self, &dbid_idlist).await?;

Expand Down Expand Up @@ -571,10 +570,8 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
get_pb_value(self, &db_id_key).await?;

// get db id list from _fd_db_id_list/<tenant>/<db_name>
let dbid_idlist = DbIdListKey {
tenant: tenant_dbname.tenant().clone(),
db_name: tenant_dbname.database_name().to_string(),
};
let dbid_idlist =
DatabaseIdHistoryIdent::new(tenant_dbname.tenant(), tenant_dbname.database_name());
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(self, &dbid_idlist).await?;
let mut db_id_list: DbIdList;
Expand Down Expand Up @@ -617,7 +614,8 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
)));
}

let new_dbid_idlist = DbIdListKey::new(tenant_dbname.tenant(), &req.new_db_name);
let new_dbid_idlist =
DatabaseIdHistoryIdent::new(tenant_dbname.tenant(), &req.new_db_name);
let (new_db_id_list_seq, new_db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(self, &new_dbid_idlist).await?;
let mut new_db_id_list: DbIdList;
Expand Down Expand Up @@ -704,11 +702,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
debug!(req :? =(&req); "SchemaApi: {}", func_name!());

// List tables by tenant, db_id, table_name.
let dbid_tbname_idlist = DbIdListKey {
tenant: req.tenant.clone(),
// Using a empty db to to list all
db_name: "".to_string(),
};
let dbid_tbname_idlist = DatabaseIdHistoryIdent::new(&req.tenant, "");
let db_id_list_keys = list_keys(self, &dbid_tbname_idlist).await?;

let mut db_info_list = vec![];
Expand Down Expand Up @@ -765,10 +759,7 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
db_id,
seq: db_meta_seq,
},
name_ident: DatabaseNameIdent::new(
db_id_list_key.tenant(),
&db_id_list_key.db_name,
),
name_ident: DatabaseNameIdent::new_from(db_id_list_key.clone()),
meta: db_meta,
};

Expand Down Expand Up @@ -3418,11 +3409,10 @@ impl<KV: kvapi::KVApi<Error = MetaError> + ?Sized> SchemaApi for KV {
for drop_id in req.drop_ids {
match drop_id {
DroppedId::Db(db_id, db_name) => {
gc_dropped_db_by_id(self, db_id, req.tenant.clone(), db_name).await?
gc_dropped_db_by_id(self, db_id, &req.tenant, db_name).await?
}
DroppedId::Table(db_id, table_id, table_name) => {
gc_dropped_table_by_id(self, req.tenant.clone(), db_id, table_id, table_name)
.await?
gc_dropped_table_by_id(self, &req.tenant, db_id, table_id, table_name).await?
}
}
}
Expand Down Expand Up @@ -4294,10 +4284,8 @@ async fn drop_database_meta(
);

// if remove db, MUST also removed db id from db id list
let dbid_idlist = DbIdListKey {
tenant: tenant_dbname.tenant().clone(),
db_name: tenant_dbname.database_name().to_string(),
};
let dbid_idlist =
DatabaseIdHistoryIdent::new(tenant_dbname.tenant(), tenant_dbname.database_name());
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(kv_api, &dbid_idlist).await?;

Expand Down Expand Up @@ -4342,10 +4330,8 @@ async fn drop_database_meta(
}

// add DbIdListKey if not exists
let dbid_idlist = DbIdListKey {
tenant: tenant_dbname.tenant().clone(),
db_name: tenant_dbname.database_name().to_string(),
};
let dbid_idlist =
DatabaseIdHistoryIdent::new(tenant_dbname.tenant(), tenant_dbname.database_name());
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(kv_api, &dbid_idlist).await?;

Expand Down Expand Up @@ -4924,14 +4910,11 @@ pub(crate) async fn get_index_or_err(
async fn gc_dropped_db_by_id(
kv_api: &(impl kvapi::KVApi<Error = MetaError> + ?Sized),
db_id: u64,
tenant: Tenant,
tenant: &Tenant,
db_name: String,
) -> Result<(), KVAppError> {
// List tables by tenant, db_id, table_name.
let dbid_idlist = DbIdListKey {
tenant: tenant.clone(),
db_name,
};
let dbid_idlist = DatabaseIdHistoryIdent::new(tenant, db_name);
let (db_id_list_seq, db_id_list_opt): (_, Option<DbIdList>) =
get_pb_value(kv_api, &dbid_idlist).await?;

Expand Down Expand Up @@ -4989,7 +4972,7 @@ async fn gc_dropped_db_by_id(

for tb_id in tb_id_list.id_list {
gc_dropped_table_data(kv_api, tb_id, &mut condition, &mut if_then).await?;
gc_dropped_table_index(kv_api, &tenant, tb_id, &mut if_then).await?;
gc_dropped_table_index(kv_api, tenant, tb_id, &mut if_then).await?;
}

let id_key = iter.next().unwrap();
Expand Down Expand Up @@ -5029,7 +5012,7 @@ async fn gc_dropped_db_by_id(

async fn gc_dropped_table_by_id(
kv_api: &(impl kvapi::KVApi<Error = MetaError> + ?Sized),
tenant: Tenant,
tenant: &Tenant,
db_id: u64,
table_id: u64,
table_name: String,
Expand Down Expand Up @@ -5065,7 +5048,7 @@ async fn gc_dropped_table_by_id(
));
}
gc_dropped_table_data(kv_api, table_id, &mut condition, &mut if_then).await?;
gc_dropped_table_index(kv_api, &tenant, table_id, &mut if_then).await?;
gc_dropped_table_index(kv_api, tenant, table_id, &mut if_then).await?;

let txn_req = TxnRequest {
condition,
Expand Down
42 changes: 16 additions & 26 deletions src/meta/api/src/schema_api_test_suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ use databend_common_meta_app::schema::CreateTableReq;
use databend_common_meta_app::schema::CreateVirtualColumnReq;
use databend_common_meta_app::schema::DBIdTableName;
use databend_common_meta_app::schema::DatabaseId;
use databend_common_meta_app::schema::DatabaseIdHistoryIdent;
use databend_common_meta_app::schema::DatabaseIdToName;
use databend_common_meta_app::schema::DatabaseInfo;
use databend_common_meta_app::schema::DatabaseMeta;
use databend_common_meta_app::schema::DatabaseType;
use databend_common_meta_app::schema::DbIdList;
use databend_common_meta_app::schema::DbIdListKey;
use databend_common_meta_app::schema::DeleteLockRevReq;
use databend_common_meta_app::schema::DropCatalogReq;
use databend_common_meta_app::schema::DropDatabaseReq;
Expand Down Expand Up @@ -2134,16 +2134,15 @@ impl SchemaApiTestSuite {
where MT: SchemaApi + kvapi::AsKVApi<Error = MetaError> {
// test drop a db without db_id_list
{
let tenant = "tenant1";
let tenant_name = "tenant1";
let tenant = Tenant::new_literal(tenant_name);

let db = "db1";
let mut util = Util::new(mt, tenant, db, "tb2", "JSON");
let mut util = Util::new(mt, tenant_name, db, "tb2", "JSON");
util.create_db().await?;

// remove db id list
let dbid_idlist = DbIdListKey {
tenant: Tenant::new_or_err(tenant, func_name!())?,
db_name: db.to_string(),
};
let dbid_idlist = DatabaseIdHistoryIdent::new(&tenant, db);
util.mt
.as_kv_api()
.upsert_kv(UpsertKV::delete(dbid_idlist.to_string_key()))
Expand All @@ -2166,24 +2165,24 @@ impl SchemaApiTestSuite {
}
// test get_database_history can return db without db_id_list
{
let tenant = "tenant2";
let tenant2_name = "tenant2";
let tenant2 = Tenant::new_literal(tenant2_name);

let db = "db2";
let mut util = Util::new(mt, tenant, db, "tb2", "JSON");
let mut util = Util::new(mt, tenant2_name, db, "tb2", "JSON");
util.create_db().await?;

// remove db id list
let dbid_idlist = DbIdListKey {
tenant: Tenant::new_or_err(tenant, func_name!())?,
db_name: db.to_string(),
};
let dbid_idlist = DatabaseIdHistoryIdent::new(&tenant2, db);

util.mt
.as_kv_api()
.upsert_kv(UpsertKV::delete(dbid_idlist.to_string_key()))
.await?;

let res = mt
.get_database_history(ListDatabaseReq {
tenant: Tenant::new_or_err(tenant, func_name!())?,
tenant: Tenant::new_or_err(tenant2_name, func_name!())?,
filter: None,
})
.await?;
Expand Down Expand Up @@ -3406,18 +3405,12 @@ impl SchemaApiTestSuite {
let db_name = "db1_database_gc_out_of_retention_time";
let db_name_ident1 = DatabaseNameIdent::new(&tenant, db_name);

let dbid_idlist1 = DbIdListKey {
tenant: Tenant::new_or_err(tenant_name, func_name!())?,
db_name: db_name.to_string(),
};
let dbid_idlist1 = DatabaseIdHistoryIdent::new(&tenant, db_name);

let db_name2 = "db2_database_gc_out_of_retention_time";
let db_name_ident2 = DatabaseNameIdent::new(&tenant, db_name2);

let dbid_idlist2 = DbIdListKey {
tenant: Tenant::new_or_err(tenant_name, func_name!())?,
db_name: db_name2.to_string(),
};
let dbid_idlist2 = DatabaseIdHistoryIdent::new(&tenant, db_name2);

let drop_on = Some(Utc::now() - Duration::days(1));

Expand Down Expand Up @@ -3814,10 +3807,7 @@ impl SchemaApiTestSuite {
let data = serialize_struct(&drop_data)?;
upsert_test_data(mt.as_kv_api(), &id_key, data).await?;

let dbid_idlist1 = DbIdListKey {
tenant: tenant.clone(),
db_name: db1_name.to_string(),
};
let dbid_idlist1 = DatabaseIdHistoryIdent::new(&tenant, db1_name);
let old_id_list: DbIdList = get_kv_data(mt.as_kv_api(), &dbid_idlist1).await?;
assert_eq!(old_id_list.len(), 1);

Expand Down
2 changes: 2 additions & 0 deletions src/meta/app/src/background/job_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ mod kvapi_impl {
pub struct Resource;
impl TenantResource for Resource {
const PREFIX: &'static str = "__fd_background_job";
const TYPE: &'static str = "BackgroundJobIdent";
type ValueType = BackgroundJobId;
}

Expand All @@ -39,6 +40,7 @@ mod kvapi_impl {
}
}

// // Use these error types to replace usage of ErrorCode if possible.
// impl From<ExistError<Resource>> for ErrorCode {
// fn from(err: ExistError<Resource>) -> Self {
// ErrorCode::ConnectionAlreadyExists(err.to_string())
Expand Down
2 changes: 2 additions & 0 deletions src/meta/app/src/background/task_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ mod kvapi_impl {
pub struct Resource;
impl TenantResource for Resource {
const PREFIX: &'static str = "__fd_background_task_by_name";
const TYPE: &'static str = "BackgroundTaskIdent";
type ValueType = BackgroundTaskInfo;
}

Expand All @@ -38,6 +39,7 @@ mod kvapi_impl {
}
}

// // Use these error types to replace usage of ErrorCode if possible.
// impl From<ExistError<Resource>> for ErrorCode {
// impl From<UnknownError<Resource>> for ErrorCode {
}
Expand Down
1 change: 1 addition & 0 deletions src/meta/app/src/principal/role_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ mod kvapi_impl {
}
}

// // Use these error types to replace usage of ErrorCode if possible.
// impl From<ExistError<Resource>> for ErrorCode {
// impl From<UnknownError<Resource>> for ErrorCode {
}
Expand Down
1 change: 1 addition & 0 deletions src/meta/app/src/principal/udf_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ mod kvapi_impl {
}
}

// // Use these error types to replace usage of ErrorCode if possible.
// impl From<ExistError<Resource>> for ErrorCode {
// impl From<UnknownError<Resource>> for ErrorCode {
}
Expand Down
2 changes: 2 additions & 0 deletions src/meta/app/src/schema/catalog_name_ident.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ mod kvapi_impl {
pub struct Resource;
impl TenantResource for Resource {
const PREFIX: &'static str = "__fd_catalog";
const TYPE: &'static str = "CatalogNameIdent";
type ValueType = CatalogId;
}

Expand All @@ -40,6 +41,7 @@ mod kvapi_impl {
}
}

// // Use these error types to replace usage of ErrorCode if possible.
// impl From<ExistError<Resource>> for ErrorCode {
// impl From<UnknownError<Resource>> for ErrorCode {
}
Expand Down
Loading
Loading