From 9878c7a19432518fe93e20c51ef34edb792ed669 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Fri, 5 Jun 2026 09:51:54 +0200 Subject: [PATCH 1/3] fix(store): omit empty statuses from findOldestCreatedAt (false lag-gauge reading) PostgresOutboxStore and MysqlOutboxStore pre-seeded the result map with clock.instant() for every requested status before querying, then let the SQL overwrite only the statuses that have rows. A status with no entries therefore came back with a near-now timestamp instead of being absent. The lag gauge (okapi-micrometer) relies on absence to report 0 for empty statuses: `snapshot.oldest[status]?.let { Duration.between(it, now) } ?: 0.0`. With the pre-seed, an idle status reported a small false lag (the elapsed time between the store query and the gauge publish) instead of 0. The unit test never caught this: it uses a stub whose findOldestCreatedAt does `filterKeys { it in statuses }` (correctly omitting absent statuses), so the stub and the real adapters disagreed. Fixed by starting from an empty map so absent statuses are omitted and the gauge's `?: 0.0` fallback applies. Added a shared OutboxStore contract test (runs against real Postgres AND MySQL via Testcontainers) asserting that findOldestCreatedAt returns a key only for statuses that actually have rows. Note: `clock` is now unused by both stores. Removing it is a breaking constructor change touching ~20 call sites (autoconfig, benchmarks, tests) and is left to a separate follow-up to keep this fix focused. --- .../test/store/OutboxStoreContractTests.kt | 21 +++++++++++++++++++ .../okapi/mysql/MysqlOutboxStore.kt | 2 +- .../okapi/postgres/PostgresOutboxStore.kt | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/store/OutboxStoreContractTests.kt b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/store/OutboxStoreContractTests.kt index 04ce1a2..2af0948 100644 --- a/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/store/OutboxStoreContractTests.kt +++ b/okapi-integration-tests/src/test/kotlin/com/softwaremill/okapi/test/store/OutboxStoreContractTests.kt @@ -225,6 +225,27 @@ fun FunSpec.outboxStoreContractTests( counts shouldContain (OutboxStatus.FAILED to 1L) } + test("[$dbName] findOldestCreatedAt returns the oldest entry only for statuses that have rows") { + // Regression guard: a status with no rows must be ABSENT from the result map, not defaulted + // to "now". The lag gauge (okapi-micrometer) relies on absence to report 0 for empty statuses; + // if the store returns a near-now timestamp for an empty status the gauge shows a tiny false lag. + val older = Instant.parse("2024-01-01T00:00:00Z") + val newer = Instant.parse("2024-01-02T00:00:00Z") + jdbc.withTransaction { + store.persist(createTestEntry(now = older, messageType = "type.older")) + store.persist(createTestEntry(now = newer, messageType = "type.newer")) + } + + val oldest = jdbc.withTransaction { + store.findOldestCreatedAt(setOf(OutboxStatus.PENDING, OutboxStatus.DELIVERED, OutboxStatus.FAILED)) + } + + // PENDING has two entries -> oldest createdAt is the earlier one. + oldest shouldContain (OutboxStatus.PENDING to older) + // DELIVERED / FAILED have no rows -> omitted entirely, not seeded with the current time. + oldest.keys shouldBe setOf(OutboxStatus.PENDING) + } + test("[$dbName] claimPending returns empty when no PENDING entries") { val claimed = jdbc.withTransaction { store.claimPending(10) } diff --git a/okapi-mysql/src/main/kotlin/com/softwaremill/okapi/mysql/MysqlOutboxStore.kt b/okapi-mysql/src/main/kotlin/com/softwaremill/okapi/mysql/MysqlOutboxStore.kt index 39eeb48..dcdcbfc 100644 --- a/okapi-mysql/src/main/kotlin/com/softwaremill/okapi/mysql/MysqlOutboxStore.kt +++ b/okapi-mysql/src/main/kotlin/com/softwaremill/okapi/mysql/MysqlOutboxStore.kt @@ -107,7 +107,7 @@ class MysqlOutboxStore( } override fun findOldestCreatedAt(statuses: Set): Map { - val result = statuses.associateWith { clock.instant() }.toMutableMap() + val result = mutableMapOf() val placeholders = statuses.joinToString(",") { "?" } val sql = "SELECT status, MIN(created_at) AS min_created_at FROM okapi_outbox WHERE status IN ($placeholders) GROUP BY status" diff --git a/okapi-postgres/src/main/kotlin/com/softwaremill/okapi/postgres/PostgresOutboxStore.kt b/okapi-postgres/src/main/kotlin/com/softwaremill/okapi/postgres/PostgresOutboxStore.kt index 2c4036d..a1af83c 100644 --- a/okapi-postgres/src/main/kotlin/com/softwaremill/okapi/postgres/PostgresOutboxStore.kt +++ b/okapi-postgres/src/main/kotlin/com/softwaremill/okapi/postgres/PostgresOutboxStore.kt @@ -101,7 +101,7 @@ class PostgresOutboxStore( } override fun findOldestCreatedAt(statuses: Set): Map { - val result = statuses.associateWith { clock.instant() }.toMutableMap() + val result = mutableMapOf() val placeholders = statuses.joinToString(",") { "?" } val sql = "SELECT status, MIN(created_at) AS min_created_at FROM okapi_outbox WHERE status IN ($placeholders) GROUP BY status" From 00eb5e0f6fb03da20aa07573963fc3743627ab24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Fri, 5 Jun 2026 10:13:51 +0200 Subject: [PATCH 2/3] docs(store): document that findOldestCreatedAt omits empty statuses The absent-means-no-backlog contract was implicit and was the root of the lag-gauge bug just fixed. Stating it on the interface inoculates against a future implementer re-seeding defaults and silently reintroducing it. (Code review follow-up.) --- .../main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt b/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt index b9252fb..dbe0ac0 100644 --- a/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt +++ b/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt @@ -19,7 +19,11 @@ interface OutboxStore { */ fun removeDeliveredBefore(time: Instant, limit: Int): Int - /** Returns the oldest createdAt per status (useful for lag metrics). */ + /** + * Returns the oldest createdAt per status (useful for lag metrics). + * Statuses with no matching entries are omitted from the returned map -- callers rely on + * absence to mean "no backlog" (e.g. the lag gauge reports 0 for an omitted status). + */ fun findOldestCreatedAt(statuses: Set): Map /** Returns entry count per status. */ From 249159d5865842efd12ab965c0b8d1de94767772 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20Kobyli=C5=84ski?= Date: Fri, 5 Jun 2026 10:20:53 +0200 Subject: [PATCH 3/3] docs(store): clarify findOldestCreatedAt return shape Refines the previous KDoc: spells out that the map is status -> createdAt of that status's oldest entry (min createdAt), not just 'oldest createdAt per status'. Keeps the absent-means-no-backlog contract note. --- .../main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt b/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt index dbe0ac0..d0d7ee2 100644 --- a/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt +++ b/okapi-core/src/main/kotlin/com/softwaremill/okapi/core/OutboxStore.kt @@ -20,9 +20,11 @@ interface OutboxStore { fun removeDeliveredBefore(time: Instant, limit: Int): Int /** - * Returns the oldest createdAt per status (useful for lag metrics). - * Statuses with no matching entries are omitted from the returned map -- callers rely on + * For each of the given [statuses] that has at least one entry, maps that status to the + * [OutboxEntry.createdAt] of its oldest entry (the minimum `createdAt` among entries in + * that status). Statuses with no entries are omitted from the result -- callers rely on * absence to mean "no backlog" (e.g. the lag gauge reports 0 for an omitted status). + * Useful for lag metrics. */ fun findOldestCreatedAt(statuses: Set): Map