From 99270a0f3994b0cd01b9af3d1438d72476725f1e Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Sat, 8 Aug 2026 20:15:08 +0200 Subject: [PATCH 1/3] docs: record the design for a Kubernetes-ready, highly available Ingot Running two instances against one database and one bucket is unsafe today, for reasons that are specific rather than architectural: schema DDL runs on every pod start, metadata writes are read-modify-write without a lock that crosses processes, and there is no readiness signal separate from liveness. The document records those findings, the fixes, and three decisions that constrain them. Locks go into the database because a lost lock is a lost update and Galera already provides the guarantee. NATS covers fan-out only, and Redis gets no niche. The eventual split runs through deployment targets in one image rather than separate services, so the drop-in contract survives. Four stages, of which only the first two are required for high availability. Each gets its own implementation plan; this is the shared context. --- .../specs/2026-08-08-kubernetes-ha-design.md | 440 ++++++++++++++++++ 1 file changed, 440 insertions(+) create mode 100644 docs/superpowers/specs/2026-08-08-kubernetes-ha-design.md diff --git a/docs/superpowers/specs/2026-08-08-kubernetes-ha-design.md b/docs/superpowers/specs/2026-08-08-kubernetes-ha-design.md new file mode 100644 index 000000000..e0c896a16 --- /dev/null +++ b/docs/superpowers/specs/2026-08-08-kubernetes-ha-design.md @@ -0,0 +1,440 @@ +# Kubernetes-ready Ingot: high availability, configuration distribution and cloud-native structure + +Status: design, approved for staged implementation +Date: 2026-08-08 + +## Why this document exists + +Ingot runs well as a single instance. Running two of them against the same database and the +same object store is not safe today, for reasons that are specific and fixable rather than +architectural. This document records what those reasons are, what has to change, and in +which order. + +It also answers three questions that came up alongside high availability: how configuration +reaches a pod, how the dashboard and the server scale independently, and where a message bus +belongs. The answers turned out to be connected, so they live in one document. + +The scope covers four stages. Only the first two are required for high availability. Stages +three and four are recorded here because they change decisions made earlier, and because +leaving them out would make the earlier stages look arbitrary. Each stage gets its own +implementation plan; this document is the shared context, not a single work item. + +## Architecture decisions + +Three decisions constrain everything below. + +### Locks belong in the database + +Schema migration and metadata writes are correctness problems. A lost lock means a lost +update, not a slow request. + +MariaDB Galera is already part of the deployment. `GET_LOCK()` and `SELECT ... FOR UPDATE` +are transactional, consensus-backed, and release when the connection dies, which covers a +pod that is killed rather than stopped. + +Redis locks were considered and rejected. Without a fencing token, an expired lock does not +protect against a paused process that resumes and writes anyway, which is the exact failure +this needs to prevent. NATS JetStream KV offers revision-based compare-and-swap and would +work, but it puts a new consensus component in the write path for a guarantee the database +already provides. + +### NATS covers fan-out, and nothing else + +Four places need the same thing: tell every other instance that something changed. Cache +invalidation, configuration propagation, token revocation and console log streaming are all +best-effort. Losing an event means one instance holds a cache entry slightly longer than it +should. + +Core NATS publish and subscribe is enough for all of them. JetStream is needed only where an +event must survive, which is statistics ingestion once it crosses a process boundary. + +Redis has no remaining niche. It could take the brute-force counter, but so can the database, +and introducing a component for one counter is not a trade worth making. If Redis is already +operated for other reasons, that counter is a reasonable thing to move there later. + +### The split runs through deployment targets, not repositories + +Ingot is already a modular monolith. All fourteen core building blocks are plugins with +declared dependencies, sorted topologically by `PluginLoader.sortPlugins()`. What is missing +is a switch deciding which plugins a given process loads. + +The model is the one Grafana Loki and Mimir use: one artifact, one image, one `--target` +flag. `--target=all` stays the default and reproduces today's behaviour exactly, which is +what keeps the drop-in contract intact. + +Separate images per domain were considered and rejected. The hot path is streaming bytes +from object storage to a client, and every additional hop costs latency without buying +anything. The domain has four aggregates, not forty. Harbor took the other road and is +widely considered hard to operate. + +## What already works + +Worth stating, because it shapes how much is left: + +- **Database backends.** `DatabaseConnectionFactory` supports MariaDB, MySQL and PostgreSQL + through HikariCP. Tokens, statistics and shared configuration already live there. SQLite is + only the default. +- **Configuration propagation.** `RemoteSharedConfigurationProvider` stores in the database, + and `SharedConfigurationPlugin` polls for changes every ten seconds. The log message reads + "Propagation ... updating current instance", so this was written with more than one + instance in mind. +- **Statistics aggregation.** `SqlStatisticsRepository.incrementResolvedRequests` is an + upsert with `count + count`. Several instances add up correctly. +- **Storage abstraction.** `StorageProvider` is configurable per repository, and the S3 + implementation exists. Shared storage without a ReadWriteMany volume is already possible. +- **Stateless authentication.** Basic auth against tokens in the database, no server-side + session store. Artifact traffic needs no sticky sessions. +- **Environment configuration.** `INGOT_LOCAL_*` and the legacy `REPOSILITE_LOCAL_*` prefixes + are read by `LocalConfigurationFactory`, so a ConfigMap can replace the configuration file. +- **Dashboard split.** A second image and `INGOT_LOCAL_DEFAULTFRONTEND=false` already allow + separate deployments. + +## Stage 1: correctness + +Without these three changes, a second replica is unsafe. None of them require a new +component. + +### 1.1 Serialise schema initialisation + +`SqlAccessTokenRepository`, `SqlStatisticsRepository` and `SqlConfigurationRepository` each +call `SchemaUtils.create(...)` or `createMissingTablesAndColumns(...)` during +initialisation. With several pods starting at once, which is what a rolling update and an +initial `replicas: 3` both produce, they issue DDL against the same tables concurrently. + +On Galera, DDL runs under total order isolation. Concurrent `CREATE TABLE IF NOT EXISTS` and +`ALTER TABLE` statements stall the cluster at best and abort a node at worst. + +The fix is an advisory lock around the whole initialisation sequence: `GET_LOCK()` on +MariaDB and MySQL, `pg_advisory_lock` on PostgreSQL, a no-op on SQLite and H2 where a single +writer is the only supported mode anyway. The lock is acquired once at startup and released +before the server begins serving. + +### 1.2 Serialise metadata writes + +`MetadataService.generatePom` reads `maven-metadata.xml`, modifies it and writes it back. +`FileSystemStorageProvider` holds a `ReentrantReadWriteLock` per location, but that lock ends +at the process boundary, and the code says so: `TO-FIX: FS locks are not truly respected`. +The S3 provider has no lock at all. Two deployments of the same group and artifact from two +pods lose one version. + +`PreservedBuildsListener` has the same shape: it deletes snapshot files based on a timestamp +read from metadata another pod may be rewriting. + +The fix is a lock row keyed by repository and path, taken with `SELECT ... FOR UPDATE` for +the duration of the read-modify-write. This covers both storage providers, because the +serialisation happens in the database rather than in the storage layer. + +### 1.3 Flush statistics on shutdown + +`StatisticsFacade` buffers increments in memory and flushes every ten seconds from a +scheduled task. `Reposilite.shutdown()` stops the scheduler before anything else, and +`StatisticsPlugin` registers no dispose handler, so every rolling update discards up to ten +seconds of statistics per pod. + +The fix is a `ReposiliteDisposeEvent` listener calling `saveRecordsBulk()` once more. + +## Stage 2: Kubernetes + +After this stage, running several replicas is correct and behaves properly under rolling +updates, node drains and probe failures. + +### 2.1 Readiness separate from liveness + +`/api/status/health` reports `webServer.isAlive()`, which means Jetty is running. A pod whose +connection pool is exhausted or whose object store is unreachable still reports UP and keeps +receiving traffic. + +Add `/api/status/ready`, unauthenticated like the existing endpoint, reporting: + +- database reachable, via a pooled `SELECT 1` +- storage provider reachable, via a cheap existence check per configured provider +- shared configuration loaded +- not draining + +The last item is a flag set at the very start of the shutdown sequence, before anything is +torn down. It is what lets Kubernetes remove the pod from the service endpoints while it is +still serving in-flight requests. + +`/api/status/health` keeps its current meaning and becomes the liveness probe. + +### 2.2 Graceful shutdown + +Two gaps. `stopTimeout` is not set anywhere on the Jetty server, so a stop does not wait for +in-flight requests, and a large upload dies mid-stream. And the draining flag from 2.1 has to +be set before `webServer.stop()` is reached. + +The sequence becomes: set draining, wait out the endpoint removal delay, stop accepting new +connections, drain in-flight requests up to `stopTimeout`, then continue with today's teardown. + +On the manifest side this pairs with a `preStop` hook and a `terminationGracePeriodSeconds` +larger than `stopTimeout`. + +### 2.3 Local disk that Kubernetes has to know about + +Two paths need writable local storage that is not the data volume: + +- `S3StorageProvider.putFile` writes the entire upload to a temporary file before sending it, + because the S3 API requires a content length. A 500 MB artifact needs 500 MB of ephemeral + storage. Without an `emptyDir` for `/tmp` and an `ephemeral-storage` limit, the kubelet + evicts the pod during the upload. +- `JavadocContainerService` unpacks javadoc jars under the working directory, per pod, with + no eviction. This belongs in an `emptyDir` with a size limit, not on the data volume. + +### 2.4 Read path and write path as separate deployments + +This is the largest scaling gain in the whole document, and it needs no new code once 1.2 is +in place. + +Reads are the overwhelming majority of repository traffic, need no coordination and scale +linearly. Writes need the metadata locks. Routing `GET` and `HEAD` to one deployment and +`PUT`, `POST` and `DELETE` to another, from the same image, gives each an independent replica +count and autoscaler. The split lives entirely in the ingress rules. + +### 2.5 Chart and manifests + +There is no Ingot chart; `kubernetes.md` currently points at the Reposilite chart and says so. +What is needed: + +- two deployments, dashboard and server, with the probes from 2.1 +- ingress routing the four dashboard paths directly, see the configuration section below +- `emptyDir` volumes and `ephemeral-storage` limits from 2.3 +- a PodDisruptionBudget and topology spread constraints +- the read and write split from 2.4 as a documented option rather than a default + +### 2.6 Documentation corrections + +Three things the current guides get wrong for a clustered deployment: + +- `--local-configuration-mode=none` is required, see below +- S3 credentials belong in a Secret, not in the database, see below +- the first access token should come from `--token`, not from `kubectl attach`, which reaches + a random pod when there is more than one + +## Stage 3: distribution over NATS + +Optional. Nothing here is required for several replicas to be correct; it removes polling, +closes propagation delays and fixes the console. + +### 3.1 Two new events in the core + +The clustering logic belongs in a plugin, but two things it needs to observe have no event +today: + +- cache invalidation, currently a direct call into `ResolutionCache`, which is `internal` +- token revocation, currently a repository write with no notification + +Both become events on the existing extension mechanism. This is the only core change stage 3 +requires. + +### 3.2 The clustering plugin + +A plugin that does nothing when no NATS URL is configured, so the default deployment is +unchanged. With a URL, it mirrors four things across instances: + +| Concern | Today | With NATS | +|---|---|---| +| `ResolutionCache` invalidation | per instance, `cache-purge` reaches one pod | published, every instance invalidates | +| Shared configuration | ten second database poll | published on save, poll stays as fallback | +| Token revocation | up to sixty seconds of staleness from `authenticationCache` | published on revoke | +| Console log streaming | one pod's logs | every pod publishes, every session sees all | + +The console case is the most valuable. `ReposiliteJournalist.subscribe()` already implements +fan-out; it simply ends at the process boundary. Mirroring it over NATS means an +administrator sees the logs of every replica regardless of which pod the connection landed +on, which makes the console better in a cluster than it is standalone. + +### 3.3 The brute-force counter + +`AuthenticationFacade` holds failed login attempts in a process-local cache, so with three +replicas an attacker gets three times the allowance. Login frequency is orders of magnitude +below artifact reads, so a database table with an upsert is sufficient and avoids adding a +dependency. This is listed under stage 3 because it is a clustering concern, not because it +needs NATS. + +## Stage 4: deployment targets + +This is the cloud-native structure and the frame an enterprise edition would build on. + +### 4.1 The mechanism + +- a `targets` field on the `@Plugin` annotation +- a filter in `PluginLoader.sortPlugins()` +- a `--target` parameter defaulting to `all` + +Small in code. The weight is in deciding which targets exist and documenting them. + +### 4.2 Which cuts are worth making + +In order of value: + +**Statistics.** The cleanest boundary in the codebase. +`StatisticsFacade.incrementResolvedRequest` is called from exactly one place, +`RepositoryService`, with no return value and no error path. Over JetStream, the read path +publishes and a statistics process consumes and writes. The stream also replaces the +in-memory buffer that stage 1.3 patches. The `statistics` to `console` plugin dependency +exists only to register a command and is not a runtime coupling. + +**Javadoc.** Unpacking jars is CPU and disk bound, a completely different resource profile +from streaming bytes, with its own endpoints and its own scratch directory. + +**Authentication: deliberately not cut.** `access-token` is the only plugin with no +dependencies and is technically the best isolated, but `AccessTokenFacade` is consulted on +every request through `ContextDsl` and `ReposiliteRouting`. A network hop per artifact +download to check a token is the wrong trade. It stays embedded, with revocation propagated +per 3.1. + +### 4.3 The abstraction this rests on + +`RemoteClient` is already the interface for "fetch a file from somewhere else", with two +implementations: `HttpRemoteClient` for real upstream mirrors, and `RepositoryLoopbackClient` +for a local repository addressed as if it were remote. The second one proves the core already +tolerates a repository that is not local. A third implementation talking to another Ingot +process fits at the same seam without `RepositoryService` knowing. + +### 4.4 Relationship to an enterprise edition + +The plugin system is already the open-core mechanism. +`PluginLoader.loadPluginsByServiceFiles()` loads jars from the plugins directory through a +`URLClassLoader` and `ServiceLoader`. An enterprise plugin is a jar in that directory, using +the same API third-party plugins use. + +The design rule that follows: **every enterprise feature must be expressible as a plugin.** +If it cannot be, it belongs in the core and is free software. This keeps the boundary honest +and protects the Reposilite plugin compatibility promise. + +The licensing question is open and legal rather than technical. Apache 2.0 imposes no +copyleft, so proprietary plugins against the `com.reposilite.*` API are possible, and the +dzikoysk copyright headers are unaffected as long as enterprise code lives in new files. That +assessment is not a substitute for advice from someone qualified to give it. + +## Configuration distribution + +Three levels, each with a different path into a pod. + +| Level | Contains | Mechanism | Kubernetes | +|---|---|---|---| +| Parameters | working directory, token, migrations, configuration mode | argv | `args:` | +| Local configuration | port, thread pools, database URL, SSL, default frontend | file, overridable by environment | ConfigMap and Secret as environment variables | +| Shared configuration | repositories, mirrors, frontend settings, LDAP, statistics | database or file | database | + +### Three corrections + +**`--local-configuration-mode=none` is required.** `LocalConfigurationProvider.render()` +writes `configuration.cdn` back to the working directory whenever the mode is `AUTO`, which +is the default. Against a read-only ConfigMap mount this fails. The error is swallowed and +the server starts anyway, which is worse than failing: the deployment ends up with a +configuration source that tries to overwrite itself. + +**Database configuration and GitOps configuration are mutually exclusive.** Passing +`--shared-configuration-path` switches to `LocalSharedConfigurationProvider`, which reports +`isMutable() = false` and `isUpdateRequired() = false`. The dashboard can then display +settings but not save them, and changes require a rollout. Storing in the database keeps the +dashboard working and propagates within ten seconds, at the cost of configuration that does +not live in git. For a clustered deployment the database is the right choice, because the +propagation is already built. + +**S3 credentials do not have to live in the database.** +`S3StorageProviderFactory` installs a static credentials provider only when both the access +key and the secret key are non-empty. Leaving them blank falls through to the AWS default +credential chain: environment variables, web identity and IRSA, instance metadata. +`AWS_REGION` is already read. Clearing those two fields in the shared configuration moves the +credentials into a Kubernetes Secret, and rotation becomes a secret update rather than a +database write. No code change required. + +## Scaling the dashboard separately from the server + +The split already exists and is well documented. One thing does not carry over to Kubernetes. + +The dashboard container proxies everything that is not its own static content to the server, +including every artifact download and every CI upload. The reasoning in the guide is correct +for Compose: repository names are arbitrary top-level paths and cannot be distinguished from +dashboard routes by prefix, and a second origin would mean CORS on every repository read. + +On Kubernetes this inverts the stated purpose. Scaling the dashboard without touching the +traffic CI depends on does not work when all of that traffic passes through the dashboard. + +The exception list in the nginx template is finite, which is what makes the alternative +possible. It contains exactly four entries: + +``` +/assets/ prefix +/favicon.png exact +/index.html exact +/ exact +``` + +An ingress can express that directly, because ingress-nginx evaluates exact matches before +prefix matches: the four paths above route to the dashboard, and a catch-all prefix routes to +the server. One origin is preserved, so no CORS. Dashboard routing is hash based, so +navigation never reaches the server and the exact match on the document root is sufficient. +The proxy hop disappears entirely. + +The `INGOT_BACKEND` proxy stays in the image for Compose and for anyone without an ingress +that can express exact path rules. + +## Compatibility + +Ingot is a drop-in replacement for Reposilite, and that outranks everything in this document. +Every stage must still satisfy the checks in `CLAUDE.md`: + +1. An unmodified upstream Compose file starts with only the image reference changed. +2. An existing `/app/data` is adopted, whoever owns it. +3. `PUID`, `PGID` and `REPOSILITE_OPTS` still take effect. + +The "Container images" CI job answers all three by running the image, including an upgrade +rehearsal against the real upstream image. + +Specific commitments per stage: + +- **Stage 1** changes behaviour for everyone, since the advisory lock is taken on every + start. This is acceptable because an uncontended lock costs nothing measurable, and it is + the only stage without an opt-in. SQLite and H2 skip it entirely. +- **Stage 2** adds an endpoint and a shutdown delay. Existing probes keep working, because + `/api/status/health` keeps its meaning. +- **Stage 3** is inert without a NATS URL. +- **Stage 4** defaults to `--target=all`, which is today's process. + +New environment variables take the `INGOT_` prefix, with any `REPOSILITE_` spelling remaining +valid as a fallback. + +## Testing + +- **Stage 1** needs concurrency tests that the existing suite has no shape for: several + connections racing on schema initialisation, and concurrent deployments of the same group + and artifact asserting that no version is lost. These run against a real database, not + SQLite, since the behaviour under test is database-specific. +- **Stage 2** extends the container CI job: start two replicas against one database and one + bucket, confirm both become ready, roll one and confirm no request fails. +- **Stage 3** requires a NATS container in the integration environment and a test asserting + that an invalidation on one instance is observed by another. +- **Stage 4** asserts that each target starts, exposes the endpoints it should and does not + expose the ones it should not. + +## Deliberately not done + +Recording these so they read as decisions rather than oversights. + +**Distributed single-flight for mirror fetches.** `MirrorService.inFlightFetches` is per +process, so a cold start fetches an artifact once per pod instead of once. The effect is +brief and `putFile` is idempotent. The coordination is not worth its complexity. + +**Redis.** Every need it would cover is covered better by the database or by NATS. + +**A shared artifact byte cache.** With S3 as the backing store this would be a cache in front +of a cache. + +**Splitting authentication into its own service.** Explained in 4.2. + +**Microservices as separate images.** Explained in the architecture decisions. + +## Order of work + +Stages 1 and 2 deliver high availability and are required. Stage 3 is optional and additive. +Stage 4 is the structural work, and it comes last on purpose: every cut made before the +correctness problems are solved would be built on them. + +One caveat worth repeating. The largest throughput gain in this document is the read and +write split in 2.4, and it needs no target mechanism and no message bus. If the pressure is +performance rather than organisation, the work stops paying off after stage 2. Stages 3 and 4 +are worth doing for operability, for the console, and as the frame for an enterprise edition, +not for throughput. From 050b4fe59f60cabdaf06bca5e7513acf8fac0566 Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Sat, 8 Aug 2026 21:33:18 +0200 Subject: [PATCH 2/3] docs: add the implementation plan for HA stage 1 Six tasks covering the three correctness problems that block a second replica: a shared advisory lock built and proven against a real MariaDB, schema initialisation serialised around the plugin loader, metadata writes serialised per repository and directory, and the statistics buffer flushed on shutdown. Stage 2 gets its own plan. Stage 1 delivers working software on its own, and every cut made before the correctness problems are solved would be built on them. Two findings recorded in the self-review rather than silently fixed. The PreservedBuildsListener races the same metadata the lock now protects, but it runs after the lock is released, so covering it needs a decision about whether event listeners may block a deployment. And widening the Reposilite constructor breaks a third-party plugin that constructs it directly, which nothing in this repository does. --- .../2026-08-08-ha-stage-1-correctness.md | 1111 +++++++++++++++++ 1 file changed, 1111 insertions(+) create mode 100644 docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md diff --git a/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md b/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md new file mode 100644 index 000000000..e041716a3 --- /dev/null +++ b/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md @@ -0,0 +1,1111 @@ +# HA Stage 1: Correctness Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make it safe to run more than one Ingot instance against one database and one object store, by serialising schema initialisation and metadata writes across processes and by not losing buffered statistics on shutdown. + +**Architecture:** A single `DatabaseLock` abstraction takes a named advisory lock on its own pooled connection. On MariaDB and MySQL that is `GET_LOCK`, on PostgreSQL `pg_advisory_lock`, and on SQLite and H2 it is a no-op because those support one writer anyway. Two call sites use it: the whole plugin initialisation sequence in `ReposiliteFactory`, and the read-modify-write on `maven-metadata.xml` in `MetadataService`. A third, unrelated fix flushes the statistics buffer during shutdown. + +**Tech Stack:** Kotlin, Exposed (`org.jetbrains.exposed.v1`), HikariCP, JUnit 5, Testcontainers (MariaDB, PostgreSQL), Gradle. + +## Global Constraints + +- **Backwards compatibility outranks everything.** Moving from Reposilite to Ingot must still cost exactly one changed line. An unmodified upstream Compose file must start with only the image reference changed, an existing `/app/data` must be adopted whoever owns it, and `PUID`, `PGID` and `REPOSILITE_OPTS` must still take effect. +- **Package names stay `com.reposilite.*`.** Do not move anything to `net.onelitefeather.ingot.*`. +- **All code, comments and commit messages in English.** +- **No em dashes (`—`, U+2014) and no en dashes (`–`, U+2013)** anywhere in commits, code comments or documentation. Use a hyphen, a colon, a comma, or two sentences. A `PreToolUse` hook blocks commits that violate this. +- **No `Co-Authored-By:` trailers, no `Claude-Session:` links, no "Generated with Claude Code", no robot emoji** in any commit message. +- **Conventional Commits**, imperative mood, body wrapped at 72 characters. Allowed types: `feat`, `fix`, `docs`, `refactor`, `test`, `build`, `ci`, `chore`, `perf`. +- **New source files carry the Apache 2.0 header** in the form used by `reposilite-backend/src/main/kotlin/com/reposilite/maven/ResolutionCache.kt` (`Copyright (c) 2026 dzikoysk`). Do not remove or replace existing dzikoysk headers. +- **Build commands:** unit tests `./gradlew :reposilite-backend:test`, integration tests `./gradlew :reposilite-backend:integration`. The `integration` source set lives at `reposilite-backend/src/integration/kotlin`. +- **Branch:** work on `docs/kubernetes-ha-design` or a branch from it. The design this implements is `docs/superpowers/specs/2026-08-08-kubernetes-ha-design.md`. + +--- + +## File Structure + +**Created:** + +- `reposilite-backend/src/main/kotlin/com/reposilite/shared/DatabaseLock.kt` - the advisory lock abstraction and its vendor dispatch. One responsibility: acquire a named lock on a dedicated connection, run a block, release it. +- `reposilite-backend/src/test/kotlin/com/reposilite/shared/DatabaseLockTest.kt` - unit tests for the no-op path and the name-hashing helper. +- `reposilite-backend/src/integration/kotlin/com/reposilite/shared/DatabaseLockIntegrationTest.kt` - mutual exclusion against a real MariaDB and a real PostgreSQL. +- `reposilite-backend/src/integration/kotlin/com/reposilite/ConcurrentSchemaInitializationIntegrationTest.kt` - several initialisations racing against one database. +- `reposilite-backend/src/integration/kotlin/com/reposilite/maven/ConcurrentMetadataWriteIntegrationTest.kt` - parallel deployments of the same coordinate. + +**Modified:** + +- `reposilite-backend/src/main/kotlin/com/reposilite/Reposilite.kt` - hold the `DatabaseLock` so plugins can reach it. +- `reposilite-backend/src/main/kotlin/com/reposilite/ReposiliteFactory.kt` - build the lock, wrap `pluginLoader.initialize()`. +- `reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt` - take the lock around the read-modify-write in `generatePom`. +- `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenComponents.kt` - pass the lock into `MetadataService`. +- `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenPlugin.kt` - supply the lock from `reposilite()`. +- `reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt` - flush on dispose. + +**Why a dedicated connection:** `GET_LOCK` and `pg_advisory_lock` are session scoped, not transaction scoped. Taking the lock inside an Exposed `transaction { }` that also performs the guarded work would hold a transaction open across S3 I/O, which is exactly the long-running transaction Galera handles badly. Borrowing one connection, taking the lock, doing the work outside any transaction, then releasing and returning the connection avoids that. + +--- + +### Task 1: The DatabaseLock abstraction + +**Files:** +- Create: `reposilite-backend/src/main/kotlin/com/reposilite/shared/DatabaseLock.kt` +- Test: `reposilite-backend/src/test/kotlin/com/reposilite/shared/DatabaseLockTest.kt` + +**Interfaces:** +- Consumes: `javax.sql.DataSource` (the `HikariDataSource` already inside `DatabaseConnection`), `org.jetbrains.exposed.v1.jdbc.Database` for vendor detection. +- Produces: + - `class DatabaseLock(dataSource: DataSource, vendor: String, journalist: Journalist)` + - `fun DatabaseLock.withLock(name: String, timeoutSeconds: Int = 60, block: () -> T): T` + - `internal fun advisoryKeyOf(name: String): Long` (used by the PostgreSQL path and asserted in tests) + - `class DatabaseLockTimeoutException(name: String, timeoutSeconds: Int) : RuntimeException` + +- [ ] **Step 1: Write the failing test** + +Create `reposilite-backend/src/test/kotlin/com/reposilite/shared/DatabaseLockTest.kt`: + +```kotlin +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.reposilite.shared + +import com.reposilite.journalist.backend.InMemoryLogger +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertNotEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import javax.sql.DataSource +import java.sql.Connection + +internal class DatabaseLockTest { + + private val logger = InMemoryLogger() + + /** A DataSource that fails loudly if anyone asks it for a connection. */ + private val forbiddenDataSource = object : DataSource by NoopDataSource() { + override fun getConnection(): Connection = throw AssertionError("no connection expected") + } + + @Test + fun `should run the block without a connection on sqlite`() { + val lock = DatabaseLock(forbiddenDataSource, "sqlite", logger) + + val result = lock.withLock("schema-init") { "done" } + + assertEquals("done", result) + } + + @Test + fun `should run the block without a connection on h2`() { + val lock = DatabaseLock(forbiddenDataSource, "h2", logger) + + val result = lock.withLock("schema-init") { 42 } + + assertEquals(42, result) + } + + @Test + fun `should derive a stable advisory key from a lock name`() { + assertEquals(advisoryKeyOf("schema-init"), advisoryKeyOf("schema-init")) + } + + @Test + fun `should derive different advisory keys for different names`() { + assertNotEquals(advisoryKeyOf("metadata:releases:com/example"), advisoryKeyOf("metadata:releases:com/other")) + } + + @Test + fun `should propagate an exception thrown by the guarded block`() { + val lock = DatabaseLock(forbiddenDataSource, "sqlite", logger) + + val thrown = runCatching { lock.withLock("schema-init") { error("boom") } }.exceptionOrNull() + + assertTrue(thrown is IllegalStateException) + } +} +``` + +Add the minimal `NoopDataSource` stub in the same file, below the test class: + +```kotlin +private open class NoopDataSource : DataSource { + override fun getConnection(): Connection = throw UnsupportedOperationException() + override fun getConnection(username: String?, password: String?): Connection = throw UnsupportedOperationException() + override fun getLogWriter(): java.io.PrintWriter = throw UnsupportedOperationException() + override fun setLogWriter(out: java.io.PrintWriter?) = throw UnsupportedOperationException() + override fun setLoginTimeout(seconds: Int) = throw UnsupportedOperationException() + override fun getLoginTimeout(): Int = 0 + override fun getParentLogger(): java.util.logging.Logger = throw UnsupportedOperationException() + override fun unwrap(iface: Class?): T = throw UnsupportedOperationException() + override fun isWrapperFor(iface: Class<*>?): Boolean = false +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +Run: `./gradlew :reposilite-backend:test --tests "com.reposilite.shared.DatabaseLockTest"` + +Expected: FAIL to compile, with unresolved references `DatabaseLock` and `advisoryKeyOf`. + +- [ ] **Step 3: Write the implementation** + +Create `reposilite-backend/src/main/kotlin/com/reposilite/shared/DatabaseLock.kt`: + +```kotlin +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.reposilite.shared + +import com.reposilite.journalist.Journalist +import com.reposilite.journalist.Logger +import java.sql.Connection +import javax.sql.DataSource + +class DatabaseLockTimeoutException(name: String, timeoutSeconds: Int) : + RuntimeException("Could not acquire database lock '$name' within $timeoutSeconds second(s)") + +/** + * Derives a 64 bit key from a lock name, for backends that key advisory locks by number + * rather than by string. Stable across processes and JVM restarts, which String.hashCode + * is but is only 32 bit, so the two halves are combined. + */ +internal fun advisoryKeyOf(name: String): Long { + var hash = -0x340d631b7bdddcdbL // FNV-1a 64 bit offset basis + for (byte in name.toByteArray(Charsets.UTF_8)) { + hash = hash xor (byte.toLong() and 0xff) + hash *= 0x100000001b3L // FNV-1a 64 bit prime + } + return hash +} + +/** + * A named lock held across processes, so that several Ingot instances sharing one database + * do not run the same guarded section at the same time. + * + * The lock is taken on a connection borrowed for the purpose rather than inside a + * transaction. Both GET_LOCK and pg_advisory_lock are session scoped, so nothing here needs + * a transaction, and holding one open across the guarded work (which may be object storage + * I/O) is what a Galera cluster handles worst. + * + * Embedded databases support a single writer regardless, so they run the block directly and + * never ask the pool for a connection. + */ +class DatabaseLock( + private val dataSource: DataSource, + vendor: String, + private val journalist: Journalist +) : Journalist { + + private val strategy: LockStrategy = when (vendor.lowercase()) { + "mariadb", "mysql" -> MySqlLockStrategy + "postgresql" -> PostgresLockStrategy + else -> NoopLockStrategy + } + + fun withLock(name: String, timeoutSeconds: Int = 60, block: () -> T): T { + if (strategy === NoopLockStrategy) { + return block() + } + + return dataSource.connection.use { connection -> + strategy.acquire(connection, name, timeoutSeconds) + logger.debug("DatabaseLock | Acquired '$name'") + + try { + block() + } finally { + runCatching { strategy.release(connection, name) } + .onFailure { logger.warn("DatabaseLock | Failed to release '$name': ${it.message}") } + logger.debug("DatabaseLock | Released '$name'") + } + } + } + + override fun getLogger(): Logger = + journalist.logger + + private interface LockStrategy { + fun acquire(connection: Connection, name: String, timeoutSeconds: Int) + fun release(connection: Connection, name: String) + } + + private object NoopLockStrategy : LockStrategy { + override fun acquire(connection: Connection, name: String, timeoutSeconds: Int) = Unit + override fun release(connection: Connection, name: String) = Unit + } + + private object MySqlLockStrategy : LockStrategy { + override fun acquire(connection: Connection, name: String, timeoutSeconds: Int) { + // GET_LOCK returns 1 on success, 0 on timeout and NULL on error. + connection.prepareStatement("SELECT GET_LOCK(?, ?)").use { statement -> + statement.setString(1, name) + statement.setInt(2, timeoutSeconds) + statement.executeQuery().use { result -> + val acquired = result.next() && result.getInt(1) == 1 && !result.wasNull() + if (!acquired) throw DatabaseLockTimeoutException(name, timeoutSeconds) + } + } + } + + override fun release(connection: Connection, name: String) { + connection.prepareStatement("SELECT RELEASE_LOCK(?)").use { statement -> + statement.setString(1, name) + statement.executeQuery().use { } + } + } + } + + private object PostgresLockStrategy : LockStrategy { + override fun acquire(connection: Connection, name: String, timeoutSeconds: Int) { + // pg_advisory_lock has no timeout argument, so the wait is bounded with a + // statement timeout that applies to this session only. + connection.createStatement().use { it.execute("SET LOCAL lock_timeout = '${timeoutSeconds}s'") } + connection.prepareStatement("SELECT pg_advisory_lock(?)").use { statement -> + statement.setLong(1, advisoryKeyOf(name)) + try { + statement.executeQuery().use { } + } catch (exception: java.sql.SQLException) { + throw DatabaseLockTimeoutException(name, timeoutSeconds).initCause(exception) + } + } + } + + override fun release(connection: Connection, name: String) { + connection.prepareStatement("SELECT pg_advisory_unlock(?)").use { statement -> + statement.setLong(1, advisoryKeyOf(name)) + statement.executeQuery().use { } + } + } + } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +Run: `./gradlew :reposilite-backend:test --tests "com.reposilite.shared.DatabaseLockTest"` + +Expected: PASS, 5 tests. + +- [ ] **Step 5: Commit** + +```bash +git add reposilite-backend/src/main/kotlin/com/reposilite/shared/DatabaseLock.kt \ + reposilite-backend/src/test/kotlin/com/reposilite/shared/DatabaseLockTest.kt +git commit -m "feat(shared): add a cross-process advisory database lock + +Several instances sharing one database need to agree on who runs a +guarded section. GET_LOCK on MariaDB and MySQL and pg_advisory_lock on +PostgreSQL are session scoped, so the lock is taken on a borrowed +connection rather than inside a transaction: holding one open across the +guarded work is what a Galera cluster handles worst. + +Embedded databases support a single writer anyway and run the block +directly, without asking the pool for a connection at all." +``` + +--- + +### Task 2: Prove mutual exclusion against real databases + +**Files:** +- Create: `reposilite-backend/src/integration/kotlin/com/reposilite/shared/DatabaseLockIntegrationTest.kt` + +**Interfaces:** +- Consumes: `DatabaseLock`, `DatabaseLockTimeoutException` from Task 1. +- Produces: nothing that later tasks depend on. + +- [ ] **Step 1: Write the failing test** + +Create `reposilite-backend/src/integration/kotlin/com/reposilite/shared/DatabaseLockIntegrationTest.kt`: + +```kotlin +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.reposilite.shared + +import com.reposilite.journalist.backend.InMemoryLogger +import com.zaxxer.hikari.HikariConfig +import com.zaxxer.hikari.HikariDataSource +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.testcontainers.containers.MariaDBContainer +import org.testcontainers.junit.jupiter.Container +import org.testcontainers.junit.jupiter.Testcontainers +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +@Testcontainers +internal class DatabaseLockIntegrationTest { + + @Container + private val mariadb = MariaDBContainer("mariadb:11.4") + + private val logger = InMemoryLogger() + + private fun dataSource(poolSize: Int): HikariDataSource = + HikariDataSource( + HikariConfig().apply { + jdbcUrl = mariadb.jdbcUrl + username = mariadb.username + password = mariadb.password + driverClassName = "org.mariadb.jdbc.Driver" + maximumPoolSize = poolSize + } + ) + + @Test + fun `should let only one thread into the guarded section at a time`() { + val threads = 8 + dataSource(threads).use { source -> + val lock = DatabaseLock(source, "mariadb", logger) + val concurrent = AtomicInteger(0) + val maxObserved = AtomicInteger(0) + val start = CountDownLatch(1) + val pool = Executors.newFixedThreadPool(threads) + + repeat(threads) { + pool.submit { + start.await() + lock.withLock("integration-test") { + val now = concurrent.incrementAndGet() + maxObserved.updateAndGet { previous -> maxOf(previous, now) } + Thread.sleep(50) + concurrent.decrementAndGet() + } + } + } + + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(60, TimeUnit.SECONDS), "workers did not finish in time") + assertEquals(1, maxObserved.get(), "more than one thread was inside the guarded section") + } + } + + @Test + fun `should time out rather than wait forever when the lock is held`() { + dataSource(4).use { source -> + val holder = DatabaseLock(source, "mariadb", logger) + val contender = DatabaseLock(source, "mariadb", logger) + val held = CountDownLatch(1) + val release = CountDownLatch(1) + + val holderThread = Thread { + holder.withLock("timeout-test") { + held.countDown() + release.await() + } + } + holderThread.start() + assertTrue(held.await(30, TimeUnit.SECONDS), "holder never acquired the lock") + + val thrown = runCatching { + contender.withLock("timeout-test", timeoutSeconds = 1) { "unreachable" } + }.exceptionOrNull() + + release.countDown() + holderThread.join(30_000) + + assertTrue(thrown is DatabaseLockTimeoutException, "expected a timeout, got $thrown") + } + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails or is absent** + +Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.shared.DatabaseLockIntegrationTest"` + +Expected: PASS. This test is written against the implementation from Task 1, so it should pass immediately. If it fails, the Task 1 implementation is wrong and must be fixed before continuing. To confirm the test has teeth, temporarily change the `MySqlLockStrategy` dispatch in `DatabaseLock` to `NoopLockStrategy`, re-run, and observe `should let only one thread into the guarded section at a time` fail with a `maxObserved` above 1. Revert that change afterwards. + +- [ ] **Step 3: Commit** + +```bash +git add reposilite-backend/src/integration/kotlin/com/reposilite/shared/DatabaseLockIntegrationTest.kt +git commit -m "test(shared): prove the database lock excludes concurrent holders + +Asserts against a real MariaDB rather than an embedded database, because +GET_LOCK semantics are the behaviour under test and no embedded engine +has them. Covers both mutual exclusion and the timeout path, so a lock +that silently never blocks cannot pass." +``` + +--- + +### Task 3: Serialise schema initialisation + +**Files:** +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/Reposilite.kt` +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/ReposiliteFactory.kt` +- Create: `reposilite-backend/src/integration/kotlin/com/reposilite/ConcurrentSchemaInitializationIntegrationTest.kt` + +**Interfaces:** +- Consumes: `DatabaseLock` and `withLock` from Task 1. +- Produces: `Reposilite.databaseLock: DatabaseLock`, reachable from plugins via `reposilite().databaseLock`. Task 4 depends on this property. + +**Why here:** `SqlAccessTokenRepository`, `SqlStatisticsRepository` and `SqlConfigurationRepository` each run `SchemaUtils.create` in their constructor, and they are built by three different plugins. Wrapping `pluginLoader.initialize()` covers all three, and any plugin added later, in one place. + +- [ ] **Step 1: Add the lock to the Reposilite holder** + +In `reposilite-backend/src/main/kotlin/com/reposilite/Reposilite.kt`, add the import and the constructor parameter: + +```kotlin +import com.reposilite.shared.DatabaseLock +``` + +Add `val databaseLock: DatabaseLock,` to the constructor parameter list, directly after `val databaseConnection: DatabaseConnection,`: + +```kotlin +class Reposilite( + val journalist: ReposiliteJournalist, + val parameters: ReposiliteParameters, + val localConfiguration: LocalConfiguration, + val databaseConnection: DatabaseConnection, + val databaseLock: DatabaseLock, + val ioService: ExecutorService, + val scheduler: ScheduledExecutorService, + val webServer: HttpServer, + val extensions: Extensions +) : Facade, Journalist { +``` + +- [ ] **Step 2: Build the lock and wrap plugin initialisation** + +In `reposilite-backend/src/main/kotlin/com/reposilite/ReposiliteFactory.kt`, add the import: + +```kotlin +import com.reposilite.shared.DatabaseLock +``` + +Replace the `val reposilite = Reposilite(...)` construction so the connection is built first and reused: + +```kotlin + val databaseConnection = DatabaseConnectionFactory.createConnection( + workingDirectory = parameters.workingDirectory, + databaseConfiguration = parameters.database, + databaseThreadPoolSize = localConfiguration.databaseThreadPool.get() + ) + + val reposilite = Reposilite( + journalist = journalist, + parameters = parameters, + localConfiguration = localConfiguration, + databaseConnection = databaseConnection, + databaseLock = DatabaseLock( + dataSource = databaseConnection.databaseSource, + vendor = databaseConnection.database.vendor, + journalist = journalist + ), + webServer = HttpServer(), + ioService = newFixedThreadPool( + min = 0, + max = localConfiguration.ioThreadPool.get(), + prefix = "Ingot | IO" + ), + scheduler = newSingleThreadScheduledExecutor("Ingot | Scheduler"), + extensions = Extensions(journalist) + ) +``` + +Then wrap the initialisation call at the bottom of the same function: + +```kotlin + val pluginLoader = PluginLoader(parameters.pluginDirectory, reposilite.extensions) + pluginLoader.extensions.registerFacade(reposilite) + pluginLoader.loadPluginsByServiceFiles() + + // Plugins create their tables as they initialise. With several instances starting at + // once, which a rolling update produces by definition, that means concurrent DDL + // against the same tables. Galera runs DDL under total order isolation, where + // concurrent CREATE TABLE and ALTER TABLE stall the cluster or abort a node. + reposilite.databaseLock.withLock(SCHEMA_INITIALIZATION_LOCK, timeoutSeconds = 300) { + pluginLoader.initialize() + } + + return reposilite +``` + +Add the constant at the top of the `ReposiliteFactory` object body: + +```kotlin + private const val SCHEMA_INITIALIZATION_LOCK = "ingot-schema-initialization" +``` + +- [ ] **Step 3: Compile to verify nothing else constructs Reposilite** + +Run: `./gradlew :reposilite-backend:compileKotlin :reposilite-backend:compileTestKotlin :reposilite-backend:compileIntegrationKotlin` + +Expected: PASS. If a test constructs `Reposilite` directly it will fail here with a missing argument; add `databaseLock = DatabaseLock(, "sqlite", )` at that call site. + +- [ ] **Step 4: Write the concurrency test** + +Create `reposilite-backend/src/integration/kotlin/com/reposilite/ConcurrentSchemaInitializationIntegrationTest.kt`: + +```kotlin +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.reposilite + +import com.reposilite.journalist.backend.InMemoryLogger +import com.reposilite.shared.DatabaseLock +import com.reposilite.token.infrastructure.SqlAccessTokenRepository +import com.zaxxer.hikari.HikariConfig +import com.zaxxer.hikari.HikariDataSource +import org.jetbrains.exposed.v1.jdbc.Database +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.testcontainers.containers.MariaDBContainer +import org.testcontainers.junit.jupiter.Container +import org.testcontainers.junit.jupiter.Testcontainers +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.CopyOnWriteArrayList + +@Testcontainers +internal class ConcurrentSchemaInitializationIntegrationTest { + + @Container + private val mariadb = MariaDBContainer("mariadb:11.4") + + private val logger = InMemoryLogger() + + @Test + fun `should initialise the schema from several instances without failing`() { + val instances = 5 + val failures = CopyOnWriteArrayList() + val start = CountDownLatch(1) + val pool = Executors.newFixedThreadPool(instances) + + val sources = (1..instances).map { + HikariDataSource( + HikariConfig().apply { + jdbcUrl = mariadb.jdbcUrl + username = mariadb.username + password = mariadb.password + driverClassName = "org.mariadb.jdbc.Driver" + maximumPoolSize = 2 + } + ) + } + + try { + sources.forEach { source -> + pool.submit { + runCatching { + start.await() + val database = Database.connect(source) + DatabaseLock(source, "mariadb", logger).withLock("ingot-schema-initialization", timeoutSeconds = 120) { + SqlAccessTokenRepository(database, logger, emptyArray()) + } + }.onFailure { failures.add(it) } + } + } + + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(180, TimeUnit.SECONDS), "instances did not finish in time") + assertEquals(emptyList(), failures.toList(), "concurrent schema initialisation failed") + } finally { + sources.forEach { it.close() } + } + } +} +``` + +`InMemoryLogger` satisfies both the `DatabaseLock` and the `SqlAccessTokenRepository` journalist parameter, because `com.reposilite.journalist.Logger` extends `Journalist` and `InMemoryLogger` is a `Logger`. No wrapper needed. + +- [ ] **Step 5: Run the integration test** + +Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.ConcurrentSchemaInitializationIntegrationTest"` + +Expected: PASS, no failures collected. + +- [ ] **Step 6: Verify the full suite still passes** + +Run: `./gradlew :reposilite-backend:test :reposilite-backend:integration` + +Expected: PASS. A single-instance start on SQLite must be unaffected, since the lock is a no-op there. + +- [ ] **Step 7: Commit** + +```bash +git add reposilite-backend/src/main/kotlin/com/reposilite/Reposilite.kt \ + reposilite-backend/src/main/kotlin/com/reposilite/ReposiliteFactory.kt \ + reposilite-backend/src/integration/kotlin/com/reposilite/ConcurrentSchemaInitializationIntegrationTest.kt +git commit -m "fix(configuration): serialise schema initialisation across instances + +Three repositories create their tables while constructing, and three +different plugins build them, so a rolling update has every starting pod +issuing DDL against the same tables at once. Galera runs DDL under total +order isolation, where that stalls the cluster at best and aborts a node +at worst. + +Wrapping the whole plugin initialisation rather than each repository +covers all three, and every plugin added later, in one place. Embedded +databases take no lock at all." +``` + +--- + +### Task 4: Serialise metadata writes + +**Files:** +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt` +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenComponents.kt` +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenPlugin.kt` +- Create: `reposilite-backend/src/integration/kotlin/com/reposilite/maven/ConcurrentMetadataWriteIntegrationTest.kt` + +**Interfaces:** +- Consumes: `Reposilite.databaseLock` from Task 3, `DatabaseLock.withLock` from Task 1. +- Produces: `MetadataService(repositorySecurityProvider: RepositorySecurityProvider, databaseLock: DatabaseLock)`, a changed constructor signature that `MavenComponents` must match. + +**The defect:** `generatePom` reads `maven-metadata.xml`, adds a version and writes it back. `FileSystemStorageProvider` holds a `ReentrantReadWriteLock` per location, but it ends at the process boundary and its own comment says it is not truly respected. The S3 provider holds none. Two pods deploying the same coordinate lose a version. + +- [ ] **Step 1: Write the failing test** + +Create `reposilite-backend/src/integration/kotlin/com/reposilite/maven/ConcurrentMetadataWriteIntegrationTest.kt`: + +```kotlin +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.reposilite.maven + +import com.reposilite.journalist.backend.InMemoryLogger +import com.reposilite.shared.DatabaseLock +import com.zaxxer.hikari.HikariConfig +import com.zaxxer.hikari.HikariDataSource +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue +import org.junit.jupiter.api.Test +import org.testcontainers.containers.MariaDBContainer +import org.testcontainers.junit.jupiter.Container +import org.testcontainers.junit.jupiter.Testcontainers +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicInteger + +/** + * The lost update this guards against is not specific to the storage provider, so the test + * models the read-modify-write directly: a counter read, incremented and written back under + * the same lock the metadata write uses. Without the lock, concurrent writers overwrite each + * other and the final value falls short of the number of writers. + */ +@Testcontainers +internal class ConcurrentMetadataWriteIntegrationTest { + + @Container + private val mariadb = MariaDBContainer("mariadb:11.4") + + private val logger = InMemoryLogger() + + @Test + fun `should not lose an update when several writers touch one coordinate`() { + val writers = 10 + val shared = AtomicInteger(0) + val start = CountDownLatch(1) + val pool = Executors.newFixedThreadPool(writers) + + HikariDataSource( + HikariConfig().apply { + jdbcUrl = mariadb.jdbcUrl + username = mariadb.username + password = mariadb.password + driverClassName = "org.mariadb.jdbc.Driver" + maximumPoolSize = writers + } + ).use { source -> + val lock = DatabaseLock(source, "mariadb", logger) + + repeat(writers) { + pool.submit { + start.await() + lock.withLock("ingot-metadata:releases:com/example/artifact") { + val read = shared.get() + Thread.sleep(20) // widen the window a lost update would slip through + shared.set(read + 1) + } + } + } + + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(120, TimeUnit.SECONDS), "writers did not finish in time") + assertEquals(writers, shared.get(), "an update was lost") + } + } +} +``` + +- [ ] **Step 2: Run the test to verify it has teeth** + +Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.maven.ConcurrentMetadataWriteIntegrationTest"` + +Expected: PASS with the lock in place. To confirm the test detects the defect, temporarily replace `lock.withLock("...") { ... }` with a direct call to the block, re-run, and observe the assertion fail with a value well below 10. Revert afterwards. + +- [ ] **Step 3: Take the lock in MetadataService** + +In `reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt`, add the import: + +```kotlin +import com.reposilite.shared.DatabaseLock +``` + +Change the constructor: + +```kotlin +internal class MetadataService( + private val repositorySecurityProvider: RepositorySecurityProvider, + private val databaseLock: DatabaseLock +) { +``` + +Add the lock-name helper as a private method on the class, next to `resolveMetadataFile`: + +```kotlin + /** + * One lock per repository and directory, so deployments to unrelated coordinates never + * wait on each other. The prefix keeps these distinct from other Ingot locks sharing the + * same database. + */ + private fun metadataLockName(repository: Repository, gav: Location): String = + "ingot-metadata:${repository.name}:$gav" +``` + +Wrap the read-modify-write inside `generatePom`. The guarded section starts at the `putFile` for the POM and ends after `saveMetadata`, so replace the body from `repository.storageProvider` through `.mapToUnit()` with: + +```kotlin + return databaseLock.withLock(metadataLockName(repository, parentDirectory)) { + repository.storageProvider + .putFile( + location = gav, + inputStream = """ + + + 4.0.0 + $groupId + $artifactId + $version + POM was generated by Ingot + + """.trimIndent().trim().byteInputStream() + ) + .map { findMetadata(repository, parentDirectory).orElseGet { Metadata() } } + .map { + it.copy( + groupId = groupId, + artifactId = artifactId, + versioning = (it.versioning ?: Versioning()).copy( + latest = version, + release = version, + lastUpdated = timestampFormatter.format(ZonedDateTime.now()), + _versions = (it.versioning?.versions?.toMutableList() ?: mutableListOf()) + version + ) + ) + } + .flatMap { + saveMetadata( + SaveMetadataRequest( + repository = repository, + gav = parentDirectory, + metadata = it + ) + ) + } + .mapToUnit() + } +``` + +- [ ] **Step 4: Pass the lock through the component wiring** + +In `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenComponents.kt`, add the import `com.reposilite.shared.DatabaseLock` and add `private val databaseLock: DatabaseLock,` to the class constructor. + +The construction site is at line 60 and currently reads `MetadataService(securityProvider())`. Note the method is called `securityProvider()`, not `repositorySecurityProvider()`, even though the `MetadataService` parameter carries the longer name. Replace it with: + +```kotlin + private fun metadataService(): MetadataService = + MetadataService( + repositorySecurityProvider = securityProvider(), + databaseLock = databaseLock + ) +``` + +`metadataService()` is also threaded through as a default argument further down the same file (around lines 99 and 107). Leave those call sites alone; they resolve through this function. + +In `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenPlugin.kt`, add `databaseLock = reposilite().databaseLock,` to the `MavenComponents(...)` argument list, next to the other facade arguments. + +- [ ] **Step 5: Run the tests** + +Run: `./gradlew :reposilite-backend:test :reposilite-backend:integration` + +Expected: PASS. `MavenIntegrationTest` and `MavenApiIntegrationTest` exercise deployment and must be unaffected, because a single instance never contends. + +- [ ] **Step 6: Commit** + +```bash +git add reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt \ + reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenComponents.kt \ + reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenPlugin.kt \ + reposilite-backend/src/integration/kotlin/com/reposilite/maven/ConcurrentMetadataWriteIntegrationTest.kt +git commit -m "fix(maven): serialise metadata writes across instances + +generatePom reads maven-metadata.xml, adds a version and writes it back. +The filesystem provider holds a lock per location, but it ends at the +process boundary and its own comment admits it is not truly respected, +and the S3 provider holds none. Two pods deploying the same coordinate +lose a version. + +The lock is keyed by repository and directory, so deployments to +unrelated coordinates never wait on each other. Serialising in the +database rather than in the storage layer covers both providers with one +mechanism." +``` + +--- + +### Task 5: Flush statistics on shutdown + +**Files:** +- Modify: `reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt` +- Test: `reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt` (add a case to the existing file; create it from the pattern in `reposilite-backend/src/test/kotlin/com/reposilite/statistics/` if it does not exist) + +**Interfaces:** +- Consumes: `StatisticsFacade.saveRecordsBulk()`, already public. +- Produces: nothing later tasks depend on. + +**The defect:** `StatisticsFacade` buffers increments in memory and flushes every ten seconds from a scheduled task. `Reposilite.shutdown()` calls `scheduler.shutdown()` first, and `StatisticsPlugin` registers no dispose handler, so every rolling update discards up to ten seconds of statistics per pod. + +- [ ] **Step 1: Write the failing test** + +Add to `reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt`: + +```kotlin + @Test + fun `should persist buffered records when flushed explicitly`() { + val identifier = Identifier("releases", "com/example/artifact/1.0.0/artifact-1.0.0.jar") + statisticsFacade.incrementResolvedRequest(IncrementResolvedRequest(identifier)) + + statisticsFacade.saveRecordsBulk() + + assertEquals(1, statisticsFacade.countRecords()) + } + + @Test + fun `should not persist anything when the buffer is empty`() { + statisticsFacade.saveRecordsBulk() + + assertEquals(0, statisticsFacade.countRecords()) + } +``` + +Use the imports and the `statisticsFacade` fixture that the surrounding `StatisticsSpecification` already provides. If the file does not exist, create it extending `StatisticsSpecification` from `reposilite-backend/src/test/kotlin/com/reposilite/statistics/specification/StatisticsSpecification.kt`, with the Apache header shown in Task 1. + +- [ ] **Step 2: Run the test** + +Run: `./gradlew :reposilite-backend:test --tests "com.reposilite.statistics.StatisticsFacadeTest"` + +Expected: PASS. These assert existing behaviour and exist to pin it, so that the dispose wiring in Step 3 has something to rely on. + +- [ ] **Step 3: Register the dispose handler** + +In `reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt`, add the import: + +```kotlin +import com.reposilite.plugin.api.ReposiliteDisposeEvent +``` + +Add this block directly after the existing `event { _: ReposiliteInitializeEvent -> ... }` block: + +```kotlin + // The scheduled flush runs every ten seconds, and shutdown stops the scheduler before + // anything else, so without this every rolling update discards up to ten seconds of + // statistics per pod. Runs on the shutdown thread rather than through ioService, + // which has already been asked to stop by this point. + event { _: ReposiliteDisposeEvent -> + if (statisticsFacade.statisticsEnabled().get()) { + statisticsFacade.saveRecordsBulk() + } + } +``` + +- [ ] **Step 4: Verify the full suite** + +Run: `./gradlew :reposilite-backend:test :reposilite-backend:integration` + +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt \ + reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt +git commit -m "fix(statistics): flush the buffer before shutting down + +Increments are buffered in memory and written every ten seconds, and +shutdown stops the scheduler before anything else, so every rolling +update discarded up to ten seconds of statistics per pod. Multiply that +by the replica count and a routine deployment loses a visible amount. + +The flush runs on the shutdown thread, since ioService has already been +asked to stop by the time the dispose event fires." +``` + +--- + +### Task 6: Document the guarantee + +**Files:** +- Modify: `reposilite-site/data/guides/installation/kubernetes.md` + +**Interfaces:** +- Consumes: everything above. +- Produces: nothing. + +- [ ] **Step 1: Add a section on running several replicas** + +Append to `reposilite-site/data/guides/installation/kubernetes.md`: + +```markdown +### Running more than one replica + +Several Ingot instances may share one database and one bucket. Two things have to be true. + +**The database has to be shared and not embedded.** SQLite and H2 support a single writer, +so each pod would have its own state. Point every instance at the same MariaDB, MySQL or +PostgreSQL through the `database` setting. + +**The artifact storage has to be shared.** Use the S3 storage provider. A ReadWriteMany +volume with the filesystem provider is not equivalent: the filesystem locks are held inside +one process and mean nothing to another pod. + +With both in place, instances coordinate through the database. Schema initialisation is +serialised behind an advisory lock, so a rolling update does not have several pods issuing +DDL against the same tables, and deployments to the same coordinate serialise their metadata +writes so no version is lost. Deployments to unrelated coordinates never wait on each other. + +Instances still keep some state of their own: the mirror resolution cache, the credentials +cache and the count of failed logins are per pod. None of them affect correctness; the +practical consequence is that brute force protection allows `maxAttempts` per replica rather +than in total, and that `cache-purge` on the console reaches the pod serving that session. +``` + +- [ ] **Step 2: Check for forbidden characters** + +Run: `grep -n $'[—–]' reposilite-site/data/guides/installation/kubernetes.md` + +Expected: no output. + +- [ ] **Step 3: Commit** + +```bash +git add reposilite-site/data/guides/installation/kubernetes.md +git commit -m "docs(kubernetes): state what running several replicas requires + +Two requirements decide whether a second instance is safe, and neither +was written down: a shared non-embedded database, and S3 rather than a +ReadWriteMany volume, because the filesystem locks mean nothing across +pods. + +Also names the state that stays per instance, so the weaker brute force +allowance reads as a documented limit rather than a surprise." +``` + +--- + +## Self-Review + +**Spec coverage.** Stage 1 of the design has three items: serialise schema initialisation (Task 3), serialise metadata writes (Task 4), flush statistics on shutdown (Task 5). Tasks 1 and 2 build and prove the shared mechanism both depend on. Task 6 documents the resulting guarantee. The design's testing section asks for concurrency tests against a real database rather than SQLite; Tasks 2, 3 and 4 use MariaDB via Testcontainers, which is already a test dependency. + +**Not covered here, by design.** `PreservedBuildsListener` deletes snapshot files based on a timestamp read from metadata another pod may be rewriting. It is named in the spec under 1.2 but is not fixed by this plan: the listener runs on `DeployEvent`, after `generatePom` has released its lock, so covering it means either widening the lock to span the event dispatch or giving the listener its own acquisition. That decision needs a look at whether event listeners may block a deployment, which this plan does not settle. **Add it to the Stage 2 plan or raise it as its own task.** + +**Placeholder scan.** No TBD, TODO or "handle edge cases" steps. Every code step carries the code. Two steps deliberately instruct a temporary revert to prove a test detects the defect it targets, and both say to revert afterwards. + +**Type consistency.** `DatabaseLock(dataSource, vendor, journalist)` is used with the same three arguments in Tasks 1, 2, 3 and 4. `withLock(name, timeoutSeconds, block)` keeps its signature at every call site, with `timeoutSeconds` defaulting to 60 and overridden to 300 for schema initialisation and 1 in the timeout test. `advisoryKeyOf` is `internal` and used only in the main source set and the unit test, which share a module. + +**One risk worth naming.** Task 3 changes the `Reposilite` constructor, which is public API that a plugin could construct. Nothing in this repository does so outside `ReposiliteFactory`, and Step 3 of that task catches any that do at compile time, but a third-party plugin constructing `Reposilite` directly would break. That is an accepted cost, and it is worth mentioning in the pull request body. + +--- + +## Execution Handoff + +Plan complete and saved to `docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md`. Two execution options: + +**1. Subagent-Driven (recommended)** - a fresh subagent per task, review between tasks, fast iteration. + +**2. Inline Execution** - tasks executed in this session with checkpoints for review. From 90f417ef942bfed008e70986272f438f8aad82ec Mon Sep 17 00:00:00 2001 From: TheMeinerLP Date: Sat, 8 Aug 2026 21:49:40 +0200 Subject: [PATCH 3/3] docs: give every test in the stage 1 plan a reason to fail The plan had two tests that only proved the code does not crash, with their teeth checked by editing the implementation by hand and reverting. That is not reproducible, and an interrupted task leaves a disabled lock in the tree. Both now carry a negative control instead: the identical body run through the no-op strategy against the same database, asserting the violation does appear. The statistics task asserted behaviour that already worked, so it would have passed before the change it was meant to drive. It now emits the dispose event against a running instance and asserts the buffer is unwritten before and written after, which is the wiring that actually changes. --- .../2026-08-08-ha-stage-1-correctness.md | 186 +++++++++++++++--- 1 file changed, 154 insertions(+), 32 deletions(-) diff --git a/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md b/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md index e041716a3..11382f1cd 100644 --- a/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md +++ b/docs/superpowers/plans/2026-08-08-ha-stage-1-correctness.md @@ -456,13 +456,52 @@ internal class DatabaseLockIntegrationTest { } ``` -- [ ] **Step 2: Run the test to verify it fails or is absent** +- [ ] **Step 2: Add the negative control** + +The positive test only proves the lock does not crash. A lock that never blocks would pass it too. Rather than sabotaging the implementation by hand, exercise the no-op path against the same database: `DatabaseLock` with `vendor = "sqlite"` never asks for a connection and never locks, so the identical body must show the violation. + +Add to the same file: + +```kotlin + @Test + fun `should let threads overlap when the vendor takes no lock`() { + val threads = 8 + dataSource(threads).use { source -> + // Same database, same body, no-op strategy: this is the control that proves the + // assertion above can fail at all. + val lock = DatabaseLock(source, "sqlite", logger) + val concurrent = AtomicInteger(0) + val maxObserved = AtomicInteger(0) + val start = CountDownLatch(1) + val pool = Executors.newFixedThreadPool(threads) + + repeat(threads) { + pool.submit { + start.await() + lock.withLock("control-test") { + val now = concurrent.incrementAndGet() + maxObserved.updateAndGet { previous -> maxOf(previous, now) } + Thread.sleep(50) + concurrent.decrementAndGet() + } + } + } + + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(60, TimeUnit.SECONDS), "workers did not finish in time") + assertTrue(maxObserved.get() > 1, "expected the no-op strategy to allow overlap, so the locking assertion has teeth") + } + } +``` + +- [ ] **Step 3: Run the tests** Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.shared.DatabaseLockIntegrationTest"` -Expected: PASS. This test is written against the implementation from Task 1, so it should pass immediately. If it fails, the Task 1 implementation is wrong and must be fixed before continuing. To confirm the test has teeth, temporarily change the `MySqlLockStrategy` dispatch in `DatabaseLock` to `NoopLockStrategy`, re-run, and observe `should let only one thread into the guarded section at a time` fail with a `maxObserved` above 1. Revert that change afterwards. +Expected: PASS, 3 tests. The mutual-exclusion test proves the lock works, the control proves the test could detect its absence, and the timeout test proves it does not wait forever. -- [ ] **Step 3: Commit** +- [ ] **Step 4: Commit** ```bash git add reposilite-backend/src/integration/kotlin/com/reposilite/shared/DatabaseLockIntegrationTest.kt @@ -470,8 +509,12 @@ git commit -m "test(shared): prove the database lock excludes concurrent holders Asserts against a real MariaDB rather than an embedded database, because GET_LOCK semantics are the behaviour under test and no embedded engine -has them. Covers both mutual exclusion and the timeout path, so a lock -that silently never blocks cannot pass." +has them. + +A control case runs the identical body through the no-op strategy on the +same database and asserts the threads do overlap. Without it, a lock that +never blocked would pass the suite unnoticed, and the mutual exclusion +assertion would be proving nothing." ``` --- @@ -811,13 +854,57 @@ internal class ConcurrentMetadataWriteIntegrationTest { } ``` -- [ ] **Step 2: Run the test to verify it has teeth** +- [ ] **Step 2: Add the negative control** + +As in Task 2, the positive assertion needs a control that proves it could fail. Add to the same file, using the no-op strategy against the same database: + +```kotlin + @Test + fun `should lose an update when no lock is taken`() { + val writers = 10 + val shared = AtomicInteger(0) + val start = CountDownLatch(1) + val pool = Executors.newFixedThreadPool(writers) + + HikariDataSource( + HikariConfig().apply { + jdbcUrl = mariadb.jdbcUrl + username = mariadb.username + password = mariadb.password + driverClassName = "org.mariadb.jdbc.Driver" + maximumPoolSize = writers + } + ).use { source -> + // The no-op strategy models the behaviour before this change: no coordination + // between writers. If this still reached 10, the test above would prove nothing. + val lock = DatabaseLock(source, "sqlite", logger) + + repeat(writers) { + pool.submit { + start.await() + lock.withLock("ingot-metadata:releases:com/example/artifact") { + val read = shared.get() + Thread.sleep(20) + shared.set(read + 1) + } + } + } + + start.countDown() + pool.shutdown() + assertTrue(pool.awaitTermination(120, TimeUnit.SECONDS), "writers did not finish in time") + assertTrue(shared.get() < writers, "expected a lost update without a lock, so the assertion above has teeth") + } + } +``` + +- [ ] **Step 3: Run the tests** Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.maven.ConcurrentMetadataWriteIntegrationTest"` -Expected: PASS with the lock in place. To confirm the test detects the defect, temporarily replace `lock.withLock("...") { ... }` with a direct call to the block, re-run, and observe the assertion fail with a value well below 10. Revert afterwards. +Expected: PASS, 2 tests. -- [ ] **Step 3: Take the lock in MetadataService** +- [ ] **Step 4: Take the lock in MetadataService** In `reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt`, add the import: @@ -892,7 +979,7 @@ Wrap the read-modify-write inside `generatePom`. The guarded section starts at t } ``` -- [ ] **Step 4: Pass the lock through the component wiring** +- [ ] **Step 5: Pass the lock through the component wiring** In `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenComponents.kt`, add the import `com.reposilite.shared.DatabaseLock` and add `private val databaseLock: DatabaseLock,` to the class constructor. @@ -910,13 +997,13 @@ The construction site is at line 60 and currently reads `MetadataService(securit In `reposilite-backend/src/main/kotlin/com/reposilite/maven/application/MavenPlugin.kt`, add `databaseLock = reposilite().databaseLock,` to the `MavenComponents(...)` argument list, next to the other facade arguments. -- [ ] **Step 5: Run the tests** +- [ ] **Step 6: Run the tests** Run: `./gradlew :reposilite-backend:test :reposilite-backend:integration` Expected: PASS. `MavenIntegrationTest` and `MavenApiIntegrationTest` exercise deployment and must be unaffected, because a single instance never contends. -- [ ] **Step 6: Commit** +- [ ] **Step 7: Commit** ```bash git add reposilite-backend/src/main/kotlin/com/reposilite/maven/MetadataService.kt \ @@ -943,44 +1030,71 @@ mechanism." **Files:** - Modify: `reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt` -- Test: `reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt` (add a case to the existing file; create it from the pattern in `reposilite-backend/src/test/kotlin/com/reposilite/statistics/` if it does not exist) +- Create: `reposilite-backend/src/integration/kotlin/com/reposilite/statistics/StatisticsShutdownFlushIntegrationTest.kt` **Interfaces:** -- Consumes: `StatisticsFacade.saveRecordsBulk()`, already public. +- Consumes: `StatisticsFacade.saveRecordsBulk()` and `StatisticsFacade.countRecords()`, both already public. `ReposiliteRunner` exposes `reposilite: Reposilite`, which carries `extensions`. - Produces: nothing later tasks depend on. **The defect:** `StatisticsFacade` buffers increments in memory and flushes every ten seconds from a scheduled task. `Reposilite.shutdown()` calls `scheduler.shutdown()` first, and `StatisticsPlugin` registers no dispose handler, so every rolling update discards up to ten seconds of statistics per pod. +**Why an integration test:** the change is a piece of wiring, not a function. A unit test on `saveRecordsBulk()` would assert behaviour that already works and would pass before the fix, proving nothing. Emitting the dispose event against a running instance tests exactly what changes. The test emits the event directly rather than calling `reposilite.shutdown()`, because shutdown closes the database connection and the assertion needs to read through it afterwards. + - [ ] **Step 1: Write the failing test** -Add to `reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt`: +Create `reposilite-backend/src/integration/kotlin/com/reposilite/statistics/StatisticsShutdownFlushIntegrationTest.kt`: ```kotlin - @Test - fun `should persist buffered records when flushed explicitly`() { - val identifier = Identifier("releases", "com/example/artifact/1.0.0/artifact-1.0.0.jar") - statisticsFacade.incrementResolvedRequest(IncrementResolvedRequest(identifier)) +/* + * Copyright (c) 2026 dzikoysk + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ - statisticsFacade.saveRecordsBulk() +package com.reposilite.statistics - assertEquals(1, statisticsFacade.countRecords()) - } +import com.reposilite.ReposiliteRunner +import com.reposilite.maven.api.Identifier +import com.reposilite.plugin.api.ReposiliteDisposeEvent +import com.reposilite.plugin.facade +import com.reposilite.statistics.api.IncrementResolvedRequest +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Test + +internal class StatisticsShutdownFlushIntegrationTest : ReposiliteRunner() { @Test - fun `should not persist anything when the buffer is empty`() { - statisticsFacade.saveRecordsBulk() + fun `should flush buffered statistics when the instance is disposed`() { + val statisticsFacade = reposilite.extensions.facade() + val identifier = Identifier("releases", "com/example/artifact/1.0.0/artifact-1.0.0.jar") - assertEquals(0, statisticsFacade.countRecords()) + statisticsFacade.incrementResolvedRequest(IncrementResolvedRequest(identifier)) + assertEquals(0, statisticsFacade.countRecords(), "the increment should still be buffered, not written") + + reposilite.extensions.emitEvent(ReposiliteDisposeEvent(reposilite)) + + assertEquals(1, statisticsFacade.countRecords(), "dispose should have flushed the buffer") } +} ``` -Use the imports and the `statisticsFacade` fixture that the surrounding `StatisticsSpecification` already provides. If the file does not exist, create it extending `StatisticsSpecification` from `reposilite-backend/src/test/kotlin/com/reposilite/statistics/specification/StatisticsSpecification.kt`, with the Apache header shown in Task 1. +If `ReposiliteRunner` requires a JUnit extension annotation on concrete subclasses, copy the annotations from an existing subclass such as `reposilite-backend/src/integration/kotlin/com/reposilite/statistics/StatisticsIntegrationTest.kt` and use its inheritance shape rather than extending `ReposiliteRunner` directly. Match whatever that file does. -- [ ] **Step 2: Run the test** +- [ ] **Step 2: Run the test to verify it fails** -Run: `./gradlew :reposilite-backend:test --tests "com.reposilite.statistics.StatisticsFacadeTest"` +Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.statistics.StatisticsShutdownFlushIntegrationTest"` -Expected: PASS. These assert existing behaviour and exist to pin it, so that the dispose wiring in Step 3 has something to rely on. +Expected: FAIL on the second assertion, with `expected: <1> but was: <0>`, because nothing flushes on dispose yet. If it fails on the first assertion instead, the scheduled flush ran during the test; shorten the test or note it in the report before continuing. - [ ] **Step 3: Register the dispose handler** @@ -1004,17 +1118,23 @@ Add this block directly after the existing `event { _: ReposiliteInitializeEvent } ``` -- [ ] **Step 4: Verify the full suite** +- [ ] **Step 4: Run the test to verify it passes** + +Run: `./gradlew :reposilite-backend:integration --tests "com.reposilite.statistics.StatisticsShutdownFlushIntegrationTest"` + +Expected: PASS. + +- [ ] **Step 5: Verify the full suite** Run: `./gradlew :reposilite-backend:test :reposilite-backend:integration` Expected: PASS. -- [ ] **Step 5: Commit** +- [ ] **Step 6: Commit** ```bash git add reposilite-backend/src/main/kotlin/com/reposilite/statistics/application/StatisticsPlugin.kt \ - reposilite-backend/src/test/kotlin/com/reposilite/statistics/StatisticsFacadeTest.kt + reposilite-backend/src/integration/kotlin/com/reposilite/statistics/StatisticsShutdownFlushIntegrationTest.kt git commit -m "fix(statistics): flush the buffer before shutting down Increments are buffered in memory and written every ten seconds, and @@ -1094,7 +1214,9 @@ allowance reads as a documented limit rather than a surprise." **Not covered here, by design.** `PreservedBuildsListener` deletes snapshot files based on a timestamp read from metadata another pod may be rewriting. It is named in the spec under 1.2 but is not fixed by this plan: the listener runs on `DeployEvent`, after `generatePom` has released its lock, so covering it means either widening the lock to span the event dispatch or giving the listener its own acquisition. That decision needs a look at whether event listeners may block a deployment, which this plan does not settle. **Add it to the Stage 2 plan or raise it as its own task.** -**Placeholder scan.** No TBD, TODO or "handle edge cases" steps. Every code step carries the code. Two steps deliberately instruct a temporary revert to prove a test detects the defect it targets, and both say to revert afterwards. +**Placeholder scan.** No TBD, TODO or "handle edge cases" steps. Every code step carries the code. + +**Test hygiene.** Tasks 2 and 4 each carry a negative control: the identical test body run through the no-op strategy against the same database, asserting the violation does appear. Without it, a lock that never blocked would pass the positive assertion and the suite would prove nothing. Task 5 asserts the buffer is still unwritten before the dispose event and written after, so it fails before the change and passes after. Every test in this plan fails for the right reason before its implementation exists. **Type consistency.** `DatabaseLock(dataSource, vendor, journalist)` is used with the same three arguments in Tasks 1, 2, 3 and 4. `withLock(name, timeoutSeconds, block)` keeps its signature at every call site, with `timeoutSeconds` defaulting to 60 and overridden to 300 for schema initialisation and 1 in the timeout test. `advisoryKeyOf` is `internal` and used only in the main source set and the unit test, which share a module.