Skip to content

Commit 67548ed

Browse files
camilesingTCeason
andauthored
feat: add explicit FLUSH PRIVILEGES to refresh role cache for query node (#19066)
* feat: add explicit FLUSH PRIVILEGES to refresh role cache for query node * - SystemPlan is only produced by the SQL binder and that binder always runs in a QueryContext whose tenant is fixed to GlobalConfig::instance().query.tenant_id unless the server is in management mode. When management mode is enabled, ManagementModeAccess blocks Plan::System altogether so no user query can ever build a system action with a different tenant. As a result, the tenant field inside SystemPlan was guaranteed to equal the global config tenant and provided no extra routing information anywhere else in the stack. - All flight actions (system_action, kill_query, set_priority, truncate_table) run inside a query server whose GlobalConfig already defines a single tenant. There is no supported deployment where a single query process serves multiple tenants concurrently over flight RPC. Therefore, the create_session helper in src/query/service/src/servers/flight/v1/actions/mod.rs can safely derive the tenant from GlobalConfig every time; the optional parameter was never used to switch context to another tenant. - Because both ends (planner and flight handler) collapse to the same static tenant, the tenant field in SystemPlan and the Option<Tenant> parameter to create_session were redundant wiring that couldn't change behavior. Removing them simplifies the code without affecting any observable semantics and makes it clear that system/flight actions always run under the server's configured tenant. --------- Co-authored-by: TCeason <tai_chong@foxmail.com>
1 parent 2b36a0b commit 67548ed

File tree

9 files changed

+27
-7
lines changed

9 files changed

+27
-7
lines changed

src/query/ast/src/ast/statements/system_action.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ impl Display for SystemStmt {
3232
#[derive(Debug, Clone, PartialEq, Eq, Drive, DriveMut)]
3333
pub enum SystemAction {
3434
Backtrace(bool),
35+
FlushPrivileges,
3536
}
3637

3738
impl Display for SystemAction {
@@ -41,6 +42,7 @@ impl Display for SystemAction {
4142
true => write!(f, "ENABLE EXCEPTION_BACKTRACE"),
4243
false => write!(f, "DISABLE EXCEPTION_BACKTRACE"),
4344
},
45+
SystemAction::FlushPrivileges => write!(f, "FLUSH PRIVILEGES"),
4446
}
4547
}
4648
}

src/query/ast/src/parser/statement.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5054,15 +5054,21 @@ pub fn priority(i: Input) -> IResult<Priority> {
50545054
}
50555055

50565056
pub fn action(i: Input) -> IResult<SystemAction> {
5057-
let mut backtrace = parser_fn(map(
5057+
let backtrace = parser_fn(map(
50585058
rule! {
50595059
#switch ~ EXCEPTION_BACKTRACE
50605060
},
50615061
|(switch, _)| SystemAction::Backtrace(switch),
50625062
));
5063+
let flush_privileges = parser_fn(map(
5064+
rule! {
5065+
FLUSH ~ PRIVILEGES
5066+
},
5067+
|_| SystemAction::FlushPrivileges,
5068+
));
50635069
// add other system action type here
50645070
rule!(
5065-
#backtrace
5071+
#backtrace | #flush_privileges
50665072
)
50675073
.parse(i)
50685074
}

src/query/ast/src/parser/token.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,8 @@ pub enum TokenKind {
683683
FORMAT_NAME,
684684
#[token("FORMATS", ignore(ascii_case))]
685685
FORMATS,
686+
#[token("FLUSH", ignore(ascii_case))]
687+
FLUSH,
686688
#[token("FRAGMENTS", ignore(ascii_case))]
687689
FRAGMENTS,
688690
#[token("FRIDAY", ignore(ascii_case))]

src/query/service/src/interpreters/interpreter_system_action.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use databend_common_exception::set_backtrace;
2020
use databend_common_exception::Result;
2121
use databend_common_sql::plans::SystemAction;
2222
use databend_common_sql::plans::SystemPlan;
23+
use databend_common_users::RoleCacheManager;
2324

2425
use crate::clusters::ClusterHelper;
2526
use crate::clusters::FlightParams;
@@ -90,6 +91,10 @@ impl Interpreter for SystemActionInterpreter {
9091
SystemAction::Backtrace(switch) => {
9192
set_backtrace(switch);
9293
}
94+
SystemAction::FlushPrivileges => {
95+
let tenant = self.ctx.get_tenant();
96+
RoleCacheManager::instance().force_reload(&tenant).await?;
97+
}
9398
}
9499
Ok(PipelineBuildResult::create())
95100
}

src/query/service/src/servers/flight/v1/actions/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ use crate::sessions::SessionManager;
4545

4646
pub(crate) fn create_session() -> Result<Arc<Session>> {
4747
let config = GlobalConfig::instance();
48-
let settings = Settings::create(config.query.tenant_id.clone());
48+
let tenant_id = config.query.tenant_id.clone();
49+
let settings = Settings::create(tenant_id);
4950
match SessionManager::instance().create_with_settings(SessionType::FlightRPC, settings, None) {
5051
Err(cause) => Err(cause),
5152
Ok(session) => Ok(Arc::new(session)),

src/query/sql/src/planner/binder/system.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@ impl Binder {
2929
AstSystemAction::Backtrace(switch) => Ok(Plan::System(Box::new(SystemPlan {
3030
action: SystemAction::Backtrace(*switch),
3131
}))),
32+
AstSystemAction::FlushPrivileges => Ok(Plan::System(Box::new(SystemPlan {
33+
action: SystemAction::FlushPrivileges,
34+
}))),
3235
}
3336
}
3437
}

src/query/sql/src/planner/plans/system.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@ pub struct SystemPlan {
2323
#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)]
2424
pub enum SystemAction {
2525
Backtrace(bool),
26+
FlushPrivileges,
2627
}

tests/nox/java_client/prepare.py

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,10 +57,7 @@ def create_user():
5757
"CREATE USER databend IDENTIFIED BY 'databend' with default_role='account_admin'"
5858
)
5959
exec("GRANT ROLE account_admin TO USER databend")
60-
# need for cluster to sync the GRANT op
61-
time.sleep(16)
62-
for p in [8001, 8002, 8003]:
63-
exec("SHOW GRANTS FOR USER databend", port=p)
60+
exec("SYSTEM FLUSH PRIVILEGES")
6461

6562

6663
def download_testng():

tests/sqllogictests/suites/base/20+_others/20_0017_system_action.test

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,3 +3,6 @@ SYSTEM ENABLE EXCEPTION_BACKTRACE;
33

44
statement ok
55
SYSTEM DISABLE EXCEPTION_BACKTRACE;
6+
7+
statement ok
8+
SYSTEM FLUSH PRIVILEGES;

0 commit comments

Comments
 (0)