Skip to content

Commit 8331f0d

Browse files
committed
various: fix evaluator cancellation and retry lifecycles
Signed-off-by: NotAShelf <raf@notashelf.dev> Change-Id: I061874ad8c52cd82a427d598887494ef6a6a6964
1 parent 91228a7 commit 8331f0d

10 files changed

Lines changed: 428 additions & 85 deletions

File tree

crates/common/src/models.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ pub struct Evaluation {
6767
}
6868

6969
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, sqlx::Type)]
70-
#[serde(rename_all = "lowercase")]
71-
#[sqlx(type_name = "text", rename_all = "lowercase")]
70+
#[serde(rename_all = "snake_case")]
71+
#[sqlx(type_name = "text", rename_all = "snake_case")]
7272
pub enum EvaluationStatus {
7373
Pending,
7474
Running,
@@ -513,7 +513,7 @@ impl<'r> sqlx::Decode<'r, sqlx::Postgres> for BuildStatus {
513513

514514
#[cfg(test)]
515515
mod build_status_tests {
516-
use super::BuildStatus;
516+
use super::{BuildStatus, EvaluationStatus};
517517

518518
#[test]
519519
fn build_status_i32_round_trips() {
@@ -544,6 +544,14 @@ mod build_status_tests {
544544
assert_eq!(BuildStatus::from_i32(-1), None);
545545
assert_eq!(BuildStatus::from_i32(15), None);
546546
}
547+
548+
#[test]
549+
fn timed_out_status_uses_database_spelling() {
550+
assert_eq!(
551+
serde_json::to_string(&EvaluationStatus::TimedOut).unwrap(),
552+
"\"timed_out\""
553+
);
554+
}
547555
}
548556

549557
#[derive(Debug, Clone, Serialize, Deserialize, FromRow)]

crates/common/src/repo/build_dependencies.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use sqlx::PgPool;
1+
use sqlx::{Executor, PgPool, Postgres, Transaction};
22
use uuid::Uuid;
33

44
use crate::{
@@ -17,13 +17,37 @@ pub async fn create(
1717
build_id: Uuid,
1818
dependency_build_id: Uuid,
1919
) -> Result<BuildDependency> {
20+
create_with(pool, build_id, dependency_build_id).await
21+
}
22+
23+
/// Create a build dependency relationship within an existing transaction.
24+
///
25+
/// # Errors
26+
///
27+
/// Returns an error if database insert fails or dependency already exists.
28+
pub async fn create_in_transaction(
29+
tx: &mut Transaction<'_, Postgres>,
30+
build_id: Uuid,
31+
dependency_build_id: Uuid,
32+
) -> Result<BuildDependency> {
33+
create_with(&mut **tx, build_id, dependency_build_id).await
34+
}
35+
36+
async fn create_with<'e, E>(
37+
executor: E,
38+
build_id: Uuid,
39+
dependency_build_id: Uuid,
40+
) -> Result<BuildDependency>
41+
where
42+
E: Executor<'e, Database = Postgres>,
43+
{
2044
sqlx::query_as::<_, BuildDependency>(
2145
"INSERT INTO build_dependencies (build_id, dependency_build_id) VALUES \
2246
($1, $2) RETURNING *",
2347
)
2448
.bind(build_id)
2549
.bind(dependency_build_id)
26-
.fetch_one(pool)
50+
.fetch_one(executor)
2751
.await
2852
.on_unique_violation(|| {
2953
format!(

crates/common/src/repo/builds.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::collections::HashSet;
22

3-
use sqlx::PgPool;
3+
use sqlx::{Executor, PgPool, Postgres, Transaction};
44
use uuid::Uuid;
55

66
use crate::{
@@ -14,6 +14,25 @@ use crate::{
1414
///
1515
/// Returns error if database insert fails or job already exists.
1616
pub async fn create(pool: &PgPool, input: CreateBuild) -> Result<Build> {
17+
create_with(pool, input).await
18+
}
19+
20+
/// Create a new build record within an existing transaction.
21+
///
22+
/// # Errors
23+
///
24+
/// Returns an error if database insert fails or job already exists.
25+
pub async fn create_in_transaction(
26+
tx: &mut Transaction<'_, Postgres>,
27+
input: CreateBuild,
28+
) -> Result<Build> {
29+
create_with(&mut **tx, input).await
30+
}
31+
32+
async fn create_with<'e, E>(executor: E, input: CreateBuild) -> Result<Build>
33+
where
34+
E: Executor<'e, Database = Postgres>,
35+
{
1736
let is_aggregate = input.is_aggregate.unwrap_or(false);
1837
let is_fod = input.is_fod.unwrap_or(false);
1938
sqlx::query_as::<_, Build>(
@@ -37,7 +56,7 @@ pub async fn create(pool: &PgPool, input: CreateBuild) -> Result<Build> {
3756
.bind(&input.meta_homepage)
3857
.bind(&input.meta_maintainers)
3958
.bind(&input.required_features)
40-
.fetch_one(pool)
59+
.fetch_one(executor)
4160
.await
4261
.on_unique_violation(|| {
4362
format!(

crates/common/src/repo/evaluations.rs

Lines changed: 59 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use sqlx::PgPool;
1+
use sqlx::{PgPool, Postgres, Transaction};
22
use uuid::Uuid;
33

44
use crate::{
@@ -365,17 +365,18 @@ pub async fn cancel(pool: &PgPool, id: Uuid) -> Result<Option<Evaluation>> {
365365
)
366366
}
367367

368-
/// Requeue a cancelled or failed evaluation after discarding its stale builds.
368+
/// Requeue a cancelled, failed, or timed-out evaluation after discarding its
369+
/// stale builds. One-shot jobsets are re-enabled for the new attempt.
369370
///
370371
/// # Errors
371372
///
372373
/// Returns an error if the database transaction fails.
373374
pub async fn restart(pool: &PgPool, id: Uuid) -> Result<Option<Evaluation>> {
374375
let mut tx = pool.begin().await?;
375376
let evaluation = sqlx::query_as::<_, Evaluation>(
376-
"UPDATE evaluations SET status = 'pending', error_message = NULL, \
377-
inputs_hash = NULL WHERE id = $1 AND status IN ('cancelled', 'failed') \
378-
RETURNING *",
377+
"UPDATE evaluations SET status = 'pending', evaluation_time = NOW(), \
378+
error_message = NULL, inputs_hash = NULL WHERE id = $1 AND status IN \
379+
('cancelled', 'failed', 'timed_out') RETURNING *",
379380
)
380381
.bind(id)
381382
.fetch_optional(&mut *tx)
@@ -386,11 +387,64 @@ pub async fn restart(pool: &PgPool, id: Uuid) -> Result<Option<Evaluation>> {
386387
.bind(id)
387388
.execute(&mut *tx)
388389
.await?;
390+
sqlx::query(
391+
"UPDATE jobsets SET enabled = true WHERE id = (SELECT jobset_id FROM \
392+
evaluations WHERE id = $1) AND state = 'one_shot'",
393+
)
394+
.bind(id)
395+
.execute(&mut *tx)
396+
.await?;
389397
}
390398
tx.commit().await?;
391399
Ok(evaluation)
392400
}
393401

402+
/// Lock a running evaluation before atomically persisting its result.
403+
///
404+
/// # Errors
405+
///
406+
/// Returns an error if the database query fails.
407+
pub async fn lock_running(
408+
tx: &mut Transaction<'_, Postgres>,
409+
id: Uuid,
410+
) -> Result<bool> {
411+
Ok(
412+
sqlx::query_scalar::<_, Uuid>(
413+
"SELECT id FROM evaluations WHERE id = $1 AND status = 'running' FOR \
414+
UPDATE",
415+
)
416+
.bind(id)
417+
.fetch_optional(&mut **tx)
418+
.await?
419+
.is_some(),
420+
)
421+
}
422+
423+
/// Finish a locked running evaluation within its result transaction.
424+
///
425+
/// # Errors
426+
///
427+
/// Returns an error if the database query fails.
428+
pub async fn finish_running_in_transaction(
429+
tx: &mut Transaction<'_, Postgres>,
430+
id: Uuid,
431+
status: EvaluationStatus,
432+
error_message: Option<&str>,
433+
) -> Result<bool> {
434+
Ok(
435+
sqlx::query_scalar::<_, Uuid>(
436+
"UPDATE evaluations SET status = $1, error_message = $2 WHERE id = $3 \
437+
AND status = 'running' RETURNING id",
438+
)
439+
.bind(status)
440+
.bind(error_message)
441+
.bind(id)
442+
.fetch_optional(&mut **tx)
443+
.await?
444+
.is_some(),
445+
)
446+
}
447+
394448
/// Return whether an evaluator should cancel its currently-running work.
395449
///
396450
/// # Errors

crates/common/src/repo/jobsets.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,15 +269,14 @@ pub async fn list_active(pool: &PgPool) -> Result<Vec<ActiveJobset>> {
269269
)
270270
}
271271

272-
/// Mark a one-shot jobset as complete (set state to disabled).
272+
/// Mark a one-shot jobset as complete without losing its one-shot state.
273273
///
274274
/// # Errors
275275
///
276276
/// Returns error if database update fails.
277277
pub async fn mark_one_shot_complete(pool: &PgPool, id: Uuid) -> Result<()> {
278278
sqlx::query(
279-
"UPDATE jobsets SET state = 'disabled', enabled = false WHERE id = $1 AND \
280-
state = 'one_shot'",
279+
"UPDATE jobsets SET enabled = false WHERE id = $1 AND state = 'one_shot'",
281280
)
282281
.bind(id)
283282
.execute(pool)

0 commit comments

Comments
 (0)