Skip to content

Commit 61232e7

Browse files
authored
refactor(query): optimize visibility checker for large-scale deployments improved 10x (#18954)
* refactor(query): optimize grant object visibility checker for large-scale deployments This commit improves the performance of grant object visibility checking, which is critical for SHOW TABLES/DATABASES operations and privilege verification in large deployments with hundreds of thousands of objects. Key optimizations: - Skip ownership loading for account_admin role (has unrestricted access) - Replace HashMap/HashSet with FastHashMap/FastHashSet for integer keys - Introduce CatalogIdPool to map catalog names to u32 IDs, avoiding repeated string hashing in hot paths - Pre-allocate hash table capacity based on ownership count - Optimize database/table ownership lookups using composite integer keys Benchmark results show significant improvements for large-scale scenarios: - 100K objects: construction time reduced from O(n log n) to O(n) - 1M tables: SHOW TABLES performance improved by ~6x - 100K tables: SHOW TABLES performance improved by ~10x - Multi-catalog scenarios: reduced memory overhead and faster lookups * fix * extract method * use BUILTIN_ROLE_ACCOUNT_ADMIN replace account_admin
1 parent 977fb37 commit 61232e7

File tree

26 files changed

+788
-281
lines changed

26 files changed

+788
-281
lines changed

Cargo.lock

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,7 @@ rmp-serde = "1.1.1"
457457
roaring = { version = "^0.10", features = ["serde"] }
458458
rotbl = { version = "0.2.9", features = [] }
459459
rust_decimal = "1.26"
460+
rustc-hash = "2.1.0"
460461
rustix = { version = "0.38.37", features = ["fs"] }
461462
rustls = { version = "0.23.27", features = ["ring", "tls12"], default-features = false }
462463
rustls-pemfile = "2"

src/bendpy/src/context.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use std::sync::Arc;
1616

1717
use databend_common_exception::Result;
18+
use databend_common_meta_app::principal::BUILTIN_ROLE_ACCOUNT_ADMIN;
1819
use databend_common_version::BUILD_INFO;
1920
use databend_query::sessions::BuildInfoRef;
2021
use databend_query::sessions::QueryContext;
@@ -63,11 +64,17 @@ impl PySessionContext {
6364
databend_common_meta_app::principal::UserPrivilegeSet::available_privileges_on_global(),
6465
);
6566
// Grant account_admin role
66-
user_info.grants.grant_role("account_admin".to_string());
67-
user_info.option.set_default_role(Some("account_admin".to_string()));
67+
user_info
68+
.grants
69+
.grant_role(BUILTIN_ROLE_ACCOUNT_ADMIN.to_string());
70+
user_info
71+
.option
72+
.set_default_role(Some(BUILTIN_ROLE_ACCOUNT_ADMIN.to_string()));
6873
// Set all flags to bypass various checks
6974
user_info.option.set_all_flag();
70-
session.set_authed_user(user_info, Some("account_admin".to_string())).await?;
75+
session
76+
.set_authed_user(user_info, Some(BUILTIN_ROLE_ACCOUNT_ADMIN.to_string()))
77+
.await?;
7178

7279
Ok::<Arc<Session>, Box<dyn std::error::Error + Send + Sync>>(session)
7380
});

src/meta/app/src/principal/mod.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,13 @@ mod user_stage;
3838
mod ownership_object;
3939

4040
mod auto_increment;
41+
42+
/// Built-in role name with full administrative privileges.
43+
pub const BUILTIN_ROLE_ACCOUNT_ADMIN: &str = "account_admin";
44+
45+
/// Built-in role that every user inherits by default.
46+
pub const BUILTIN_ROLE_PUBLIC: &str = "public";
47+
4148
pub mod client_session;
4249
pub mod client_session_ident;
4350
pub mod connection_ident;

src/meta/app/src/principal/user_info.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ use crate::principal::AuthInfo;
2929
use crate::principal::UserGrantSet;
3030
use crate::principal::UserIdentity;
3131
use crate::principal::UserQuota;
32+
use crate::principal::BUILTIN_ROLE_ACCOUNT_ADMIN;
3233

3334
#[derive(Serialize, Deserialize, Clone, Debug, Eq, PartialEq, Default)]
3435
#[serde(default)]
@@ -103,7 +104,9 @@ impl UserInfo {
103104
}
104105

105106
pub fn is_account_admin(&self) -> bool {
106-
self.grants.roles().contains(&"account_admin".to_string())
107+
self.grants
108+
.roles()
109+
.contains(&BUILTIN_ROLE_ACCOUNT_ADMIN.to_string())
107110
}
108111

109112
pub fn has_option_flag(&self, flag: UserOptionFlag) -> bool {

src/meta/proto-conv/tests/it/v136_add_task.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ fn test_decode_v136_add_task() -> anyhow::Result<()> {
4545
when_condition: Some("c1 > 1".to_string()),
4646
after: vec!["task_a".to_string(), "task_b".to_string()],
4747
comment: Some("comment".to_string()),
48-
owner: "public".to_string(),
48+
owner: mt::BUILTIN_ROLE_PUBLIC.to_string(),
4949
owner_user: "me".to_string(),
5050
schedule_options: Some(ScheduleOptions {
5151
interval: Some(11),
@@ -82,7 +82,7 @@ fn test_decode_v136_task_message() -> anyhow::Result<()> {
8282
when_condition: Some("c1 > 1".to_string()),
8383
after: vec!["task_a".to_string(), "task_b".to_string()],
8484
comment: Some("comment".to_string()),
85-
owner: "public".to_string(),
85+
owner: mt::BUILTIN_ROLE_PUBLIC.to_string(),
8686
owner_user: "me".to_string(),
8787
schedule_options: Some(ScheduleOptions {
8888
interval: Some(11),

src/meta/proto-conv/tests/it/v140_task_message.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ fn test_decode_v140_task_message() -> anyhow::Result<()> {
3131
when_condition: Some("c1 > 1".to_string()),
3232
after: vec!["task_a".to_string(), "task_b".to_string()],
3333
comment: Some("comment".to_string()),
34-
owner: "public".to_string(),
34+
owner: mt::BUILTIN_ROLE_PUBLIC.to_string(),
3535
owner_user: "me".to_string(),
3636
schedule_options: Some(ScheduleOptions {
3737
interval: Some(11),

src/query/management/src/role/role_mgr.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use databend_common_meta_app::principal::RoleIdent;
3333
use databend_common_meta_app::principal::RoleInfo;
3434
use databend_common_meta_app::principal::TenantOwnershipObjectIdent;
3535
use databend_common_meta_app::principal::UserPrivilegeType;
36+
use databend_common_meta_app::principal::BUILTIN_ROLE_ACCOUNT_ADMIN;
3637
use databend_common_meta_app::tenant::Tenant;
3738
use databend_common_meta_app::tenant_key::errors::ExistError;
3839
use databend_common_meta_app::KeyWithTenant;
@@ -68,8 +69,6 @@ use crate::serialize_struct;
6869

6970
static TXN_MAX_RETRY_TIMES: u32 = 60;
7071

71-
static BUILTIN_ROLE_ACCOUNT_ADMIN: &str = "account_admin";
72-
7372
#[derive(Clone)]
7473
pub struct RoleMgr {
7574
kv_api: Arc<dyn kvapi::KVApi<Error = MetaError> + Send + Sync>,

src/query/service/src/interpreters/access/privilege_access.rs

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,28 @@ impl PrivilegeAccess {
237237
Ok(())
238238
}
239239

240+
async fn get_role_names_and_ownerships(
241+
&self,
242+
tenant: &Tenant,
243+
) -> Result<(Vec<String>, Vec<SeqV<OwnershipInfo>>)> {
244+
let roles = self.ctx.get_all_effective_roles().await?;
245+
let roles_name = roles
246+
.iter()
247+
.map(|role| role.name.to_string())
248+
.collect::<Vec<_>>();
249+
250+
if roles_name
251+
.iter()
252+
.any(|role_name| role_name == BUILTIN_ROLE_ACCOUNT_ADMIN)
253+
{
254+
return Ok((roles_name, Vec::new()));
255+
}
256+
257+
let user_api = UserApiProvider::instance();
258+
let ownerships = user_api.role_api(tenant).list_ownerships().await?;
259+
Ok((roles_name, ownerships))
260+
}
261+
240262
async fn validate_db_access(
241263
&self,
242264
catalog_name: &str,
@@ -961,13 +983,8 @@ impl AccessChecker for PrivilegeAccess {
961983
return Ok(());
962984
}
963985

964-
let user_api = UserApiProvider::instance();
965-
let ownerships = user_api
966-
.role_api(&tenant)
967-
.list_ownerships()
968-
.await?;
969-
let roles = self.ctx.get_all_effective_roles().await?;
970-
let roles_name: Vec<String> = roles.iter().map(|role| role.name.to_string()).collect();
986+
let (roles_name, ownerships) =
987+
self.get_role_names_and_ownerships(&tenant).await?;
971988
check_db_tb_ownership_access(&identity, catalog, database, show_db_id, &ownerships, &roles_name)?;
972989
}
973990
Some(RewriteKind::ShowStreams(database)) => {
@@ -979,13 +996,8 @@ impl AccessChecker for PrivilegeAccess {
979996
if has_priv(&tenant, database, None, show_db_id, table_id, grant_set, true).await? {
980997
return Ok(());
981998
}
982-
let user_api = UserApiProvider::instance();
983-
let ownerships = user_api
984-
.role_api(&tenant)
985-
.list_ownerships()
986-
.await?;
987-
let roles = self.ctx.get_all_effective_roles().await?;
988-
let roles_name: Vec<String> = roles.iter().map(|role| role.name.to_string()).collect();
999+
let (roles_name, ownerships) =
1000+
self.get_role_names_and_ownerships(&tenant).await?;
9891001
check_db_tb_ownership_access(&identity, &ctl_name, database, show_db_id, &ownerships, &roles_name)?;
9901002
}
9911003
Some(RewriteKind::ShowColumns(catalog_name, database, table)) => {
@@ -1140,13 +1152,8 @@ impl AccessChecker for PrivilegeAccess {
11401152
if has_priv(&tenant, &plan.database, None, show_db_id, None, grant_set, true).await? {
11411153
return Ok(());
11421154
}
1143-
let user_api = UserApiProvider::instance();
1144-
let ownerships = user_api
1145-
.role_api(&tenant)
1146-
.list_ownerships()
1147-
.await?;
1148-
let roles = self.ctx.get_all_effective_roles().await?;
1149-
let roles_name: Vec<String> = roles.iter().map(|role| role.name.to_string()).collect();
1155+
let (roles_name, ownerships) =
1156+
self.get_role_names_and_ownerships(&tenant).await?;
11501157
check_db_tb_ownership_access(&identity, &ctl_name, &plan.database, show_db_id, &ownerships, &roles_name)?;
11511158
}
11521159

@@ -1709,7 +1716,10 @@ fn check_db_tb_ownership_access(
17091716
) -> Result<()> {
17101717
// If contains account_admin even though the current role is not account_admin,
17111718
// It also as a admin user.
1712-
if roles_name.contains(&"account_admin".to_string()) {
1719+
if roles_name
1720+
.iter()
1721+
.any(|role_name| role_name == BUILTIN_ROLE_ACCOUNT_ADMIN)
1722+
{
17131723
return Ok(());
17141724
}
17151725

src/query/service/src/servers/http/v1/roles.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
// limitations under the License.
1414

1515
use databend_common_exception::Result;
16+
use databend_common_users::BUILTIN_ROLE_PUBLIC;
1617
use poem::error::InternalServerError;
1718
use poem::error::Result as PoemResult;
1819
use poem::web::Json;
@@ -22,8 +23,6 @@ use serde::Serialize;
2223

2324
use crate::servers::http::v1::HttpQueryContext;
2425

25-
const PUBLIC_ROLE: &str = "public";
26-
2726
#[derive(Serialize, Deserialize, Debug, Clone)]
2827
pub struct ListRolesResponse {
2928
pub roles: Vec<RoleInfo>,
@@ -46,11 +45,11 @@ async fn handle(ctx: &HttpQueryContext) -> Result<ListRolesResponse> {
4645
let current_role = ctx
4746
.session
4847
.get_current_role()
49-
.map_or(PUBLIC_ROLE.to_string(), |role| role.name);
48+
.map_or(BUILTIN_ROLE_PUBLIC.to_string(), |role| role.name);
5049
let default_role = current_user
5150
.option
5251
.default_role()
53-
.map_or(PUBLIC_ROLE.to_string(), |role| role.to_string());
52+
.map_or(BUILTIN_ROLE_PUBLIC.to_string(), |role| role.to_string());
5453
let mut roles = vec![];
5554
for role in all_roles {
5655
let is_current = role.name == current_role;
@@ -63,7 +62,7 @@ async fn handle(ctx: &HttpQueryContext) -> Result<ListRolesResponse> {
6362
}
6463
if roles.is_empty() {
6564
roles.push(RoleInfo {
66-
name: PUBLIC_ROLE.to_string(),
65+
name: BUILTIN_ROLE_PUBLIC.to_string(),
6766
is_current: true,
6867
is_default: true,
6968
});

0 commit comments

Comments
 (0)