mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-08 08:04:25 +00:00
refactor: run the credential maintenance pass as its own task, without a budget
The pass ran inside the monitor's join, whose deadline cancels every future in it, and a rotation cancelled between GitLab issuing a token and Windmill storing it loses the token family. A wall-clock budget with a least-recently-checked ordering kept it under the deadline. Spawning the pass instead makes the deadline irrelevant, so the budget, the ordering and the counter go; the advisory lock keeps a slow pass from overlapping the next, as it already did. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01C1xHmkxuxYb1GYvth1BS75
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
3d115bca00
commit
c91da8fdf3
+2
-2
@@ -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"
|
||||
}
|
||||
@@ -1 +1 @@
|
||||
28fc36854fe0729223055bf1b7335fa8dc2c56ec
|
||||
ae2a79e33edcf92ab7b3d11b20f4ef8b430954cf
|
||||
|
||||
+8
-45
@@ -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<Postgres>) -> 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<Postgres>) -> 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<Postgres>) -> error::Result<()
|
||||
}
|
||||
}
|
||||
}
|
||||
if skipped > 0 {
|
||||
tracing::info!(
|
||||
"git credentials: pass budget reached, {skipped} repositories deferred to the next pass"
|
||||
);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user