diff --git a/backend/.sqlx/query-58002bfd04efea3d5f955d0bc5073a916b00fd73600943b98f69639fac2071be.json b/backend/.sqlx/query-58002bfd04efea3d5f955d0bc5073a916b00fd73600943b98f69639fac2071be.json new file mode 100644 index 0000000000..48ec1178d6 --- /dev/null +++ b/backend/.sqlx/query-58002bfd04efea3d5f955d0bc5073a916b00fd73600943b98f69639fac2071be.json @@ -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" +} diff --git a/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json b/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json new file mode 100644 index 0000000000..4c04e5eb0e --- /dev/null +++ b/backend/.sqlx/query-af0bcc6aaf455f75cdf44a503eed708f200a52aaf55c7cf40f7b210f97688c15.json @@ -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" +} diff --git a/backend/windmill-api-workspaces/src/datatable_permissions.rs b/backend/windmill-api-workspaces/src/datatable_permissions.rs index 2c5ccb81b0..1986afc13f 100644 --- a/backend/windmill-api-workspaces/src/datatable_permissions.rs +++ b/backend/windmill-api-workspaces/src/datatable_permissions.rs @@ -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?; diff --git a/backend/windmill-api/src/users.rs b/backend/windmill-api/src/users.rs index 4b935cbce0..ef257146b1 100644 --- a/backend/windmill-api/src/users.rs +++ b/backend/windmill-api/src/users.rs @@ -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?;