mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-10-22 08:03:38 +00:00
fix: keep schedule push, flow advance, wac approval and scim on one pool connection (#11904)
* fix: keep schedule push, flow advance and wac approval on one pool connection Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: read flow args on the status-update tx instead of preloading them Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: pin ee ref for the scim update_group fix Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * chore: update ee-repo-ref to c465c042eeae3c5d9a0180f62a56e8f8d3e9e162 This commit updates the EE repository reference after PR #905 was merged in windmill-ee-private. Previous ee-repo-ref: dc4d0327044ddc10ef53ad0b21ae37b96cf4aad2 New ee-repo-ref: c465c042eeae3c5d9a0180f62a56e8f8d3e9e162 Automated by sync-ee-ref workflow. --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
windmill-internal-app[bot]
parent
2c559b452f
commit
8afe0607ab
@@ -42,7 +42,6 @@ use windmill_common::{
|
||||
/// Helper to fetch metadata for a schedule's script or flow
|
||||
async fn get_schedule_metadata<'c>(
|
||||
tx: &mut sqlx::Transaction<'c, sqlx::Postgres>,
|
||||
db: &DB,
|
||||
schedule: &Schedule,
|
||||
) -> Result<(
|
||||
Option<String>, // tag
|
||||
@@ -78,7 +77,12 @@ async fn get_schedule_metadata<'c>(
|
||||
Ok((
|
||||
flow_info.tag.clone(),
|
||||
None,
|
||||
flow_info.on_behalf_of(&schedule.workspace_id, db).await?,
|
||||
windmill_common::on_behalf_of_from_permissioned_as(
|
||||
flow_info.on_behalf_of.as_deref(),
|
||||
&schedule.workspace_id,
|
||||
&mut **tx,
|
||||
)
|
||||
.await?,
|
||||
None,
|
||||
Some(version),
|
||||
parsed_retry,
|
||||
@@ -106,7 +110,6 @@ async fn get_schedule_metadata<'c>(
|
||||
_job_token_scopes,
|
||||
) = windmill_common::get_latest_hash_for_path(
|
||||
&mut **tx,
|
||||
db,
|
||||
&schedule.workspace_id,
|
||||
&schedule.script_path,
|
||||
false,
|
||||
@@ -117,6 +120,10 @@ async fn get_schedule_metadata<'c>(
|
||||
}
|
||||
}
|
||||
|
||||
/// Read identity and runnable settings through `tx` rather than `db`: every caller holds `tx`
|
||||
/// open on the same pool, so a second checkout per tick starves it when a burst of schedules
|
||||
/// fires at once. `tx` may run as `windmill_user` (schedule and flow API); those tables carry
|
||||
/// no RLS, so they resolve the same as on the pool.
|
||||
pub async fn push_scheduled_job<'c>(
|
||||
db: &DB,
|
||||
mut tx: Transaction<'c, Postgres>,
|
||||
@@ -319,7 +326,7 @@ pub async fn push_scheduled_job<'c>(
|
||||
|
||||
// Get metadata from the scheduled script/flow for tag, timeout, etc.
|
||||
let (tag, timeout, on_behalf_of, hash, flow_version, retry) =
|
||||
get_schedule_metadata(&mut tx, db, schedule).await?;
|
||||
get_schedule_metadata(&mut tx, schedule).await?;
|
||||
|
||||
(
|
||||
JobPayload::SingleStepFlow {
|
||||
@@ -373,7 +380,12 @@ pub async fn push_scheduled_job<'c>(
|
||||
)
|
||||
.warn_after_seconds_with_sql(1, "get_flow_version_info_from_version".to_string())
|
||||
.await?;
|
||||
let on_behalf_of = flow_info.on_behalf_of(&schedule.workspace_id, db).await?;
|
||||
let on_behalf_of = windmill_common::on_behalf_of_from_permissioned_as(
|
||||
flow_info.on_behalf_of.as_deref(),
|
||||
&schedule.workspace_id,
|
||||
&mut *tx,
|
||||
)
|
||||
.await?;
|
||||
let FlowVersionInfo { version, tag, dedicated_worker, labels, job_token_scopes, .. } =
|
||||
flow_info;
|
||||
|
||||
@@ -453,7 +465,6 @@ pub async fn push_scheduled_job<'c>(
|
||||
job_token_scopes,
|
||||
) = windmill_common::get_latest_hash_for_path(
|
||||
&mut *tx,
|
||||
db,
|
||||
&schedule.workspace_id,
|
||||
&schedule.script_path,
|
||||
false,
|
||||
@@ -461,17 +472,11 @@ pub async fn push_scheduled_job<'c>(
|
||||
.warn_after_seconds_with_sql(1, "get_latest_hash_for_path".to_string())
|
||||
.await?;
|
||||
|
||||
// NB: read on the non-RLS pool (`db`), not `tx`. push_scheduled_job is
|
||||
// also invoked with an RLS user_db transaction (api-schedule/api-flows),
|
||||
// under which these lookups would resolve against the caller's row
|
||||
// visibility rather than the full table. The dual-connection here is
|
||||
// intentional and required for correctness.
|
||||
let rs =
|
||||
windmill_common::runnable_settings::from_handle(runnable_settings_handle, &mut *tx)
|
||||
.await?;
|
||||
let (debouncing_settings, concurrency_settings) =
|
||||
windmill_common::runnable_settings::prefetch_cached_from_handle(
|
||||
runnable_settings_handle,
|
||||
db,
|
||||
)
|
||||
.await?;
|
||||
windmill_common::runnable_settings::prefetch_cached_tx(&rs, &mut tx).await?;
|
||||
|
||||
if schedule.retry.is_some() {
|
||||
let parsed_retry = serde_json::from_value::<Retry>(schedule.retry.clone().unwrap())
|
||||
@@ -598,7 +603,7 @@ pub async fn push_scheduled_job<'c>(
|
||||
let resolved_email = windmill_common::users::get_email_from_permissioned_as(
|
||||
&permissioned_as,
|
||||
&schedule.workspace_id,
|
||||
db,
|
||||
&mut *tx,
|
||||
)
|
||||
.await?;
|
||||
(resolved_email, permissioned_as, authed, false)
|
||||
@@ -621,7 +626,7 @@ pub async fn push_scheduled_job<'c>(
|
||||
};
|
||||
|
||||
if let Some(tag) = tag.as_deref().filter(|t| !t.is_empty()) {
|
||||
let is_super_admin = windmill_common::auth::is_super_admin_email(db, &email).await?;
|
||||
let is_super_admin = windmill_common::auth::is_super_admin_email(&mut *tx, &email).await?;
|
||||
crate::check_tag_available_for_push(
|
||||
db,
|
||||
&schedule.workspace_id,
|
||||
|
||||
@@ -222,6 +222,43 @@ mod schedule_push {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// Callers hold `tx` on the pool they pass in, so a burst of schedules the size of the pool
|
||||
// starves it if a push checks out a second connection. On a one-connection pool any such
|
||||
// checkout hangs until the acquire timeout.
|
||||
#[sqlx::test(migrations = "../migrations", fixtures("base", "schedule_push"))]
|
||||
async fn test_push_holds_one_connection(db: Pool<Postgres>) -> anyhow::Result<()> {
|
||||
let one = sqlx::postgres::PgPoolOptions::new()
|
||||
.max_connections(1)
|
||||
.acquire_timeout(std::time::Duration::from_secs(5))
|
||||
.connect_with((*db.connect_options()).clone())
|
||||
.await?;
|
||||
|
||||
let schedules = [
|
||||
make_schedule(|s| {
|
||||
s.script_path = "f/system/tagged_script".to_string();
|
||||
s.path = "f/system/tagged_schedule".to_string();
|
||||
}),
|
||||
make_schedule(|s| {
|
||||
s.script_path = "f/system/obo_script".to_string();
|
||||
s.path = "f/system/obo_schedule".to_string();
|
||||
}),
|
||||
make_schedule(|s| {
|
||||
s.is_flow = true;
|
||||
s.script_path = "f/system/obo_flow".to_string();
|
||||
s.path = "f/system/obo_flow_schedule".to_string();
|
||||
}),
|
||||
];
|
||||
for schedule in &schedules {
|
||||
let tx = one.begin().await?;
|
||||
let push = push_scheduled_job(&one, tx, schedule, None, None);
|
||||
let tx = tokio::time::timeout(std::time::Duration::from_secs(15), push).await??;
|
||||
tx.commit().await?;
|
||||
}
|
||||
|
||||
assert_eq!(count_queued_jobs(&db).await, 3);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
// -----------------------------------------------------------------------
|
||||
// push_scheduled_job: with retry wraps in SingleStepFlow
|
||||
// -----------------------------------------------------------------------
|
||||
|
||||
Reference in New Issue
Block a user