From 157974080ae84b53f670d2aa201f2f9782799a73 Mon Sep 17 00:00:00 2001 From: Max Smirnov Date: Wed, 13 Mar 2024 18:06:51 +0300 Subject: [PATCH 1/7] Add unassignOrGiveUp --- app/src/main/scala/AppState.scala | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/src/main/scala/AppState.scala b/app/src/main/scala/AppState.scala index 19fdc12..1ee898f 100644 --- a/app/src/main/scala/AppState.scala +++ b/app/src/main/scala/AppState.scala @@ -54,6 +54,11 @@ object AppState: case Some(unAssignedTask) => (state.updated(task.id, unAssignedTask), xs) } + def unassignOrGiveUp(task: Work.Task): (AppState, Option[Work.Task]) = + task.clearAssignedKey match + case None => (state - task.id, Some(task)) + case Some(unAssignedTask) => (state.updated(task.id, unAssignedTask), None) + def acquiredBefore(since: Instant): List[Work.Task] = state.values.filter(_.acquiredBefore(since)).toList From 570cd5a9248b1491bf6a1255f18e074944fb8f09 Mon Sep 17 00:00:00 2001 From: Max Smirnov Date: Wed, 13 Mar 2024 18:11:22 +0300 Subject: [PATCH 2/7] Use unassignOrGiveUp in Executor.move --- app/src/main/scala/Executor.scala | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/app/src/main/scala/Executor.scala b/app/src/main/scala/Executor.scala index 6de38f6..ddc2e8f 100644 --- a/app/src/main/scala/Executor.scala +++ b/app/src/main/scala/Executor.scala @@ -68,12 +68,10 @@ object Executor: state.remove(task.id) -> (monitor.success(task) >> client.send(Lila.Response(task.request.id, task.request.moves, uci))) case _ => - val (newState, io) = task.clearAssignedKey match - case None => - state.remove(workId) -> Logger[IO].warn( - s"Give up move due to invalid move $response by $key for $task" - ) - case Some(updated) => state.add(updated) -> IO.unit + val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) + val io = maybeGivenUp.traverse_(task => Logger[IO].warn( + s"Give up move due to invalid move $response by $key for $task" + )) newState -> io *> failure(task, key) def clean(since: Instant): IO[Unit] = From b5c0ef0561d2bc02e9fa861d82c1db386946a426 Mon Sep 17 00:00:00 2001 From: Max Smirnov Date: Wed, 13 Mar 2024 18:14:13 +0300 Subject: [PATCH 3/7] Use unassignOrGiveUp in updateOrGiveUp --- app/src/main/scala/AppState.scala | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/src/main/scala/AppState.scala b/app/src/main/scala/AppState.scala index 1ee898f..b5cfed6 100644 --- a/app/src/main/scala/AppState.scala +++ b/app/src/main/scala/AppState.scala @@ -49,9 +49,8 @@ object AppState: def updateOrGiveUp(candidates: List[Work.Task]): (AppState, List[Work.Task]) = candidates.foldLeft(state -> Nil) { case ((state, xs), task) => - task.clearAssignedKey match - case None => (state - task.id, task :: xs) - case Some(unAssignedTask) => (state.updated(task.id, unAssignedTask), xs) + val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) + (newState, maybeGivenUp.fold(xs)(_ :: xs)) } def unassignOrGiveUp(task: Work.Task): (AppState, Option[Work.Task]) = From 2d9a53f170e389b4ac00fcd68dc753300ce249b7 Mon Sep 17 00:00:00 2001 From: Max Smirnov Date: Wed, 13 Mar 2024 18:28:03 +0300 Subject: [PATCH 4/7] Fix formatting --- app/src/main/scala/Executor.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/scala/Executor.scala b/app/src/main/scala/Executor.scala index ddc2e8f..5ac8cfe 100644 --- a/app/src/main/scala/Executor.scala +++ b/app/src/main/scala/Executor.scala @@ -69,9 +69,9 @@ object Executor: client.send(Lila.Response(task.request.id, task.request.moves, uci))) case _ => val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) - val io = maybeGivenUp.traverse_(task => Logger[IO].warn( - s"Give up move due to invalid move $response by $key for $task" - )) + val io = maybeGivenUp.traverse_(task => + Logger[IO].warn(s"Give up move due to invalid move $response by $key for $task") + ) newState -> io *> failure(task, key) def clean(since: Instant): IO[Unit] = From 19655b6d007acf37dacaf8abf72284b1908f1de8 Mon Sep 17 00:00:00 2001 From: Thanh Le Date: Thu, 14 Mar 2024 09:48:00 +0700 Subject: [PATCH 5/7] Refactor io => logs --- app/src/main/scala/Executor.scala | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/src/main/scala/Executor.scala b/app/src/main/scala/Executor.scala index 5ac8cfe..5f399b7 100644 --- a/app/src/main/scala/Executor.scala +++ b/app/src/main/scala/Executor.scala @@ -69,10 +69,10 @@ object Executor: client.send(Lila.Response(task.request.id, task.request.moves, uci))) case _ => val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) - val io = maybeGivenUp.traverse_(task => + val logs = maybeGivenUp.traverse_(task => Logger[IO].warn(s"Give up move due to invalid move $response by $key for $task") - ) - newState -> io *> failure(task, key) + ) *> failure(task, key) + newState -> logs def clean(since: Instant): IO[Unit] = ref.flatModify: state => From 659b6add990e6e2520c832d8ce5c8196c9589ffe Mon Sep 17 00:00:00 2001 From: Thanh Le Date: Thu, 14 Mar 2024 09:53:48 +0700 Subject: [PATCH 6/7] updateOrGiveup => unassignOrGiveUp --- app/src/main/scala/AppState.scala | 4 ++-- app/src/main/scala/Executor.scala | 2 +- app/src/test/scala/AppStateTest.scala | 10 +++++----- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/src/main/scala/AppState.scala b/app/src/main/scala/AppState.scala index b5cfed6..008fc37 100644 --- a/app/src/main/scala/AppState.scala +++ b/app/src/main/scala/AppState.scala @@ -47,7 +47,7 @@ object AppState: case Some(task) if task.isAcquiredBy(key) => GetTaskResult.Found(task) case Some(task) => GetTaskResult.AcquiredByOther(task) - def updateOrGiveUp(candidates: List[Work.Task]): (AppState, List[Work.Task]) = + def unassignOrGiveUp(candidates: List[Work.Task]): (AppState, List[Work.Task]) = candidates.foldLeft(state -> Nil) { case ((state, xs), task) => val (newState, maybeGivenUp) = state.unassignOrGiveUp(task) (newState, maybeGivenUp.fold(xs)(_ :: xs)) @@ -56,7 +56,7 @@ object AppState: def unassignOrGiveUp(task: Work.Task): (AppState, Option[Work.Task]) = task.clearAssignedKey match case None => (state - task.id, Some(task)) - case Some(unAssignedTask) => (state.updated(task.id, unAssignedTask), None) + case Some(unassignedTask) => (state.updated(task.id, unassignedTask), None) def acquiredBefore(since: Instant): List[Work.Task] = state.values.filter(_.acquiredBefore(since)).toList diff --git a/app/src/main/scala/Executor.scala b/app/src/main/scala/Executor.scala index 5f399b7..6be34fb 100644 --- a/app/src/main/scala/Executor.scala +++ b/app/src/main/scala/Executor.scala @@ -78,7 +78,7 @@ object Executor: ref.flatModify: state => val timedOut = state.acquiredBefore(since) val timedOutLogs = logTimedOut(state, timedOut) - val (newState, gavedUpMoves) = state.updateOrGiveUp(timedOut) + val (newState, gavedUpMoves) = state.unassignOrGiveUp(timedOut) newState -> timedOutLogs *> gavedUpMoves.traverse_(m => Logger[IO].warn(s"Give up move due to clean up: $m")) *> monitor.updateSize(newState) diff --git a/app/src/test/scala/AppStateTest.scala b/app/src/test/scala/AppStateTest.scala index 99c9f63..563e1f9 100644 --- a/app/src/test/scala/AppStateTest.scala +++ b/app/src/test/scala/AppStateTest.scala @@ -71,30 +71,30 @@ class AppStateTest extends ScalaCheckSuite: test("updateOrGiveUp is a subset of given tasks"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) givenUp.toSet.subsetOf(candidates.toSet) test("updateOrGiveUp preserves size"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (newState, givenUp) = state.updateOrGiveUp(candidates) + val (newState, givenUp) = state.unassignOrGiveUp(candidates) newState.size + givenUp.size == state.size test("all given up tasks are outOfTries"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) givenUp.forall(_.isOutOfTries) test("all candidates that are not given up are not outOfTries"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (_, givenUp) = state.updateOrGiveUp(candidates) + val (_, givenUp) = state.unassignOrGiveUp(candidates) val rest = candidates.filterNot(givenUp.contains) rest.forall(!_.isOutOfTries) test("after cleanup, acquiredBefore is empty"): forAll: (state: AppState, before: Instant) => val candidates = state.acquiredBefore(before) - val (newState, _) = state.updateOrGiveUp(candidates) + val (newState, _) = state.unassignOrGiveUp(candidates) newState.acquiredBefore(before).isEmpty From bec9dbfef0b39f52befc3c0bfda15ce7b269ccfb Mon Sep 17 00:00:00 2001 From: Thanh Le Date: Thu, 14 Mar 2024 09:54:07 +0700 Subject: [PATCH 7/7] Remove hardcode scalacheck's seed --- app/src/test/scala/AppStateTest.scala | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/src/test/scala/AppStateTest.scala b/app/src/test/scala/AppStateTest.scala index 563e1f9..e031cb6 100644 --- a/app/src/test/scala/AppStateTest.scala +++ b/app/src/test/scala/AppStateTest.scala @@ -9,8 +9,6 @@ import Arbitraries.given class AppStateTest extends ScalaCheckSuite: - override def scalaCheckInitialSeed = "lwfNzhdC038hCsaHpM4QBkFYs5eFtR9GLPHuzIE08KP=" - test("tasks.fromTasks == identity"): forAll: (state: AppState) => assertEquals(AppState.fromTasks(state.tasks), state)