From b197a4cd5aeec9dff01f98010bb216f763990846 Mon Sep 17 00:00:00 2001 From: Johann Wagner Date: Mon, 16 Feb 2026 09:26:23 +0100 Subject: [PATCH] fix: Implemented cleanup locks --- dashboard/src/components/task.tsx | 16 +++- dashboard/src/services/api.tsx | 2 +- .../down.sql | 17 +++++ .../2026-02-16-100033-0000_lock_clean/up.sql | 4 + vicky/src/lib/database/entities/lock.rs | 9 +++ vicky/src/lib/database/entities/task.rs | 16 ++++ vicky/src/lib/vicky/scheduler.rs | 75 +++++++++++++++++-- 7 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 vicky/migrations/2026-02-16-100033-0000_lock_clean/down.sql create mode 100644 vicky/migrations/2026-02-16-100033-0000_lock_clean/up.sql diff --git a/dashboard/src/components/task.tsx b/dashboard/src/components/task.tsx index 1b37e35..e27c62c 100644 --- a/dashboard/src/components/task.tsx +++ b/dashboard/src/components/task.tsx @@ -51,6 +51,18 @@ const Task = (props: TaskProps) => { } } + const BADGE_LOCK_COLOR = { + "WRITE": "red", + "READ": "green", + "CLEAN": "red", + } + + const BADGE_LOCK_CONTENT = { + "WRITE": "W", + "READ": "R", + "CLEAN": "C", + } + return ( @@ -75,8 +87,8 @@ const Task = (props: TaskProps) => { return ( {lock.name} diff --git a/dashboard/src/services/api.tsx b/dashboard/src/services/api.tsx index 0026713..a3eeb58 100644 --- a/dashboard/src/services/api.tsx +++ b/dashboard/src/services/api.tsx @@ -5,7 +5,7 @@ type ITask = { id: string, display_name: string, locks: { - type: "WRITE" | "READ" + type: "WRITE" | "READ" | "CLEAN" name: string, poisoned: string, }[] diff --git a/vicky/migrations/2026-02-16-100033-0000_lock_clean/down.sql b/vicky/migrations/2026-02-16-100033-0000_lock_clean/down.sql new file mode 100644 index 0000000..f27be20 --- /dev/null +++ b/vicky/migrations/2026-02-16-100033-0000_lock_clean/down.sql @@ -0,0 +1,17 @@ +-- This file should undo anything in `up.sql` + +-- can't drop enum values from an enum. +CREATE TYPE "LockKind_Type_New" AS ENUM ( + 'READ', + 'WRITE', +); + +DELETE FROM locks WHERE type = 'CLEAN'; + +ALTER TABLE tasks + ALTER COLUMN status TYPE "LockKind_Type_New" + USING (status::text::"LockKind_Type_New"); + +DROP TYPE "LockKind_Type"; + +ALTER TYPE "LockKind_Type_New" RENAME TO "LockKind_Type"; \ No newline at end of file diff --git a/vicky/migrations/2026-02-16-100033-0000_lock_clean/up.sql b/vicky/migrations/2026-02-16-100033-0000_lock_clean/up.sql new file mode 100644 index 0000000..b562674 --- /dev/null +++ b/vicky/migrations/2026-02-16-100033-0000_lock_clean/up.sql @@ -0,0 +1,4 @@ +-- Your SQL goes here + +ALTER TYPE "LockKind_Type" ADD VALUE 'CLEAN'; + diff --git a/vicky/src/lib/database/entities/lock.rs b/vicky/src/lib/database/entities/lock.rs index 6e515a1..ad242db 100644 --- a/vicky/src/lib/database/entities/lock.rs +++ b/vicky/src/lib/database/entities/lock.rs @@ -26,12 +26,16 @@ use crate::database::entities::task::db_impl::DbTask; pub enum LockKind { Read, Write, + Clean } impl LockKind { pub fn is_write(&self) -> bool { matches!(self, LockKind::Write) } + pub fn is_cleanup(&self) -> bool { + matches!(self, LockKind::Clean) + } } impl TryFrom<&str> for LockKind { @@ -41,6 +45,7 @@ impl TryFrom<&str> for LockKind { match value { "READ" => Ok(Self::Read), "WRITE" => Ok(Self::Write), + "CLEAN" => Ok(Self::Clean), _ => Err("Unexpected lock type received."), } } @@ -64,6 +69,10 @@ impl Lock { Self::new(name, LockKind::Write) } + pub fn clean>(name: S) -> Self { + Self::new(name, LockKind::Clean) + } + pub fn is_conflicting(&self, other: &Lock) -> bool { if self.name() != other.name() { return false; diff --git a/vicky/src/lib/database/entities/task.rs b/vicky/src/lib/database/entities/task.rs index b83ffa5..fda26a3 100644 --- a/vicky/src/lib/database/entities/task.rs +++ b/vicky/src/lib/database/entities/task.rs @@ -125,6 +125,11 @@ impl TaskBuilder { self } + pub fn clean_lock>(mut self, name: S) -> Self { + self.locks.push(Lock::clean(name)); + self + } + pub fn locks(mut self, locks: Vec) -> Self { self.locks = locks; self @@ -221,6 +226,17 @@ impl TaskStatus { } } } + + pub fn is_finished(&self) -> bool { + match self { + TaskStatus::NeedsUserValidation + | TaskStatus::New + | TaskStatus::Running => false, + TaskStatus::Finished(_) => { + true + } + } + } } // this was on purpose because these macro-generated entity types diff --git a/vicky/src/lib/vicky/scheduler.rs b/vicky/src/lib/vicky/scheduler.rs index 98f686e..9c7ce8f 100644 --- a/vicky/src/lib/vicky/scheduler.rs +++ b/vicky/src/lib/vicky/scheduler.rs @@ -32,11 +32,11 @@ impl<'a> ConstraintMgmt<'a> for Constraints<'a> { if !self.contains_key(lock.name()) { return true; // lock wasn't used yet } - let Some(lock) = self.get(lock.name()) else { - return false; // block execution if missing lock entry + if let Some(existing_lock) = self.get(lock.name()) { + return !lock.is_conflicting(existing_lock) }; - !lock.is_conflicting(lock) + false } fn from_tasks(tasks: &'a [Task]) -> Result { @@ -81,6 +81,13 @@ impl<'a> Scheduler<'a> { Ok(s) } + fn is_cleanup_and_conflicts(&self, lock: &Lock) -> bool { + // If there is any other lock, we conflict with them and we do not want to be added to the queue. + // We cannot use constraints here, because constraints only contains locks with the status running. + let other_task_with_same_lock_exists = self.tasks.iter().any(|task| !task.status.is_finished() && task.locks.iter().any(|lock| !lock.kind.is_cleanup() && lock.name() == lock.name())); + lock.kind.is_cleanup() && other_task_with_same_lock_exists + } + fn is_poisoned(&self, lock: &Lock) -> bool { self.poisoned_locks .iter() @@ -90,7 +97,7 @@ impl<'a> Scheduler<'a> { fn is_unconstrained(&self, task: &Task) -> bool { task.locks .iter() - .all(|lock| self.constraints.can_get_lock(lock) && !self.is_poisoned(lock)) + .all(|lock| self.constraints.can_get_lock(lock) && !self.is_poisoned(lock) && !self.is_cleanup_and_conflicts(lock)) } fn supports_all_features(&self, task: &Task) -> bool { @@ -117,7 +124,7 @@ impl<'a> Scheduler<'a> { mod tests { use uuid::Uuid; - use crate::database::entities::task::TaskStatus; + use crate::database::entities::task::{TaskResult, TaskStatus}; use crate::database::entities::{Lock, Task}; use super::Scheduler; @@ -314,6 +321,64 @@ mod tests { assert_eq!(res.get_next_task(), None) } + #[test] + fn scheduler_new_task_cleanup_single() { + let tasks = vec![ + Task::builder() + .display_name("Test 1") + .status(TaskStatus::New) + .clean_lock("foo1") + .build_expect(), + ]; + + let res = Scheduler::new(&tasks, &[], &[]).unwrap(); + // Test 1 is currently running and has the write lock + assert_eq!(res.get_next_task().unwrap().display_name, "Test 1") + } + + #[test] + fn scheduler_new_task_cleanup_with_finished() { + let tasks = vec![ + Task::builder() + .display_name("Test 5") + .status(TaskStatus::Finished(TaskResult::Success)) + .write_lock("foo1") + .build_expect(), + Task::builder() + .display_name("Test 1") + .status(TaskStatus::New) + .clean_lock("foo1") + .build_expect(), + ]; + + let res = Scheduler::new(&tasks, &[], &[]).unwrap(); + // Test 1 is currently running and has the write lock + assert_eq!(res.get_next_task().unwrap().display_name, "Test 1") + } + + + #[test] + fn scheduler_new_task_cleanup() { + let tasks = vec![ + Task::builder() + .display_name("Test 1") + .status(TaskStatus::New) + .clean_lock("foo1") + .build_expect(), + Task::builder() + .display_name("Test 2") + .status(TaskStatus::New) + .read_lock("foo1") + .build_expect(), + ]; + + let res = Scheduler::new(&tasks, &[], &[]).unwrap(); + // Test 1 is currently running and has the write lock + assert_eq!(res.get_next_task().unwrap().display_name, "Test 2") + } + + + #[test] fn schedule_with_poisoned_lock() { let tasks = vec![