diff --git a/README.md b/README.md index 7e6a06c1..8817d9fc 100644 --- a/README.md +++ b/README.md @@ -193,9 +193,17 @@ Notes and current limitations: warning; point `--gitEngine=subprocess` at such workspaces to skip the in-process attempt (a partial clone in particular needs the subprocess engine to *serve* queries, since JGit cannot lazily fetch the missing blobs a checkout needs). -* Hashes are cached on local disk via `--cacheDir` and survive restarts. The cache layer is - pluggable behind a byte-oriented interface so a remote backend (e.g. S3) can be added without - touching callers. +* Hashes are cached on local disk via `--cacheDir` and survive restarts. Left unbounded the cache + grows by one entry per distinct commit SHA queried, so a long-running server can bound it with any + combination of `--cacheMaxAge` (expire entries not read or written within a window, e.g. `7d`), + `--cacheMaxEntries`, and `--cacheMaxSize` (e.g. `10GB`, `500MB`, or a bare byte count). A background + sweeper enforces the limits once at startup and then every `--cachePruneInterval` (default `1h`), + evicting least-recently-used entries first — a cache hit refreshes an entry's recency, so revisions + under active query are not expired out from under live traffic. With no `--cacheMax*` flag set the + cache is never pruned (the previous behavior). The cache layer is pluggable behind a byte-oriented + interface so a remote backend (e.g. S3) can be added without touching callers; such a backend + manages its own retention (e.g. a bucket lifecycle policy), and the in-process `--cacheMax*` flags + do not apply to it. * Query-affecting flags (`--useCquery`, `--fineGrainedHashExternalRepos`, etc.) mirror `generate-hashes`, and are folded into the cache key so a server started with different flags never serves another configuration's cached hashes. @@ -461,6 +469,10 @@ Command-line utility to analyze the state of the bazel build graph Usage: bazel-diff serve [-hkvV] [--[no-]excludeExternalTargets] [--no-initial-fetch] [--[no-]trackDeps] [--[no-] useCquery] [-b=] --cacheDir= + [--cacheMaxAge=] + [--cacheMaxEntries=] + [--cacheMaxSize=] + [--cachePruneInterval=] [--cqueryExpression=] [--excludeTargetsQuery=] [--fineGrainedHashExternalReposFile= Directory where generated hashes are cached per commit SHA. Persists across restarts. + --cacheMaxAge= + Evict cached hashes not read or written within this + window, so the cache does not grow without bound + over time. Duration like 7d, 36h, 90m (units + d/h/m/s). Unset means no age limit. Enforced by a + background sweeper (see --cachePruneInterval). + --cacheMaxEntries= + Keep at most this many cached commit-SHA entries, + evicting the least-recently-used first. Unset + means no count limit. + --cacheMaxSize= + Keep the cache's total on-disk size at or below + this, evicting the least-recently-used entries + first. Size like 10GB, 500MB, or a bare byte + count (base 1024). Unset means no size limit. + --cachePruneInterval= + How often the background sweeper enforces the + --cacheMax* limits. Duration like 1h, 30m. + Defaults to 1h. No effect unless a --cacheMax* + limit is set. -co, --bazelCommandOptions= Additional space separated Bazel command options used when invoking `bazel query` diff --git a/cli/BUILD b/cli/BUILD index 76172dbe..5992a227 100644 --- a/cli/BUILD +++ b/cli/BUILD @@ -109,6 +109,18 @@ kt_jvm_test( runtime_deps = [":cli-test-lib"], ) +kt_jvm_test( + name = "DurationConverterTest", + test_class = "com.bazel_diff.cli.converter.DurationConverterTest", + runtime_deps = [":cli-test-lib"], +) + +kt_jvm_test( + name = "ByteSizeConverterTest", + test_class = "com.bazel_diff.cli.converter.ByteSizeConverterTest", + runtime_deps = [":cli-test-lib"], +) + kt_jvm_test( name = "DeserialiseHashesInteractorTest", test_class = "com.bazel_diff.interactor.DeserialiseHashesInteractorTest", @@ -243,6 +255,12 @@ kt_jvm_test( runtime_deps = [":cli-test-lib"], ) +kt_jvm_test( + name = "CachePrunerTest", + test_class = "com.bazel_diff.server.CachePrunerTest", + runtime_deps = [":cli-test-lib"], +) + kt_jvm_test( name = "GitClientTest", test_class = "com.bazel_diff.server.GitClientTest", diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt b/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt index c6856099..d36a9a29 100644 --- a/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt +++ b/cli/src/main/kotlin/com/bazel_diff/cli/ServeCommand.kt @@ -1,6 +1,8 @@ package com.bazel_diff.cli +import com.bazel_diff.cli.converter.ByteSizeConverter import com.bazel_diff.cli.converter.CommaSeparatedValueConverter +import com.bazel_diff.cli.converter.DurationConverter import com.bazel_diff.cli.converter.NormalisingPathConverter import com.bazel_diff.cli.converter.OptionsConverter import com.bazel_diff.di.hasherModule @@ -9,6 +11,8 @@ import com.bazel_diff.di.serialisationModule import com.bazel_diff.extensions.toHexString import com.bazel_diff.hash.sha256 import com.bazel_diff.server.BazelDiffServer +import com.bazel_diff.server.CachePruneLimits +import com.bazel_diff.server.CachePruner import com.bazel_diff.server.GitClient import com.bazel_diff.server.HashCacheStorage import com.bazel_diff.server.HashProvider @@ -17,8 +21,10 @@ import com.bazel_diff.server.ImpactedTargetsService import com.bazel_diff.server.JGitClient import com.bazel_diff.server.LocalDiskHashCacheStorage import com.bazel_diff.server.ProcessGitClient +import com.bazel_diff.server.PrunableHashCacheStorage import java.io.File import java.nio.file.Path +import java.time.Duration import java.util.concurrent.Callable import java.util.concurrent.CountDownLatch import java.util.concurrent.atomic.AtomicBoolean @@ -107,6 +113,44 @@ class ServeCommand : Callable { converter = [NormalisingPathConverter::class]) lateinit var cacheDir: Path + @CommandLine.Option( + names = ["--cacheMaxAge"], + description = + [ + "Evict cached hashes not read or written within this window, so the cache does not " + + "grow without bound over time. Duration like 7d, 36h, 90m (units d/h/m/s). Unset " + + "means no age limit. Enforced by a background sweeper (see --cachePruneInterval)."], + converter = [DurationConverter::class]) + var cacheMaxAge: Duration? = null + + @CommandLine.Option( + names = ["--cacheMaxEntries"], + description = + [ + "Keep at most this many cached commit-SHA entries, evicting the least-recently-used " + + "first. Unset means no count limit."]) + var cacheMaxEntries: Int? = null + + @CommandLine.Option( + names = ["--cacheMaxSize"], + description = + [ + "Keep the cache's total on-disk size at or below this, evicting the " + + "least-recently-used entries first. Size like 10GB, 500MB, or a bare byte count " + + "(base 1024). Unset means no size limit."], + converter = [ByteSizeConverter::class]) + var cacheMaxSize: Long? = null + + @CommandLine.Option( + names = ["--cachePruneInterval"], + description = + [ + "How often the background sweeper enforces the --cacheMax* limits. Duration like 1h, " + + "30m. Defaults to 1h. No effect unless a --cacheMax* limit is set."], + converter = [DurationConverter::class], + defaultValue = "1h") + var cachePruneInterval: Duration = Duration.ofHours(1) + @CommandLine.Option( names = ["--warmupRevision"], description = @@ -220,6 +264,10 @@ class ServeCommand : Callable { "size and memory. Defaults to false."]) var trackDeps = false + // The background cache sweeper, when a --cacheMax* limit is configured. Held so the shutdown path + // can stop it; null when pruning is disabled or the backend manages its own retention. + private var cachePruner: CachePruner? = null + override fun call(): Int { org.koin.core.context.GlobalContext.stopKoin() startKoin { @@ -288,12 +336,41 @@ class ServeCommand : Callable { ImpactedTargetsService(gitClient, hashService, depsTracked = trackDeps) val ready = AtomicBoolean(false) - val server = BazelDiffServer(port, impactedTargetsService, requestTimeoutSeconds) { ready.get() } + val server = + BazelDiffServer(port, impactedTargetsService, requestTimeoutSeconds) { ready.get() } server.start() performInitialFetch(gitClient, hashService, ready, server) + // Start sweeping only after warmup, so the immediate first pass evaluates an already-warm + // cache. + cachePruner = buildCachePruner(storage)?.also { it.start() } return server } + /** + * The cache-pruning limits requested via the `--cacheMax*` flags (all-null = pruning disabled). + */ + fun cachePruneLimits(): CachePruneLimits = + CachePruneLimits(maxAge = cacheMaxAge, maxEntries = cacheMaxEntries, maxBytes = cacheMaxSize) + + /** + * Builds the background [CachePruner] for [storage], or null when no `--cacheMax*` limit is set + * or the backend manages its own retention. Only a [PrunableHashCacheStorage] can be swept + * in-process -- a remote backend (e.g. S3) would instead rely on a bucket lifecycle policy, so a + * limit set against a non-prunable backend is reported and ignored rather than silently + * pretended-to. + */ + fun buildCachePruner(storage: HashCacheStorage): CachePruner? { + val limits = cachePruneLimits() + if (!limits.hasAny) return null + if (storage !is PrunableHashCacheStorage) { + System.err.println( + "[Warn] --cacheMax* is set but the cache backend does not support in-process pruning; " + + "ignoring (rely on the backend's own retention policy instead)") + return null + } + return CachePruner(storage, limits, cachePruneInterval) + } + /** * Performs the startup git fetch (unless [noInitialFetch]) and flips [ready] to true once the * clone is known good. On a fetch failure the server is left running but un-ready ("lame duck") @@ -357,6 +434,7 @@ class ServeCommand : Callable { Runtime.getRuntime() .addShutdownHook( Thread { + cachePruner?.stop() server.stop(1) latch.countDown() }) @@ -364,6 +442,7 @@ class ServeCommand : Callable { latch.await() } catch (e: InterruptedException) { // Treated as a shutdown signal: stop serving and restore the interrupt flag. + cachePruner?.stop() server.stop(1) Thread.currentThread().interrupt() } diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/converter/ByteSizeConverter.kt b/cli/src/main/kotlin/com/bazel_diff/cli/converter/ByteSizeConverter.kt new file mode 100644 index 00000000..c3049764 --- /dev/null +++ b/cli/src/main/kotlin/com/bazel_diff/cli/converter/ByteSizeConverter.kt @@ -0,0 +1,39 @@ +package com.bazel_diff.cli.converter + +import picocli.CommandLine.ITypeConverter +import picocli.CommandLine.TypeConversionException + +/** + * Parses a human-friendly byte size such as `10GB`, `500MB`, `2g`, `1kb`, or a bare `1048576` + * (bytes when no unit is given). Units are base 1024 and case-insensitive: `b`, `k`/`kb`, `m`/`mb`, + * `g`/`gb`, `t`/`tb`. Returns the size in bytes. + */ +class ByteSizeConverter : ITypeConverter { + override fun convert(value: String): Long { + val match = + PATTERN.matchEntire(value.trim().lowercase()) + ?: throw TypeConversionException( + "invalid size '$value' (expected e.g. 1048576, 500mb, 10gb)") + val amount = match.groupValues[1].toLong() + val multiplier = + when (match.groupValues[2]) { + "", + "b" -> 1L + "k", + "kb" -> KB + "m", + "mb" -> KB * KB + "g", + "gb" -> KB * KB * KB + "t", + "tb" -> KB * KB * KB * KB + else -> throw TypeConversionException("invalid size unit in '$value'") + } + return amount * multiplier + } + + private companion object { + const val KB = 1024L + val PATTERN = Regex("(\\d+)\\s*([a-z]*)") + } +} diff --git a/cli/src/main/kotlin/com/bazel_diff/cli/converter/DurationConverter.kt b/cli/src/main/kotlin/com/bazel_diff/cli/converter/DurationConverter.kt new file mode 100644 index 00000000..6af20b77 --- /dev/null +++ b/cli/src/main/kotlin/com/bazel_diff/cli/converter/DurationConverter.kt @@ -0,0 +1,40 @@ +package com.bazel_diff.cli.converter + +import java.time.Duration +import picocli.CommandLine.ITypeConverter +import picocli.CommandLine.TypeConversionException + +/** + * Parses a human-friendly duration such as `7d`, `36h`, `90m`, `45s`, or a combination like + * `1d12h30m`. Recognised unit suffixes are `d` (days), `h` (hours), `m` (minutes) and `s` + * (seconds). The whole string must be made up of `` tokens; anything else is rejected + * so a typo fails fast rather than being silently truncated. + */ +class DurationConverter : ITypeConverter { + override fun convert(value: String): Duration { + val normalized = value.trim().lowercase() + val tokens = TOKEN.findAll(normalized).toList() + val consumed = tokens.sumOf { it.value.length } + if (tokens.isEmpty() || consumed != normalized.length) { + throw TypeConversionException( + "invalid duration '$value' (expected e.g. 7d, 36h, 90m, 45s, or 1d12h)") + } + var total = Duration.ZERO + for (token in tokens) { + val amount = token.groupValues[1].toLong() + total += + when (token.groupValues[2]) { + "d" -> Duration.ofDays(amount) + "h" -> Duration.ofHours(amount) + "m" -> Duration.ofMinutes(amount) + "s" -> Duration.ofSeconds(amount) + else -> throw TypeConversionException("invalid duration unit in '$value'") + } + } + return total + } + + private companion object { + val TOKEN = Regex("(\\d+)([dhms])") + } +} diff --git a/cli/src/main/kotlin/com/bazel_diff/server/CachePruner.kt b/cli/src/main/kotlin/com/bazel_diff/server/CachePruner.kt new file mode 100644 index 00000000..e2ca454c --- /dev/null +++ b/cli/src/main/kotlin/com/bazel_diff/server/CachePruner.kt @@ -0,0 +1,72 @@ +package com.bazel_diff.server + +import com.bazel_diff.log.Logger +import java.time.Duration +import java.util.concurrent.Executors +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit +import org.koin.core.component.KoinComponent +import org.koin.core.component.inject + +/** + * Periodically evicts entries from a [PrunableHashCacheStorage] to keep a long-running server's + * cache within [limits], preventing the per-commit-SHA cache from growing without bound. + * + * Sweeping runs on a single daemon thread so a clean JVM shutdown is never blocked, and uses a + * fixed *delay* (not rate): the next sweep starts [interval] after the previous one finishes, so a + * slow sweep over a large cache directory can never let runs pile up. The first sweep runs + * immediately on [start] so a freshly (re)started server reclaims disk at once rather than after a + * full interval. + * + * A no-op when [limits] sets no bounds ([CachePruneLimits.hasAny] is false). + */ +class CachePruner( + private val storage: PrunableHashCacheStorage, + private val limits: CachePruneLimits, + private val interval: Duration, +) : KoinComponent { + private val logger: Logger by inject() + + private val executor: ScheduledExecutorService = + Executors.newSingleThreadScheduledExecutor { runnable -> + Thread(runnable, "bazel-diff-cache-pruner").apply { isDaemon = true } + } + + /** + * Schedules the recurring sweep (and an immediate first pass). Idempotent limits-wise: a no-op + * when no bound is configured. + */ + fun start() { + if (!limits.hasAny) { + logger.i { "cache pruning disabled: no --cacheMax* limit set" } + return + } + val periodSeconds = interval.seconds.coerceAtLeast(1) + executor.scheduleWithFixedDelay(::runOnce, 0, periodSeconds, TimeUnit.SECONDS) + logger.i { "cache pruner active (limits=$limits, sweeping every ${periodSeconds}s)" } + } + + /** Stops the background sweeper. Safe to call more than once. */ + fun stop() { + executor.shutdownNow() + } + + /** + * Runs one prune pass. Public so tests can drive a sweep deterministically. Never throws: a + * transient filesystem error is logged and the next scheduled sweep retries, so a single failure + * cannot kill the sweeper thread (which would silently disable pruning for the process lifetime). + */ + fun runOnce() { + try { + val result = storage.prune(limits) + if (result.evicted > 0) { + logger.i { + "cache prune evicted ${result.evicted} of ${result.scanned} entries " + + "(${result.bytesFreed} bytes freed)" + } + } + } catch (e: Exception) { + logger.e(e) { "cache prune pass failed; retrying next interval" } + } + } +} diff --git a/cli/src/main/kotlin/com/bazel_diff/server/HashCacheStorage.kt b/cli/src/main/kotlin/com/bazel_diff/server/HashCacheStorage.kt index d1b88114..f53ef92e 100644 --- a/cli/src/main/kotlin/com/bazel_diff/server/HashCacheStorage.kt +++ b/cli/src/main/kotlin/com/bazel_diff/server/HashCacheStorage.kt @@ -1,8 +1,12 @@ package com.bazel_diff.server +import java.io.IOException import java.nio.file.Files +import java.nio.file.NoSuchFileException import java.nio.file.Path import java.nio.file.StandardCopyOption +import java.nio.file.attribute.FileTime +import java.time.Duration /** * Persistent store for generated hash JSON, keyed by an opaque cache key (a git SHA combined with a @@ -24,11 +28,54 @@ interface HashCacheStorage { fun contains(key: String): Boolean = get(key) != null } +/** + * Bounds a [PrunableHashCacheStorage] may be pruned down to. Each dimension is independent; a null + * field means "no limit on that dimension", and a value of all-null disables pruning entirely (see + * [hasAny]). Combining limits is allowed -- e.g. `maxAge` to expire stale revisions plus `maxBytes` + * as a hard ceiling on disk use. + * + * @param maxAge evict entries not used (read or written) within this window. + * @param maxEntries keep at most this many entries, evicting the least-recently-used first. + * @param maxBytes keep the total stored size at or below this many bytes, evicting the + * least-recently-used first. + */ +data class CachePruneLimits( + val maxAge: Duration? = null, + val maxEntries: Int? = null, + val maxBytes: Long? = null, +) { + /** True when at least one dimension is limited; pruning is a no-op otherwise. */ + val hasAny: Boolean + get() = maxAge != null || maxEntries != null || maxBytes != null +} + +/** What a single [PrunableHashCacheStorage.prune] pass did, for logging. */ +data class CachePruneResult(val scanned: Int, val evicted: Int, val bytesFreed: Long) + +/** + * A [HashCacheStorage] that can evict its own entries to stay within [CachePruneLimits], bounding + * the footprint of a long-running server. Backends whose retention is managed externally (e.g. an + * S3 bucket lifecycle policy) simply do not implement this interface, and the caller + * ([CachePruner]) leaves them alone. + */ +interface PrunableHashCacheStorage : HashCacheStorage { + /** + * Evicts entries violating [limits] and returns what was removed. Never throws for a single + * unreadable/undeletable entry -- pruning is best-effort. + */ + fun prune(limits: CachePruneLimits): CachePruneResult +} + /** * [HashCacheStorage] that stores each entry as a file `.json` under [directory]. The directory * is created if it does not exist. + * + * Recency is tracked via each file's last-modified time: [put] sets it (atomic move), and [get] + * bumps it on a cache hit, so [prune] can evict by genuine least-recent *use* (LRU) rather than by + * age since generation -- a base revision queried every CI run stays warm and is not expired out + * from under active traffic. */ -class LocalDiskHashCacheStorage(private val directory: Path) : HashCacheStorage { +class LocalDiskHashCacheStorage(private val directory: Path) : PrunableHashCacheStorage { init { Files.createDirectories(directory) } @@ -37,12 +84,22 @@ class LocalDiskHashCacheStorage(private val directory: Path) : HashCacheStorage override fun get(key: String): ByteArray? { val path = pathFor(key) - return if (Files.isRegularFile(path)) Files.readAllBytes(path) else null + if (!Files.isRegularFile(path)) return null + return try { + val data = Files.readAllBytes(path) + // Mark the entry as freshly used so the LRU-based pruner keeps hot revisions. + touchQuietly(path) + data + } catch (e: NoSuchFileException) { + // Pruned (or otherwise removed) between the existence check and the read: treat as a miss. + null + } } override fun put(key: String, data: ByteArray) { // Write to a temp file in the same directory then atomically move it into place, so a crash // mid-write never leaves a truncated entry that a later read would treat as a valid cache hit. + // The `.tmp` suffix also keeps in-progress writes out of prune()'s `*.json` scan. val target = pathFor(key) val tmp = Files.createTempFile(directory, "$key-", ".tmp") try { @@ -54,4 +111,75 @@ class LocalDiskHashCacheStorage(private val directory: Path) : HashCacheStorage } override fun contains(key: String): Boolean = Files.isRegularFile(pathFor(key)) + + override fun prune(limits: CachePruneLimits): CachePruneResult { + if (!limits.hasAny) return CachePruneResult(0, 0, 0) + + // Snapshot the entries oldest-first. Files that vanish mid-scan (a concurrent prune elsewhere, + // or an OS cleanup) are skipped rather than failing the whole pass. + val entries = listEntries().sortedBy { it.lastUsedMillis } + val evict = LinkedHashSet() + + // Age: expire anything older than the cutoff regardless of the count/size budgets. + limits.maxAge?.let { age -> + val cutoff = System.currentTimeMillis() - age.toMillis() + entries.forEach { if (it.lastUsedMillis < cutoff) evict += it } + } + + // Survivors (still oldest-first) feed the count and size caps, which drop from the oldest end. + val survivors = entries.filterNot { it in evict }.toMutableList() + + limits.maxEntries?.let { max -> + while (survivors.size > max.coerceAtLeast(0)) evict += survivors.removeAt(0) + } + + limits.maxBytes?.let { max -> + var total = survivors.sumOf { it.size } + while (total > max.coerceAtLeast(0) && survivors.isNotEmpty()) { + val oldest = survivors.removeAt(0) + evict += oldest + total -= oldest.size + } + } + + var evicted = 0 + var bytesFreed = 0L + for (entry in evict) { + try { + if (Files.deleteIfExists(entry.path)) { + evicted++ + bytesFreed += entry.size + } + } catch (e: IOException) { + // Best-effort: an entry we cannot delete (e.g. a transient lock) is retried next sweep. + } + } + return CachePruneResult(entries.size, evicted, bytesFreed) + } + + /** A cache file plus the metadata prune() sorts and budgets on. */ + private class Entry(val path: Path, val size: Long, val lastUsedMillis: Long) + + private fun listEntries(): List { + if (!Files.isDirectory(directory)) return emptyList() + return Files.newDirectoryStream(directory, "*.json").use { stream -> + stream.mapNotNull { path -> + try { + if (Files.isRegularFile(path)) { + Entry(path, Files.size(path), Files.getLastModifiedTime(path).toMillis()) + } else null + } catch (e: IOException) { + null // vanished or unreadable mid-scan + } + } + } + } + + private fun touchQuietly(path: Path) { + try { + Files.setLastModifiedTime(path, FileTime.fromMillis(System.currentTimeMillis())) + } catch (e: IOException) { + // The mtime bump is only LRU bookkeeping; failing it must never fail the read. + } + } } diff --git a/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt b/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt index cebd4a02..2d85af93 100644 --- a/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/cli/ServeCommandTest.kt @@ -3,18 +3,23 @@ package com.bazel_diff.cli import assertk.assertThat import assertk.assertions.hasLength import assertk.assertions.isEqualTo +import assertk.assertions.isFalse import assertk.assertions.isInstanceOf import assertk.assertions.isNotEqualTo +import assertk.assertions.isNotNull +import assertk.assertions.isNull import com.bazel_diff.SilentLogger import com.bazel_diff.log.Logger import com.bazel_diff.server.GitClient import com.bazel_diff.server.GitClientException import com.bazel_diff.server.HashCacheStorage import com.bazel_diff.server.JGitClient +import com.bazel_diff.server.LocalDiskHashCacheStorage import com.bazel_diff.server.ProcessGitClient import com.google.gson.GsonBuilder import java.net.HttpURLConnection import java.net.URL +import java.time.Duration import org.junit.After import org.junit.Rule import org.junit.Test @@ -81,11 +86,8 @@ class ServeCommandTest : KoinTest { } private object NoopImpactedTargets : com.bazel_diff.server.ImpactedTargetsProvider { - override fun getImpactedTargets( - fromRev: String, - toRev: String, - targetTypes: Set? - ) = com.bazel_diff.server.ImpactedTargetsResult(fromRev, toRev, emptyList()) + override fun getImpactedTargets(fromRev: String, toRev: String, targetTypes: Set?) = + com.bazel_diff.server.ImpactedTargetsResult(fromRev, toRev, emptyList()) override fun getImpactedTargetsWithDistances( fromRev: String, @@ -241,7 +243,8 @@ class ServeCommandTest : KoinTest { val provider = RecordingHashProvider(failFor = setOf("bogus")) val ready = java.util.concurrent.atomic.AtomicBoolean(false) val server = - com.bazel_diff.server.BazelDiffServer(0, NoopImpactedTargets) { ready.get() } + com.bazel_diff.server + .BazelDiffServer(0, NoopImpactedTargets) { ready.get() } .also { startedServers += it } server.start() @@ -251,6 +254,66 @@ class ServeCommandTest : KoinTest { assertThat(healthCode(server)).isEqualTo(200) } + @Test + fun parsesCachePruningFlagsIncludingTheIntervalDefault() { + val cmd = ServeCommand() + picocli + .CommandLine(cmd) + .parseArgs( + "--workspacePath", + "/tmp/ws", + "--cacheDir", + "/tmp/cache", + "--cacheMaxAge", + "7d", + "--cacheMaxSize", + "10gb", + "--cacheMaxEntries", + "500") + + assertThat(cmd.cacheMaxAge).isEqualTo(Duration.ofDays(7)) + assertThat(cmd.cacheMaxSize).isEqualTo(10L * 1024 * 1024 * 1024) + assertThat(cmd.cacheMaxEntries).isEqualTo(500) + // The default is a duration string parsed through the same converter. + assertThat(cmd.cachePruneInterval).isEqualTo(Duration.ofHours(1)) + } + + @Test + fun cachePruneLimitsReflectsFlags() { + val cmd = ServeCommand() + assertThat(cmd.cachePruneLimits().hasAny).isFalse() + + cmd.cacheMaxAge = Duration.ofDays(7) + cmd.cacheMaxEntries = 100 + cmd.cacheMaxSize = 1024L + val limits = cmd.cachePruneLimits() + + assertThat(limits.maxAge).isEqualTo(Duration.ofDays(7)) + assertThat(limits.maxEntries).isEqualTo(100) + assertThat(limits.maxBytes).isEqualTo(1024L) + } + + @Test + fun buildCachePrunerIsNullWhenNoLimitsAreSet() { + assertThat(command().buildCachePruner(InMemoryStorage())).isNull() + } + + @Test + fun buildCachePrunerIsNullWhenTheBackendCannotPrune() { + // A limit is set, but InMemoryStorage is not a PrunableHashCacheStorage: ignored (with a + // warning) + // rather than silently pretending the cache is bounded. + val cmd = command().apply { cacheMaxEntries = 10 } + assertThat(cmd.buildCachePruner(InMemoryStorage())).isNull() + } + + @Test + fun buildCachePrunerIsBuiltForAPrunableBackendWithLimits() { + val cmd = command().apply { cacheMaxEntries = 10 } + val storage = LocalDiskHashCacheStorage(temp.newFolder().toPath()) + assertThat(cmd.buildCachePruner(storage)).isNotNull() + } + @Test fun buildAndStartServerLameDucksOnFetchFailure() { val cmd = command() diff --git a/cli/src/test/kotlin/com/bazel_diff/cli/converter/ByteSizeConverterTest.kt b/cli/src/test/kotlin/com/bazel_diff/cli/converter/ByteSizeConverterTest.kt new file mode 100644 index 00000000..1e1ac47d --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/cli/converter/ByteSizeConverterTest.kt @@ -0,0 +1,36 @@ +package com.bazel_diff.cli.converter + +import assertk.assertThat +import assertk.assertions.isEqualTo +import org.junit.Assert.assertThrows +import org.junit.Test +import picocli.CommandLine.TypeConversionException + +class ByteSizeConverterTest { + private val converter = ByteSizeConverter() + + @Test + fun parsesUnitsBase1024() { + assertThat(converter.convert("1024")).isEqualTo(1024L) + assertThat(converter.convert("100b")).isEqualTo(100L) + assertThat(converter.convert("1k")).isEqualTo(1024L) + assertThat(converter.convert("1kb")).isEqualTo(1024L) + assertThat(converter.convert("500mb")).isEqualTo(500L * 1024 * 1024) + assertThat(converter.convert("2g")).isEqualTo(2L * 1024 * 1024 * 1024) + assertThat(converter.convert("10GB")).isEqualTo(10L * 1024 * 1024 * 1024) + assertThat(converter.convert("1tb")).isEqualTo(1024L * 1024 * 1024 * 1024) + } + + @Test + fun toleratesWhitespaceAndCase() { + assertThat(converter.convert(" 8 MB ")).isEqualTo(8L * 1024 * 1024) + } + + @Test + fun rejectsGarbage() { + assertThrows(TypeConversionException::class.java) { converter.convert("") } + assertThrows(TypeConversionException::class.java) { converter.convert("big") } + assertThrows(TypeConversionException::class.java) { converter.convert("10zz") } + assertThrows(TypeConversionException::class.java) { converter.convert("-5mb") } + } +} diff --git a/cli/src/test/kotlin/com/bazel_diff/cli/converter/DurationConverterTest.kt b/cli/src/test/kotlin/com/bazel_diff/cli/converter/DurationConverterTest.kt new file mode 100644 index 00000000..03a063a8 --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/cli/converter/DurationConverterTest.kt @@ -0,0 +1,38 @@ +package com.bazel_diff.cli.converter + +import assertk.assertThat +import assertk.assertions.isEqualTo +import java.time.Duration +import org.junit.Assert.assertThrows +import org.junit.Test +import picocli.CommandLine.TypeConversionException + +class DurationConverterTest { + private val converter = DurationConverter() + + @Test + fun parsesSingleUnits() { + assertThat(converter.convert("45s")).isEqualTo(Duration.ofSeconds(45)) + assertThat(converter.convert("90m")).isEqualTo(Duration.ofMinutes(90)) + assertThat(converter.convert("36h")).isEqualTo(Duration.ofHours(36)) + assertThat(converter.convert("7d")).isEqualTo(Duration.ofDays(7)) + } + + @Test + fun parsesCombinationsAndTrimsAndIsCaseInsensitive() { + assertThat(converter.convert("1d12h30m")) + .isEqualTo(Duration.ofDays(1).plusHours(12).plusMinutes(30)) + assertThat(converter.convert(" 2H ")).isEqualTo(Duration.ofHours(2)) + } + + @Test + fun rejectsBareNumbersAndGarbage() { + // A bare number has no unit, so it is ambiguous and rejected rather than silently assumed. + assertThrows(TypeConversionException::class.java) { converter.convert("7") } + assertThrows(TypeConversionException::class.java) { converter.convert("") } + assertThrows(TypeConversionException::class.java) { converter.convert("7x") } + assertThrows(TypeConversionException::class.java) { converter.convert("1d 2h") } + // Trailing digits with no unit must not be silently dropped. + assertThrows(TypeConversionException::class.java) { converter.convert("1d2") } + } +} diff --git a/cli/src/test/kotlin/com/bazel_diff/server/CachePrunerTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/CachePrunerTest.kt new file mode 100644 index 00000000..ad37d048 --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/server/CachePrunerTest.kt @@ -0,0 +1,105 @@ +package com.bazel_diff.server + +import assertk.assertThat +import assertk.assertions.isEqualTo +import assertk.assertions.isTrue +import com.bazel_diff.SilentLogger +import com.bazel_diff.log.Logger +import java.nio.file.Files +import java.time.Duration +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.koin.dsl.module +import org.koin.test.KoinTest +import org.koin.test.KoinTestRule + +class CachePrunerTest : KoinTest { + @get:Rule + val koinTestRule = KoinTestRule.create { modules(module { single { SilentLogger } }) } + + @get:Rule val temp: TemporaryFolder = TemporaryFolder() + + /** Records prune calls; optionally throws to exercise the sweeper's error handling. */ + private class FakeStorage( + private val result: CachePruneResult = CachePruneResult(0, 0, 0), + private val throwOnPrune: Boolean = false, + ) : PrunableHashCacheStorage { + val pruneCount = AtomicInteger(0) + val firstPrune = CountDownLatch(1) + @Volatile var lastLimits: CachePruneLimits? = null + + override fun get(key: String): ByteArray? = null + + override fun put(key: String, data: ByteArray) = Unit + + override fun prune(limits: CachePruneLimits): CachePruneResult { + lastLimits = limits + pruneCount.incrementAndGet() + firstPrune.countDown() + if (throwOnPrune) throw RuntimeException("boom") + return result + } + } + + @Test + fun startWithNoLimitsNeverSchedulesASweep() { + val storage = FakeStorage() + val pruner = CachePruner(storage, CachePruneLimits(), Duration.ofSeconds(1)) + // With no bound configured, start() returns before scheduling anything, so this is + // deterministic. + pruner.start() + pruner.stop() + assertThat(storage.pruneCount.get()).isEqualTo(0) + } + + @Test + fun startSchedulesAnImmediateSweepWithTheConfiguredLimits() { + val storage = FakeStorage() + val limits = CachePruneLimits(maxEntries = 5) + val pruner = CachePruner(storage, limits, Duration.ofHours(1)) + try { + pruner.start() + assertThat(storage.firstPrune.await(5, TimeUnit.SECONDS)).isTrue() + assertThat(storage.lastLimits).isEqualTo(limits) + } finally { + pruner.stop() + } + } + + @Test + fun runOnceSwallowsStorageErrorsSoTheSweeperThreadSurvives() { + val storage = FakeStorage(throwOnPrune = true) + val pruner = CachePruner(storage, CachePruneLimits(maxEntries = 1), Duration.ofHours(1)) + // Driving a pass directly must not propagate the storage failure. + pruner.runOnce() + assertThat(storage.pruneCount.get()).isEqualTo(1) + pruner.stop() + } + + @Test + fun endToEndTheScheduledSweeperEvictsRealFilesDownToTheLimit() { + // Wire the real scheduler to the real on-disk storage: writing more entries than the cap and + // starting the pruner must actually delete files from the cache directory. + val dir = temp.newFolder().toPath() + val storage = LocalDiskHashCacheStorage(dir) + repeat(5) { i -> storage.put("k$i", ByteArray(10)) } + val pruner = CachePruner(storage, CachePruneLimits(maxEntries = 2), Duration.ofSeconds(1)) + try { + pruner.start() + val deadline = System.currentTimeMillis() + 5_000 + while (jsonCount(dir) > 2 && System.currentTimeMillis() < deadline) { + Thread.sleep(25) + } + assertThat(jsonCount(dir)).isEqualTo(2) + } finally { + pruner.stop() + } + } + + private fun jsonCount(dir: java.nio.file.Path): Int = + Files.newDirectoryStream(dir, "*.json").use { it.count() } +} diff --git a/cli/src/test/kotlin/com/bazel_diff/server/LocalDiskHashCacheStorageTest.kt b/cli/src/test/kotlin/com/bazel_diff/server/LocalDiskHashCacheStorageTest.kt index 40351510..3f1acd0d 100644 --- a/cli/src/test/kotlin/com/bazel_diff/server/LocalDiskHashCacheStorageTest.kt +++ b/cli/src/test/kotlin/com/bazel_diff/server/LocalDiskHashCacheStorageTest.kt @@ -3,10 +3,13 @@ package com.bazel_diff.server import assertk.assertThat import assertk.assertions.isEqualTo import assertk.assertions.isFalse +import assertk.assertions.isNotNull import assertk.assertions.isNull import assertk.assertions.isTrue import java.nio.charset.StandardCharsets import java.nio.file.Files +import java.nio.file.attribute.FileTime +import java.time.Duration import org.junit.Rule import org.junit.Test import org.junit.rules.TemporaryFolder @@ -16,6 +19,15 @@ class LocalDiskHashCacheStorageTest { private fun storage() = LocalDiskHashCacheStorage(temp.root.toPath()) + private fun bytes(s: String) = s.toByteArray(StandardCharsets.UTF_8) + + /** Backdates an entry's last-used time so age/LRU ordering is deterministic under test. */ + private fun setAgeMinutes(key: String, minutes: Long) { + val path = temp.root.toPath().resolve("$key.json") + Files.setLastModifiedTime( + path, FileTime.fromMillis(System.currentTimeMillis() - minutes * 60_000)) + } + @Test fun missReturnsNull() { val storage = storage() @@ -55,4 +67,99 @@ class LocalDiskHashCacheStorageTest { assertThat(String(LocalDiskHashCacheStorage(dir).get("k")!!, StandardCharsets.UTF_8)) .isEqualTo("v") } + + @Test + fun pruneWithNoLimitsRemovesNothing() { + val storage = storage() + storage.put("a", bytes("a")) + val result = storage.prune(CachePruneLimits()) + assertThat(result.evicted).isEqualTo(0) + assertThat(storage.contains("a")).isTrue() + } + + @Test + fun pruneByMaxAgeEvictsEntriesNotUsedWithinTheWindow() { + val storage = storage() + storage.put("old", bytes("x")) + storage.put("fresh", bytes("y")) + setAgeMinutes("old", 120) + setAgeMinutes("fresh", 5) + + val result = storage.prune(CachePruneLimits(maxAge = Duration.ofHours(1))) + + assertThat(result.scanned).isEqualTo(2) + assertThat(result.evicted).isEqualTo(1) + assertThat(storage.contains("old")).isFalse() + assertThat(storage.contains("fresh")).isTrue() + } + + @Test + fun pruneByMaxEntriesKeepsTheMostRecentlyUsed() { + val storage = storage() + storage.put("a", bytes("a")) + storage.put("b", bytes("b")) + storage.put("c", bytes("c")) + setAgeMinutes("a", 30) // oldest + setAgeMinutes("b", 20) + setAgeMinutes("c", 10) // newest + + val result = storage.prune(CachePruneLimits(maxEntries = 2)) + + assertThat(result.evicted).isEqualTo(1) + assertThat(storage.contains("a")).isFalse() + assertThat(storage.contains("b")).isTrue() + assertThat(storage.contains("c")).isTrue() + } + + @Test + fun pruneByMaxSizeEvictsOldestUntilUnderBudget() { + val storage = storage() + // Each entry is exactly 100 bytes; a 250-byte budget keeps the two newest. + storage.put("a", ByteArray(100)) + storage.put("b", ByteArray(100)) + storage.put("c", ByteArray(100)) + setAgeMinutes("a", 30) + setAgeMinutes("b", 20) + setAgeMinutes("c", 10) + + val result = storage.prune(CachePruneLimits(maxBytes = 250)) + + assertThat(result.evicted).isEqualTo(1) + assertThat(result.bytesFreed).isEqualTo(100) + assertThat(storage.contains("a")).isFalse() + assertThat(storage.contains("b")).isTrue() + assertThat(storage.contains("c")).isTrue() + } + + @Test + fun getBumpsRecencySoActivelyUsedEntriesSurvivePruning() { + val storage = storage() + storage.put("a", bytes("a")) + storage.put("b", bytes("b")) + setAgeMinutes("a", 30) + setAgeMinutes("b", 20) + + // A cache hit on the older entry makes it the most-recently-used, so the LRU count cap now + // evicts "b" instead of "a". + assertThat(storage.get("a")).isNotNull() + storage.prune(CachePruneLimits(maxEntries = 1)) + + assertThat(storage.contains("a")).isTrue() + assertThat(storage.contains("b")).isFalse() + } + + @Test + fun pruneOnlyConsidersJsonEntriesAndLeavesOtherFilesAlone() { + val storage = storage() + storage.put("stale", bytes("x")) + setAgeMinutes("stale", 120) + val sibling = temp.root.toPath().resolve("README.txt") + Files.write(sibling, bytes("not a cache entry")) + + val result = storage.prune(CachePruneLimits(maxAge = Duration.ofHours(1))) + + assertThat(result.scanned).isEqualTo(1) // the non-.json file is not counted + assertThat(result.evicted).isEqualTo(1) + assertThat(Files.exists(sibling)).isTrue() + } } diff --git a/tools/readme_template.md b/tools/readme_template.md index c5a6d186..5897a5a2 100644 --- a/tools/readme_template.md +++ b/tools/readme_template.md @@ -193,9 +193,17 @@ Notes and current limitations: warning; point `--gitEngine=subprocess` at such workspaces to skip the in-process attempt (a partial clone in particular needs the subprocess engine to *serve* queries, since JGit cannot lazily fetch the missing blobs a checkout needs). -* Hashes are cached on local disk via `--cacheDir` and survive restarts. The cache layer is - pluggable behind a byte-oriented interface so a remote backend (e.g. S3) can be added without - touching callers. +* Hashes are cached on local disk via `--cacheDir` and survive restarts. Left unbounded the cache + grows by one entry per distinct commit SHA queried, so a long-running server can bound it with any + combination of `--cacheMaxAge` (expire entries not read or written within a window, e.g. `7d`), + `--cacheMaxEntries`, and `--cacheMaxSize` (e.g. `10GB`, `500MB`, or a bare byte count). A background + sweeper enforces the limits once at startup and then every `--cachePruneInterval` (default `1h`), + evicting least-recently-used entries first — a cache hit refreshes an entry's recency, so revisions + under active query are not expired out from under live traffic. With no `--cacheMax*` flag set the + cache is never pruned (the previous behavior). The cache layer is pluggable behind a byte-oriented + interface so a remote backend (e.g. S3) can be added without touching callers; such a backend + manages its own retention (e.g. a bucket lifecycle policy), and the in-process `--cacheMax*` flags + do not apply to it. * Query-affecting flags (`--useCquery`, `--fineGrainedHashExternalRepos`, etc.) mirror `generate-hashes`, and are folded into the cache key so a server started with different flags never serves another configuration's cached hashes.