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.

Sign up for GitHub

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

Jump to bottom

refactor: make IndexNameIdent.tenant a Tenant struct #14776

Merged
merged 2 commits into from
Feb 28, 2024
Merged

refactor: make IndexNameIdent.tenant a Tenant struct #14776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53e9a3b
refactor: make IndexNameIdent.tenant a Tenant struct
drmingdrmer Feb 28, 2024
b3367f4
Merge branch 'main' into 41-tenant
drmingdrmer Feb 28, 2024
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
2 changes: 2 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

41 changes: 22 additions & 19 deletions src/meta/api/src/schema_api_test_suite.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ use databend_common_meta_app::share::ShareGrantObjectPrivilege;
use databend_common_meta_app::share::ShareNameIdent;
use databend_common_meta_app::storage::StorageParams;
use databend_common_meta_app::storage::StorageS3Config;
use databend_common_meta_app::tenant::Tenant;
use databend_common_meta_kvapi::kvapi;
use databend_common_meta_kvapi::kvapi::Key;
use databend_common_meta_kvapi::kvapi::UpsertKVReq;
Expand Down Expand Up @@ -3600,20 +3601,21 @@ impl SchemaApiTestSuite {
self,
mt: &MT,
) -> anyhow::Result<()> {
let tenant = "db_table_gc_out_of_retention_time";
let tenant_name = "db_table_gc_out_of_retention_time";
let tenant = Tenant::new(tenant_name);
let db1_name = "db1";
let tb1_name = "tb1";
let idx1_name = "idx1";
let tbl_name_ident = TableNameIdent {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
db_name: db1_name.to_string(),
table_name: tb1_name.to_string(),
};

let plan = CreateDatabaseReq {
create_option: CreateOption::CreateIfNotExists(false),
name_ident: DatabaseNameIdent {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
db_name: db1_name.to_string(),
},
meta: DatabaseMeta {
Expand Down Expand Up @@ -3688,7 +3690,7 @@ impl SchemaApiTestSuite {
let agg_index_create_req = CreateIndexReq {
create_option: CreateOption::CreateIfNotExists(true),
name_ident: IndexNameIdent {
tenant: tenant.to_string(),
tenant: tenant.clone(),
index_name: idx1_name.to_string(),
},
meta: IndexMeta {
Expand Down Expand Up @@ -3718,7 +3720,7 @@ impl SchemaApiTestSuite {
upsert_test_data(mt.as_kv_api(), &id_key, data).await?;

let dbid_idlist1 = DbIdListKey {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
db_name: db1_name.to_string(),
};
let old_id_list: DbIdList = get_kv_data(mt.as_kv_api(), &dbid_idlist1).await?;
Expand All @@ -3738,7 +3740,7 @@ impl SchemaApiTestSuite {
{
let req = ListDroppedTableReq {
inner: DatabaseNameIdent {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
db_name: "".to_string(),
},
filter: TableInfoFilter::AllDroppedTables(None),
Expand All @@ -3747,7 +3749,7 @@ impl SchemaApiTestSuite {
let resp = mt.get_drop_table_infos(req).await?;

let req = GcDroppedTableReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
drop_ids: resp.drop_ids.clone(),
};
let _resp = mt.gc_drop_tables(req).await?;
Expand Down Expand Up @@ -5727,9 +5729,10 @@ impl SchemaApiTestSuite {
#[minitrace::trace]
async fn index_create_list_drop<MT>(&self, mt: &MT) -> anyhow::Result<()>
where MT: SchemaApi + kvapi::AsKVApi<Error = MetaError> {
let tenant = "tenant1";
let tenant_name = "tenant1";
let tenant = Tenant::new(tenant_name);

let mut util = Util::new(mt, tenant, "db1", "tb1", "eng1");
let mut util = Util::new(mt, tenant_name, "db1", "tb1", "eng1");
let table_id;
let index_id;

Expand Down Expand Up @@ -5767,19 +5770,19 @@ impl SchemaApiTestSuite {
};

let name_ident_1 = IndexNameIdent {
tenant: tenant.to_string(),
tenant: tenant.clone(),
index_name: index_name_1.to_string(),
};

let name_ident_2 = IndexNameIdent {
tenant: tenant.to_string(),
tenant: tenant.clone(),
index_name: index_name_2.to_string(),
};

{
info!("--- list index with no create before");
let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: Some(table_id),
};

Expand Down Expand Up @@ -5837,15 +5840,15 @@ impl SchemaApiTestSuite {
{
info!("--- list index");
let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: None,
};

let res = mt.list_indexes(req).await?;
assert_eq!(2, res.len());

let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: Some(u64::MAX),
};

Expand All @@ -5856,7 +5859,7 @@ impl SchemaApiTestSuite {
{
info!("--- list indexes by table id");
let req = ListIndexesByIdReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id,
};

Expand All @@ -5878,7 +5881,7 @@ impl SchemaApiTestSuite {
{
info!("--- list index after drop one");
let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: Some(table_id),
};

Expand All @@ -5889,7 +5892,7 @@ impl SchemaApiTestSuite {
{
info!("--- check list index content");
let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: Some(table_id),
};

Expand All @@ -5912,7 +5915,7 @@ impl SchemaApiTestSuite {
assert!(res.is_ok());

let req = ListIndexesReq {
tenant: tenant.to_string(),
tenant: tenant_name.to_string(),
table_id: Some(table_id),
};

Expand Down Expand Up @@ -5947,7 +5950,7 @@ impl SchemaApiTestSuite {
info!("--- create or replace index");
let replace_index_name = "replace_idx";
let replace_name_ident = IndexNameIdent {
tenant: tenant.to_string(),
tenant: tenant.clone(),
index_name: replace_index_name.to_string(),
};
let req = CreateIndexReq {
Expand Down
1 change: 1 addition & 0 deletions src/meta/app/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ anyerror = { workspace = true }
chrono = { workspace = true }
chrono-tz = { workspace = true }
cron = "0.12.0"
derive_more = { workspace = true }
enumflags2 = { workspace = true }
hex = "0.4.3"
itertools = { workspace = true }
Expand Down
5 changes: 5 additions & 0 deletions src/meta/app/src/key_with_tenant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ pub trait KeyWithTenant: kvapi::Key {
/// Return the tenant this key belongs to.
fn tenant(&self) -> &Tenant;

/// Return the name of the embedded tenant.
fn tenant_name(&self) -> &str {
self.tenant().name()
}

/// Return a encoded key prefix for listing keys of this kind that belong to the tenant.
///
/// It is in form of `<__PREFIX>/<tenant>/`.
Expand Down
52 changes: 35 additions & 17 deletions src/meta/app/src/schema/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,20 @@ use chrono::Utc;
use databend_common_meta_types::MetaId;

use super::CreateOption;
use crate::tenant::Tenant;
use crate::KeyWithTenant;

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq, Default)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub struct IndexNameIdent {
pub tenant: String,
pub tenant: Tenant,
pub index_name: String,
}

impl IndexNameIdent {
pub fn new(tenant: impl Into<String>, index_name: impl Into<String>) -> IndexNameIdent {
pub fn new(tenant: Tenant, index_name: impl ToString) -> IndexNameIdent {
IndexNameIdent {
tenant: tenant.into(),
index_name: index_name.into(),
tenant,
index_name: index_name.to_string(),
}
}

Expand All @@ -43,7 +45,7 @@ impl IndexNameIdent {

impl Display for IndexNameIdent {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
write!(f, "'{}'.'{}'", self.tenant, self.index_name)
write!(f, "'{}'.'{}'", self.tenant_name(), self.index_name)
}
}

Expand Down Expand Up @@ -131,7 +133,7 @@ impl Default for IndexMeta {
}
}

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct CreateIndexReq {
pub create_option: CreateOption,
pub name_ident: IndexNameIdent,
Expand All @@ -144,13 +146,16 @@ impl Display for CreateIndexReq {
write!(
f,
"create_index(if_not_exists={}):{}={:?}",
if_not_exists, self.name_ident.tenant, self.meta
if_not_exists,
self.name_ident.tenant_name(),
self.meta
)
} else {
write!(
f,
"create_or_replace_index:{}={:?}",
self.name_ident.tenant, self.meta
self.name_ident.tenant_name(),
self.meta
)
}
}
Expand All @@ -161,7 +166,7 @@ pub struct CreateIndexReply {
pub index_id: u64,
}

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct DropIndexReq {
pub if_exists: bool,
pub name_ident: IndexNameIdent,
Expand All @@ -172,15 +177,17 @@ impl Display for DropIndexReq {
write!(
f,
"drop_index(if_exists={}):{}/{}",
self.if_exists, self.name_ident.tenant, self.name_ident.index_name
self.if_exists,
self.name_ident.tenant_name(),
self.name_ident.index_name
)
}
}

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
pub struct DropIndexReply {}

#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct GetIndexReq {
pub name_ident: IndexNameIdent,
}
Expand All @@ -190,7 +197,8 @@ impl Display for GetIndexReq {
write!(
f,
"get_index:{}/{}",
self.name_ident.tenant, self.name_ident.index_name
self.name_ident.tenant_name(),
self.name_ident.index_name
)
}
}
Expand Down Expand Up @@ -250,6 +258,7 @@ mod kvapi_key_impl {
use crate::schema::IndexMeta;
use crate::schema::IndexNameIdent;
use crate::tenant::Tenant;
use crate::KeyWithTenant;

/// <prefix>/<tenant>/<index_name> -> <index_id>
impl kvapi::Key for IndexNameIdent {
Expand All @@ -258,24 +267,33 @@ mod kvapi_key_impl {
type ValueType = IndexId;

fn parent(&self) -> Option<String> {
Some(Tenant::new(&self.tenant).to_string_key())
Some(self.tenant.to_string_key())
}

fn to_string_key(&self) -> String {
kvapi::KeyBuilder::new_prefixed(Self::PREFIX)
.push_str(&self.tenant)
.push_str(self.tenant_name())
.push_str(&self.index_name)
.done()
}

fn from_str_key(s: &str) -> Result<Self, kvapi::KeyError> {
let mut p = kvapi::KeyParser::new_prefixed(s, Self::PREFIX)?;

let tenant = p.next_str()?;
let tenant = p.next_nonempty()?;
let index_name = p.next_str()?;
p.done()?;

Ok(IndexNameIdent { tenant, index_name })
Ok(IndexNameIdent::new(
Tenant::new_nonempty(tenant),
index_name,
))
}
}

impl KeyWithTenant for IndexNameIdent {
fn tenant(&self) -> &Tenant {
&self.tenant
}
}

Expand Down
11 changes: 10 additions & 1 deletion src/meta/app/src/tenant/tenant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,13 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use databend_common_meta_types::NonEmptyString;

/// Tenant is not stored directly in meta-store.
///
/// It is just a type for use on the client side.
#[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[derive(Clone, Debug, PartialEq, Eq, Hash, derive_more::Display)]
#[display(fmt = "Tenant{{{tenant}}}")]
pub struct Tenant {
// TODO: consider using NonEmptyString?
pub tenant: String,
Expand All @@ -28,6 +31,12 @@ impl Tenant {
}
}

pub fn new_nonempty(tenant: NonEmptyString) -> Self {
Self {
tenant: tenant.as_str().to_string(),
}
}

pub fn name(&self) -> &str {
&self.tenant
}
Expand Down
3 changes: 3 additions & 0 deletions src/meta/kvapi/src/kvapi/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ pub enum KeyError {
got: String,
},

#[error("Expect {i}-th segment to be non-empty")]
EmptySegment { i: usize },

#[error("Expect {expect} segments, but: '{got}'")]
WrongNumberOfSegments { expect: usize, got: String },

Expand Down
Loading
Loading