Skip to content

Commit 3a2e0c9

Browse files
committed
fix(server): support caching absence of version artifacts
Part of #2160. The goal is to reduce the number of calls to GitHub and hopefully make the server more stable when lots of requests come.
1 parent d82ade8 commit 3a2e0c9

File tree

4 files changed

+32
-9
lines changed

4 files changed

+32
-9
lines changed

jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/ArtifactRoutes.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,6 @@ import kotlinx.coroutines.launch
2828

2929
private val logger = logger { }
3030

31-
typealias CachedVersionArtifact = VersionArtifacts?
32-
3331
private val prefetchScope = CoroutineScope(Dispatchers.IO)
3432

3533
fun Routing.artifactRoutes(
@@ -120,7 +118,7 @@ private suspend fun ApplicationCall.toBindingArtifacts(
120118
if (refresh) {
121119
bindingsCache.invalidate(actionCoords)
122120
}
123-
return bindingsCache.get(actionCoords)
121+
return bindingsCache.get(actionCoords).toNullableVersionArtifacts()
124122
}
125123

126124
private fun PrometheusMeterRegistry.incrementArtifactCounter(
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package io.github.typesafegithub.workflows.jitbindingserver
2+
3+
import io.github.typesafegithub.workflows.mavenbinding.VersionArtifacts
4+
5+
/**
6+
* This wrapper exists because Caffeine/Aedile cache doesn't support caching
7+
* null values.
8+
*/
9+
sealed interface CachedVersionArtifact {
10+
object Absent : CachedVersionArtifact
11+
12+
class Present(
13+
val artifacts: VersionArtifacts,
14+
) : CachedVersionArtifact
15+
}
16+
17+
fun CachedVersionArtifact.toNullableVersionArtifacts(): VersionArtifacts? =
18+
when (this) {
19+
is CachedVersionArtifact.Absent -> null
20+
is CachedVersionArtifact.Present -> artifacts
21+
}
22+
23+
fun VersionArtifacts?.toCachedVersionArtifact(): CachedVersionArtifact =
24+
when (this) {
25+
null -> CachedVersionArtifact.Absent
26+
else -> CachedVersionArtifact.Present(this)
27+
}

jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/Main.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ private fun buildBindingsCache(
109109
.newBuilder()
110110
.refreshAfterWrite(1.hours)
111111
.recordStats()
112-
.asLoadingCache<ActionCoords, CachedVersionArtifact> { buildVersionArtifacts(it, httpClient) }
112+
.asLoadingCache { buildVersionArtifacts(it, httpClient).toCachedVersionArtifact() }
113113

114114
@Suppress("ktlint:standard:function-signature") // Conflict with detekt.
115115
private fun buildMetadataCache(

jit-binding-server/src/test/kotlin/io/github/typesafegithub/workflows/jitbindingserver/ArtifactRoutesTest.kt

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,9 @@ class ArtifactRoutesTest :
8888
// Then
8989
response2.status shouldBe HttpStatusCode.NotFound
9090

91-
// This test shows the current behavior where requesting a resource
92-
// that doesn't exist twice in a row causes calling the version artifact
93-
// twice.
94-
// Fix in scope of https://github.com/typesafegithub/github-workflows-kt/issues/2160
95-
verify(exactly = 2) { mockBuildVersionArtifacts(any(), any()) }
91+
// The fact that the resource doesn't exist is cached, and the
92+
// resource generation logic isn't called in the second request.
93+
verify(exactly = 1) { mockBuildVersionArtifacts(any(), any()) }
9694
}
9795
}
9896

0 commit comments

Comments
 (0)