mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-05 16:03:47 +00:00
fix(datatables): a rename reaches only the workspace it is renaming in
`update_username_in_workpsace` locks one workspace's settings row but wrote `usr` and `usr_to_group` across every workspace holding that name, so it waited on principals another workspace's settings row guards — the deadlock the last commit closed, one workspace over. Both writes are scoped now, and the rename walks memberships in workspace order. The deferred role drop holds the settings row while it runs, so its session carries a statement timeout: the statements run on a database the workspace does not control, and a lock held there would stall every save behind it. `PgRoleInventory` is read by the enterprise planner alone, which a community build compiles without. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S5arH3G2Sa1Qqm32veJQ1n
This commit is contained in:
co-authored by
Claude Opus 5
parent
42271dcf7f
commit
43d2c29988
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE usr SET username = $1 WHERE email = $2 AND workspace_id = $3",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Varchar",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "58002bfd04efea3d5f955d0bc5073a916b00fd73600943b98f69639fac2071be"
|
||||
}
|
||||
+16
@@ -0,0 +1,16 @@
|
||||
{
|
||||
"db_name": "PostgreSQL",
|
||||
"query": "UPDATE usr_to_group SET usr = $1 WHERE usr = $2 AND workspace_id = $3",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Left": [
|
||||
"Varchar",
|
||||
"Text",
|
||||
"Text"
|
||||
]
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15"
|
||||
}
|
||||
@@ -227,6 +227,11 @@ pub(crate) struct AdminConnection {
|
||||
/// The `wm_` logins the cluster holds, split by whether this data table may take
|
||||
/// one over.
|
||||
#[derive(Default)]
|
||||
/// Only the planner reads these, and that is the enterprise module.
|
||||
#[cfg_attr(
|
||||
not(all(feature = "private", feature = "enterprise")),
|
||||
allow(dead_code)
|
||||
)]
|
||||
pub(crate) struct PgRoleInventory {
|
||||
/// Every one of them. `pg_roles` is a cluster catalog, so this spans every
|
||||
/// database and every workspace on the instance — a name in here cannot be
|
||||
@@ -530,6 +535,19 @@ pub(crate) async fn run_planned_drop(
|
||||
.and_then(|d| d.get(datatable_name))
|
||||
.is_some_and(|v| !v.is_null());
|
||||
if !recreated {
|
||||
// The settings row is held for as long as these run, and they run on
|
||||
// a database this workspace does not control — a lock held there, or
|
||||
// a role with a great deal to reassign, would otherwise stall every
|
||||
// save of every data table in the workspace behind it.
|
||||
client
|
||||
.batch_execute("SET statement_timeout = '60s'")
|
||||
.await
|
||||
.map_err(|e| {
|
||||
Error::internal_err(format!(
|
||||
"Failed to bound the role drop: {}",
|
||||
pg_error_message(&e)
|
||||
))
|
||||
})?;
|
||||
run_statements(&mut client, &plan).await?;
|
||||
}
|
||||
tx.commit().await?;
|
||||
|
||||
@@ -216,8 +216,10 @@ async fn rename_user(
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
// Ordered, so a rename and a deletion that touch the same workspaces take
|
||||
// their settings rows in the same sequence rather than head-on.
|
||||
let workspace_usernames = sqlx::query!(
|
||||
"SELECT workspace_id, username FROM usr WHERE email = $1",
|
||||
"SELECT workspace_id, username FROM usr WHERE email = $1 ORDER BY workspace_id",
|
||||
&user_email
|
||||
)
|
||||
.fetch_all(&mut *tx)
|
||||
@@ -891,21 +893,25 @@ async fn update_username_in_workpsace<'c>(
|
||||
|
||||
// ---- instance and workspace users ----
|
||||
|
||||
// Last, after the settings row above: every path that frees or renames a
|
||||
// username takes `workspace_settings` before `usr` and `usr_to_group`, and
|
||||
// one that took them the other way round would deadlock against it.
|
||||
// Last, after the settings row above, and scoped to this workspace like
|
||||
// everything else here: every path that frees or renames a username takes
|
||||
// `workspace_settings` before `usr` and `usr_to_group`, and reaching into
|
||||
// another workspace's rows would hold this one's settings row while waiting
|
||||
// on a principal that workspace's own settings row guards.
|
||||
sqlx::query!(
|
||||
"UPDATE usr SET username = $1 WHERE email = $2",
|
||||
"UPDATE usr SET username = $1 WHERE email = $2 AND workspace_id = $3",
|
||||
new_username,
|
||||
email
|
||||
email,
|
||||
w_id
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
sqlx::query!(
|
||||
"UPDATE usr_to_group SET usr = $1 WHERE usr = $2",
|
||||
"UPDATE usr_to_group SET usr = $1 WHERE usr = $2 AND workspace_id = $3",
|
||||
new_username,
|
||||
old_username
|
||||
old_username,
|
||||
w_id
|
||||
)
|
||||
.execute(&mut **tx)
|
||||
.await?;
|
||||
|
||||
Reference in New Issue
Block a user