diff --git a/backend/.sqlx/query-85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc.json b/backend/.sqlx/query-ddd41de79b23b0436bbc7997751c90d7d82bd20ea30f1dc08f2e2066fb8d6f4b.json similarity index 66% rename from backend/.sqlx/query-85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc.json rename to backend/.sqlx/query-ddd41de79b23b0436bbc7997751c90d7d82bd20ea30f1dc08f2e2066fb8d6f4b.json index d3bb2517b7..9622e63a56 100644 --- a/backend/.sqlx/query-85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc.json +++ b/backend/.sqlx/query-ddd41de79b23b0436bbc7997751c90d7d82bd20ea30f1dc08f2e2066fb8d6f4b.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT ws.workspace_id, ws.git_sync\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id\n WHERE NOT w.deleted\n AND ws.git_sync IS NOT NULL\n AND jsonb_typeof(ws.git_sync->'repositories') = 'array'\n ORDER BY (\n SELECT min((elem->'credential'->>'checked_at')::bigint)\n FROM jsonb_array_elements(ws.git_sync->'repositories') AS elem\n ) ASC NULLS LAST", + "query": "SELECT ws.workspace_id, ws.git_sync\n FROM workspace_settings ws\n JOIN workspace w ON w.id = ws.workspace_id\n WHERE NOT w.deleted\n AND ws.git_sync IS NOT NULL\n AND jsonb_typeof(ws.git_sync->'repositories') = 'array'", "describe": { "columns": [ { @@ -22,5 +22,5 @@ true ] }, - "hash": "85730ae5368b0942c256909efab6d33a26bb2c4804c8045d09bed2de03996ecc" + "hash": "ddd41de79b23b0436bbc7997751c90d7d82bd20ea30f1dc08f2e2066fb8d6f4b" } \ No newline at end of file diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index c633c65c35..26fcd98dcd 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -28fc36854fe0729223055bf1b7335fa8dc2c56ec +ae2a79e33edcf92ab7b3d11b20f4ef8b430954cf diff --git a/backend/src/monitor.rs b/backend/src/monitor.rs index 3e8f52cbdf..f788ec99a7 100644 --- a/backend/src/monitor.rs +++ b/backend/src/monitor.rs @@ -4317,12 +4317,16 @@ pub async fn monitor_db( // Re-check what each git-sync repository's own credential says about its expiry, // and rotate the ones close to it. Every ~40 min: the values move over days, and - // `should_run` counts iterations in a u8. + // `should_run` counts iterations in a u8. Spawned rather than joined: the join + // below is cancelled at its deadline, and a rotation cut off between GitLab + // issuing a token and Windmill storing it loses the token family. The pass's + // advisory lock keeps a slow one from overlapping the next. let git_credential_maintenance_f = async { #[cfg(all(feature = "enterprise", feature = "private"))] if server_mode && iteration.is_some() && iteration.as_ref().unwrap().should_run(240) { if let Some(db) = conn.as_sql() { - maintain_git_credentials(db).await; + let db = db.clone(); + tokio::spawn(async move { maintain_git_credentials(&db).await }); } } }; @@ -4708,18 +4712,6 @@ const AUTO_PULL_POLL_SLACK_S: i64 = 30; #[cfg(all(feature = "enterprise", feature = "private"))] const GIT_CREDENTIAL_LOCK_ID: i64 = 737_483_923; -/// Wall-clock budget for one credential maintenance pass, spent between -/// repositories rather than inside one. -/// -/// `monitor_db` cancels every future in its `join!` at 600s, and an unreachable -/// GitLab costs a repository up to the client's 20s timeout, so enough of them -/// would take the whole pass — and the other maintenance futures with it. A -/// rotation must never be cancelled between GitLab issuing a token and Windmill -/// storing it, so the pass stops at a repository boundary instead and the -/// least-recently-checked ordering brings the rest along on the next tick. -#[cfg(all(feature = "enterprise", feature = "private"))] -const GIT_CREDENTIAL_PASS_BUDGET: Duration = Duration::from_secs(180); - /// Refresh every git-sync repository's credential status and rotate the ones near /// expiry, so a token dies visibly (and usually not at all) rather than taking /// sync down on its expiry date. @@ -4772,28 +4764,17 @@ async fn maintain_git_credentials_inner(db: &Pool) -> error::Result<() // Same deleted/archived exclusion as the auto-pull poller: a dead workspace's // settings row survives, and rotating a token for one would be pure damage. - // Least-recently-checked first, so a pass that runs out of budget resumes - // where it stopped instead of re-checking the same head of the list forever. - // A repository with no recorded credential sorts last: it has nothing to - // rotate, and it never records a check, so put first it would hold the head - // of the list ahead of the tokens that expire. let rows = sqlx::query!( r#"SELECT ws.workspace_id, ws.git_sync FROM workspace_settings ws JOIN workspace w ON w.id = ws.workspace_id WHERE NOT w.deleted AND ws.git_sync IS NOT NULL - AND jsonb_typeof(ws.git_sync->'repositories') = 'array' - ORDER BY ( - SELECT min((elem->'credential'->>'checked_at')::bigint) - FROM jsonb_array_elements(ws.git_sync->'repositories') AS elem - ) ASC NULLS LAST"# + AND jsonb_typeof(ws.git_sync->'repositories') = 'array'"# ) .fetch_all(db) .await?; - let started = Instant::now(); - let mut skipped = 0usize; for row in rows { let Some(git_sync) = row.git_sync else { continue; @@ -4809,20 +4790,7 @@ async fn maintain_git_credentials_inner(db: &Pool) -> error::Result<() } }; - // The workspace ordering above only decides which workspace comes first; - // within one, the repositories need the same least-recently-checked - // order or the tail of a large workspace never gets its turn. - let mut repositories: Vec<_> = settings.repositories.iter().collect(); - repositories.sort_by_key(|r| { - let checked_at = r.credential.as_ref().map(|c| c.checked_at); - (checked_at.is_none(), checked_at) - }); - - for repo in repositories { - if started.elapsed() >= GIT_CREDENTIAL_PASS_BUDGET { - skipped += 1; - continue; - } + for repo in settings.repositories.iter() { let path = &repo.git_repo_resource_path; // This refreshes and records the status on every repository it looks at, // rotating only the ones near expiry, so it is the whole maintenance pass @@ -4872,11 +4840,6 @@ async fn maintain_git_credentials_inner(db: &Pool) -> error::Result<() } } } - if skipped > 0 { - tracing::info!( - "git credentials: pass budget reached, {skipped} repositories deferred to the next pass" - ); - } Ok(()) }