From 5a1c12f67554e5e39769947a576b7e1428fc261e Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Wed, 25 Nov 2020 14:14:26 +0100
Subject: [PATCH] Improve TaskController logging and limit state history (DEV)
(#1717)
* Improve TaskController.kt logging
* Cap TaskController's state history to 50 entries.
* Determine oldest tasks to prune based on `finishedAt` instead of `startedAt`
Co-authored-by: Kolya Opahle <k.opahle@sap.com>
---
.../rki/coronawarnapp/task/TaskController.kt | 22 +++++++---
.../coronawarnapp/task/TaskControllerTest.kt | 40 +++++++++++++++++++
2 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/task/TaskController.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/task/TaskController.kt
index 932fb9beb..b7cc4ca6e 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/task/TaskController.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/task/TaskController.kt
@@ -123,7 +123,6 @@ class TaskController @Inject constructor(
private suspend fun processMap() = internalTaskData.updateSafely {
Timber.tag(TAG).d("Processing task data (count=%d)", size)
- Timber.tag(TAG).v("Tasks before processing: %s", this.values)
// Procress all unprocessed finished tasks
procressFinishedTasks(this).let {
@@ -137,7 +136,19 @@ class TaskController @Inject constructor(
this.putAll(it)
}
- Timber.tag(TAG).v("Tasks after processing: %s", this.values)
+ if (size > TASK_HISTORY_LIMIT) {
+ Timber.v("Enforcing history limits (%d), need to remove %d.", TASK_HISTORY_LIMIT, size - TASK_HISTORY_LIMIT)
+ values
+ .filter { it.isFinished }
+ .sortedBy { it.finishedAt }
+ .take(size - TASK_HISTORY_LIMIT)
+ .forEach {
+ Timber.v("Removing from history: %s", get(it.id))
+ remove(it.id)
+ }
+ }
+
+ Timber.tag(TAG).v("Tasks after processing (count=%d):\n%s", size, values.joinToString("\n"))
}
private fun procressFinishedTasks(data: Map<UUID, InternalTaskState>): Map<UUID, InternalTaskState> {
@@ -178,9 +189,9 @@ class TaskController @Inject constructor(
it.id != state.id
}
Timber.tag(TAG).d("Task has %d siblings", siblingTasks.size)
- Timber.tag(TAG).v(
- "Sibling are:\n%s", siblingTasks.joinToString("\n")
- )
+ if (siblingTasks.isNotEmpty()) {
+ Timber.tag(TAG).v("Sibling are:\n%s", siblingTasks.joinToString("\n"))
+ }
// Handle collision behavior for tasks of same type
when {
@@ -237,5 +248,6 @@ class TaskController @Inject constructor(
companion object {
private const val TAG = "TaskController"
+ private const val TASK_HISTORY_LIMIT = 50
}
}
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/task/TaskControllerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/task/TaskControllerTest.kt
index 581eab4dc..d84078cf9 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/task/TaskControllerTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/task/TaskControllerTest.kt
@@ -539,4 +539,44 @@ class TaskControllerTest : BaseIOTest() {
instance.close()
}
+
+ @Test
+ fun `old tasks are pruned from history`() = runBlockingTest {
+ val instance = createInstance(scope = this)
+
+ val expectedFiles = mutableListOf<File>()
+
+ repeat(100) {
+ val arguments = QueueingTask.Arguments(
+ delay = 5,
+ values = listOf("TestText"),
+ path = File(testDir, UUID.randomUUID().toString())
+ )
+ expectedFiles.add(arguments.path)
+
+ val request = DefaultTaskRequest(type = QueueingTask::class, arguments = arguments)
+ instance.submit(request)
+ delay(5)
+ }
+
+ this.advanceUntilIdle()
+
+ expectedFiles.forEach {
+ it.exists() shouldBe true
+ }
+
+ val taskHistory = instance.tasks.first()
+ taskHistory.size shouldBe 50
+ expectedFiles.size shouldBe 100
+
+ val sortedHistory = taskHistory.sortedBy { it.taskState.startedAt }.apply {
+ first().taskState.startedAt!!.isBefore(last().taskState.startedAt) shouldBe true
+ }
+
+ expectedFiles.subList(50, 100) shouldBe sortedHistory.map {
+ (it.taskState.request.arguments as QueueingTask.Arguments).path
+ }
+
+ instance.close()
+ }
}
--
GitLab