From 4159059ce341b862c8c90f5da6c43925485a2cd3 Mon Sep 17 00:00:00 2001 From: Thomas Neidhart Date: Wed, 2 Sep 2026 15:35:51 +0200 Subject: [PATCH] feat: give download analytics its own time-series database Adds the second datasource the download analytics need, and nothing that reads or writes it. Gated on ovsx.analytics.enabled, which no deployment sets, so by default not a single bean of it is created. TimeseriesDatabaseConfiguration builds a Hikari pool, a Flyway chain and a jOOQ DSLContext against a separate PostgreSQL database carrying the timescaledb extension. All three are defaultCandidate = false, so Boot still auto-configures the primary datasource, the registry's Flyway chain and the primary DSLContext, and only an explicit @Qualifier reaches the time-series ones. The migration set lives in db/migration-timeseries, a sibling of db/migration rather than a child: Flyway scans locations recursively, so a child would be pulled into the registry's own chain and make the registry require the timescaledb extension. It creates the download_event hypertable and the download_stats_daily continuous aggregate, with the executeInTransaction=false sidecar those statements need. Its connection timeout defaults to 2 seconds rather than Hikari's 30, because a download will record its event on the request path: an unreachable time-series database has to fail fast enough for the caller to swallow it. docker-compose gains a postgres-timeseries service on 5433, the dev config points at it and enables analytics, and the deployment configs carry the settings commented out. Test containers can be overridden with -Dovsx.test.postgres.image and -Dovsx.test.timeseries.image. TimeseriesDatabaseTest is not from #2027. Without it AbstractTimeseriesContainerTest would be an abstract base with no subclass, and nothing would exercise the migration until the analytics land. It asserts the hypertable and the aggregate exist, and that the registry database does not gain them. Co-Authored-By: gnugomez Co-Authored-By: Claude Opus 5 (1M context) --- deploy/docker/configuration/application.yml | 7 ++ deploy/openshift/application.yml | 7 ++ doc/development.md | 2 +- docker-compose.yml | 24 +++++ server/build.gradle | 8 ++ server/src/dev/resources/application.yml | 7 ++ .../TimeseriesDatabaseConfiguration.java | 102 ++++++++++++++++++ .../V1__Download_Analytics.sql | 63 +++++++++++ .../V1__Download_Analytics.sql.conf | 1 + .../AbstractPostgresContainerTest.java | 5 +- .../AbstractTimeseriesContainerTest.java | 49 +++++++++ .../timescale/TimeseriesDatabaseTest.java | 72 +++++++++++++ server/src/test/resources/application.yml | 4 + 13 files changed, 349 insertions(+), 2 deletions(-) create mode 100644 server/src/main/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseConfiguration.java create mode 100644 server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql create mode 100644 server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql.conf create mode 100644 server/src/test/java/org/eclipse/openvsx/AbstractTimeseriesContainerTest.java create mode 100644 server/src/test/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseTest.java diff --git a/deploy/docker/configuration/application.yml b/deploy/docker/configuration/application.yml index 4966e8850..d1a495fa5 100644 --- a/deploy/docker/configuration/application.yml +++ b/deploy/docker/configuration/application.yml @@ -23,6 +23,13 @@ spring: url: jdbc:postgresql://localhost:5432/openvsx username: openvsx password: openvsx + # Download analytics is disabled by default. When enabled it keeps its time-series schema in a + # separate database, migrated on its own and requiring the timescaledb extension: + # ovsx.analytics.enabled: true + # ovsx.analytics.datasource.url: jdbc:postgresql://localhost:5433/openvsx_timeseries + # ovsx.analytics.datasource.username: openvsx + # ovsx.analytics.datasource.password: openvsx + # ovsx.analytics.datasource.maximum-pool-size: 5 flyway: baseline-on-migrate: true baseline-version: 0.1.0 diff --git a/deploy/openshift/application.yml b/deploy/openshift/application.yml index 5f29f7938..86f1666a3 100644 --- a/deploy/openshift/application.yml +++ b/deploy/openshift/application.yml @@ -23,6 +23,13 @@ spring: url: jdbc:postgresql://postgresql:5432/openvsx username: openvsx password: openvsx + # Download analytics is disabled by default. When enabled it keeps its time-series schema in a + # separate database, migrated on its own and requiring the timescaledb extension: + # ovsx.analytics.enabled: true + # ovsx.analytics.datasource.url: jdbc:postgresql://postgresql-timeseries:5432/openvsx_timeseries + # ovsx.analytics.datasource.username: openvsx + # ovsx.analytics.datasource.password: openvsx + # ovsx.analytics.datasource.maximum-pool-size: 5 flyway: baseline-on-migrate: true baseline-version: 0.1.0 diff --git a/doc/development.md b/doc/development.md index 8628530eb..593ee31bc 100644 --- a/doc/development.md +++ b/doc/development.md @@ -28,7 +28,7 @@ To run the Open VSX registry in a development environment, you can use `docker c * Verify Docker Compose is installed by running `docker compose version`. If an error occurs, you may need to [install docker compose](https://docs.docker.com/compose/install/) on your machine. * Decide which profile(s) to run based on your needs. The [docker-compose.yml] file defines profiles for specific components: - * `db`: Starts the PostgreSQL container. + * `db`: Starts the PostgreSQL containers: the registry database, and the separate TimescaleDB one used by download analytics. * `es`: Starts the Elasticsearch container. * `debug`: Starts the PostgreSQL and Elasticsearch containers, which suits running the OpenVSX server and web UI locally for easier debugging. * `backend`: Starts the OpenVSX server container (java). diff --git a/docker-compose.yml b/docker-compose.yml index 7eee25687..db489df56 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -33,6 +33,26 @@ services: - db - debug + postgres-timeseries: + # download analytics only: PostgreSQL plus the timescaledb extension, kept apart from the + # registry database so the registry never needs the extension + image: timescale/timescaledb:2.17.2-pg16 + environment: + - POSTGRES_USER=openvsx + - POSTGRES_PASSWORD=openvsx + - POSTGRES_DB=openvsx_timeseries + logging: + options: + max-size: 10m + max-file: "3" + ports: + - '5433:5432' + volumes: + - postgres-timeseries-data:/var/lib/postgresql/data + profiles: + - db + - debug + elasticsearch: image: elasticsearch:9.2.8 environment: @@ -194,6 +214,7 @@ services: - 8080:8080 depends_on: - postgres + - postgres-timeseries - elasticsearch healthcheck: test: "curl --fail --silent localhost:8081/actuator/health | grep UP || exit 1" @@ -266,3 +287,6 @@ services: " profiles: - minio + +volumes: + postgres-timeseries-data: diff --git a/server/build.gradle b/server/build.gradle index bee01674c..92e141186 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -281,6 +281,14 @@ test { // observed as an OutOfMemoryError during unrelated context bootstrapping on CI. jvmArgs = ['--enable-native-access=ALL-UNNAMED', '-Xmx6144m', '-Xshare:off'] // due to https://github.com/netty/netty/issues/15161 useJUnitPlatform() + + // registry tests run on plain postgres, analytics tests on timescale/timescaledb; override + // either image with -Dovsx.test.postgres.image=... / -Dovsx.test.timeseries.image=... + ['ovsx.test.postgres.image', 'ovsx.test.timeseries.image'].each { property -> + if (System.getProperty(property) != null) { + systemProperty property, System.getProperty(property) + } + } } tasks.register('unitTests', Test) { diff --git a/server/src/dev/resources/application.yml b/server/src/dev/resources/application.yml index 77a40bbe6..8168276ac 100644 --- a/server/src/dev/resources/application.yml +++ b/server/src/dev/resources/application.yml @@ -153,6 +153,13 @@ ovsx: # path-style-access: true local: directory: /tmp/ovsx + analytics: + enabled: true + # the postgres-timeseries service of docker-compose.yml + datasource: + url: jdbc:postgresql://localhost:5433/openvsx_timeseries + username: openvsx + password: openvsx access-token: prefix: dev_ovsx # use a token prefix that clearly indicates that it's for development; the kind of # token (at_, tp_) is appended by the code, so no trailing separator here diff --git a/server/src/main/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseConfiguration.java b/server/src/main/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseConfiguration.java new file mode 100644 index 000000000..b42089009 --- /dev/null +++ b/server/src/main/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseConfiguration.java @@ -0,0 +1,102 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.analytics.timescale; + +import javax.sql.DataSource; + +import com.zaxxer.hikari.HikariConfig; +import com.zaxxer.hikari.HikariDataSource; +import org.flywaydb.core.Flyway; +import org.jooq.DSLContext; +import org.jooq.SQLDialect; +import org.jooq.impl.DSL; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.core.env.Environment; + +/** + * The time-series database behind download analytics: its own PostgreSQL instance (with the + * timescaledb extension), its own connection pool, its own Flyway migration set and its own + * jOOQ context, all configured from {@code ovsx.analytics.datasource.*}. Nothing is created + * unless {@code ovsx.analytics.enabled=true}. + */ +@Configuration +@ConditionalOnProperty(name = "ovsx.analytics.enabled", havingValue = "true") +class TimeseriesDatabaseConfiguration { + + private static final int DEFAULT_POOL_SIZE = 5; + + // A download records its event on the request path, so an unreachable time-series database has + // to surface as a fast failure the caller can swallow. Hikari's 30 second default would hold a + // request thread for that long on every download until the outage ends. + private static final long DEFAULT_CONNECTION_TIMEOUT_MS = 2_000L; + + // defaultCandidate = false keeps these beans invisible to @ConditionalOnMissingBean and to + // plain by-type injection, so Boot still auto-configures the primary DataSource, the main + // Flyway chain and the primary DSLContext; only an explicit @Qualifier reaches them. + @Bean(destroyMethod = "close", defaultCandidate = false) + DataSource timeseriesDataSource(Environment environment) { + var config = new HikariConfig(); + config.setPoolName("timeseries"); + config.setJdbcUrl(environment.getRequiredProperty("ovsx.analytics.datasource.url")); + config.setUsername(environment.getProperty("ovsx.analytics.datasource.username")); + config.setPassword(environment.getProperty("ovsx.analytics.datasource.password")); + config.setMaximumPoolSize( + environment.getProperty( + "ovsx.analytics.datasource.maximum-pool-size", + Integer.class, + DEFAULT_POOL_SIZE)); + var connectionTimeout = environment.getProperty( + "ovsx.analytics.datasource.connection-timeout", + Long.class, + DEFAULT_CONNECTION_TIMEOUT_MS); + config.setConnectionTimeout(connectionTimeout); + // Hikari rejects a validation timeout that is not below the connection timeout + config.setValidationTimeout(Math.max(250L, connectionTimeout / 2)); + return new HikariDataSource(config); + } + + /** + * Migrates the time-series schema. Configured programmatically rather than through + * {@code spring.flyway.*} so that the main and the time-series migration settings cannot leak + * into each other; in particular there is no baseline, because this database starts empty and + * an unexpected schema must fail the startup. + */ + @Bean(defaultCandidate = false) + Flyway timeseriesFlyway(@Qualifier("timeseriesDataSource") DataSource dataSource) { + var flyway = Flyway.configure() + .dataSource(dataSource) + // a sibling of db/migration, never a child: Flyway scans locations recursively, + // so a child would be swept into the registry's migration chain as well + .locations("classpath:db/migration-timeseries") + .load(); + flyway.migrate(); + return flyway; + } + + /** + * Standalone jOOQ context on the time-series pool. Being outside Spring's transaction and + * exception-translation infrastructure, queries throw jOOQ's {@code DataAccessException} + * rather than Spring's, and never join a caller's registry transaction. + */ + @Bean(defaultCandidate = false) + DSLContext timeseriesDsl( + @Qualifier("timeseriesDataSource") DataSource dataSource, + // depended upon so the schema exists before the first query + @Qualifier("timeseriesFlyway") Flyway flyway + ) { + return DSL.using(dataSource, SQLDialect.POSTGRES); + } +} diff --git a/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql b/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql new file mode 100644 index 000000000..cf8956920 --- /dev/null +++ b/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql @@ -0,0 +1,63 @@ +-- Time-series download analytics schema, applied to the separate timeseries database. Requires +-- a PostgreSQL image with the timescaledb extension available. Every migration in this set that +-- creates a hypertable, a continuous aggregate or one of their policies needs an +-- executeInTransaction=false sidecar, as those cannot run inside a transaction block. + +CREATE EXTENSION IF NOT EXISTS timescaledb; + +CREATE TABLE download_event ( + time TIMESTAMPTZ NOT NULL, + extension_id BIGINT NOT NULL, + extension_version_id BIGINT NOT NULL, + namespace VARCHAR(255) NOT NULL, + extension_name VARCHAR(255) NOT NULL, + version VARCHAR(255) NOT NULL, + target_platform VARCHAR(255) NOT NULL, + country CHAR(2), + ip VARCHAR(45), + user_agent TEXT, + count INTEGER NOT NULL DEFAULT 1 +); + +SELECT create_hypertable('download_event', by_range('time', INTERVAL '7 days')); + +CREATE INDEX de_ext_time ON download_event (extension_id, time DESC); + +-- materialized_only = false keeps the not-yet-materialized tail (e.g. today) queryable +-- through real-time aggregation, which the settling-margin logic in the query service +-- relies on. +CREATE MATERIALIZED VIEW download_stats_daily +WITH (timescaledb.continuous, timescaledb.materialized_only = false) AS +SELECT time_bucket('1 day', time) AS day, + extension_id, extension_version_id, version, target_platform, country, + SUM(count) AS downloads +FROM download_event +GROUP BY time_bucket('1 day', time), extension_id, extension_version_id, version, target_platform, country +WITH NO DATA; + +-- Materialize what is already there before the policy takes over. Until its first run the +-- watermark sits at -infinity and real-time aggregation answers everything, so the gap only +-- opens once the policy advances it: from then on buckets below the watermark are served from +-- the materialization alone, and anything never materialized reads as zero. +CALL refresh_continuous_aggregate('download_stats_daily', NULL, NULL); + +-- start_offset tracks the raw retention below rather than the schedule. Log ingestion applies no +-- date filter, so a delayed or backfilled file writes events well outside a short window, and +-- once the watermark has passed them they would never materialize while the raw rows are dropped +-- at 90 days. A refresh only reprocesses invalidated ranges, so the wider window costs nothing +-- when nothing old changed. +SELECT add_continuous_aggregate_policy('download_stats_daily', + start_offset => INTERVAL '90 days', + end_offset => INTERVAL '1 hour', + schedule_interval => INTERVAL '1 hour'); + +-- compress raw chunks after 7 days, drop them after 90 days; the daily aggregate is kept forever +ALTER TABLE download_event SET ( + timescaledb.compress, + timescaledb.compress_segmentby = 'extension_id', + timescaledb.compress_orderby = 'time DESC' +); + +SELECT add_compression_policy('download_event', INTERVAL '7 days'); + +SELECT add_retention_policy('download_event', INTERVAL '90 days'); diff --git a/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql.conf b/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql.conf new file mode 100644 index 000000000..73bd53a14 --- /dev/null +++ b/server/src/main/resources/db/migration-timeseries/V1__Download_Analytics.sql.conf @@ -0,0 +1 @@ +executeInTransaction=false diff --git a/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java b/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java index a030feeda..4ed32d30c 100644 --- a/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java +++ b/server/src/test/java/org/eclipse/openvsx/AbstractPostgresContainerTest.java @@ -16,6 +16,7 @@ import org.springframework.test.context.DynamicPropertyRegistry; import org.springframework.test.context.DynamicPropertySource; import org.testcontainers.postgresql.PostgreSQLContainer; +import org.testcontainers.utility.DockerImageName; /** * Base class for tests that need a PostgreSQL database. @@ -34,7 +35,9 @@ @Tag("integration") public abstract class AbstractPostgresContainerTest { - static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer("postgres:16.2"); + static final PostgreSQLContainer POSTGRES = new PostgreSQLContainer( + DockerImageName.parse(System.getProperty("ovsx.test.postgres.image", "postgres:16.2")) + .asCompatibleSubstituteFor("postgres")); static { POSTGRES.start(); diff --git a/server/src/test/java/org/eclipse/openvsx/AbstractTimeseriesContainerTest.java b/server/src/test/java/org/eclipse/openvsx/AbstractTimeseriesContainerTest.java new file mode 100644 index 000000000..f96bcff76 --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/AbstractTimeseriesContainerTest.java @@ -0,0 +1,49 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx; + +import org.springframework.test.context.DynamicPropertyRegistry; +import org.springframework.test.context.DynamicPropertySource; +import org.testcontainers.postgresql.PostgreSQLContainer; +import org.testcontainers.utility.DockerImageName; + +/** + * Base class for tests that need the time-series database on top of the registry database, i.e. + * download analytics. Like the registry container, this one is a JVM-wide singleton started once + * in its static initializer and reaped by Ryuk when the JVM exits. + *

+ * Two containers rather than two databases in one is deliberate: the timescale image installs the + * extension into {@code template1}, so a second database inside it would still carry timescaledb - + * exactly the coupling that keeping the two schemas apart is meant to remove. Override the image + * with {@code -Dovsx.test.timeseries.image=...} if needed. + */ +public abstract class AbstractTimeseriesContainerTest extends AbstractPostgresContainerTest { + + static final PostgreSQLContainer TIMESERIES = new PostgreSQLContainer( + DockerImageName + .parse(System.getProperty("ovsx.test.timeseries.image", "timescale/timescaledb:2.17.2-pg16")) + .asCompatibleSubstituteFor("postgres")); + + static { + TIMESERIES.start(); + } + + @DynamicPropertySource + static void timeseriesProperties(DynamicPropertyRegistry registry) { + registry.add("ovsx.analytics.enabled", () -> true); + registry.add("ovsx.analytics.datasource.url", TIMESERIES::getJdbcUrl); + registry.add("ovsx.analytics.datasource.username", TIMESERIES::getUsername); + registry.add("ovsx.analytics.datasource.password", TIMESERIES::getPassword); + registry.add("ovsx.analytics.datasource.maximum-pool-size", () -> 2); + } +} diff --git a/server/src/test/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseTest.java b/server/src/test/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseTest.java new file mode 100644 index 000000000..f42358b7f --- /dev/null +++ b/server/src/test/java/org/eclipse/openvsx/analytics/timescale/TimeseriesDatabaseTest.java @@ -0,0 +1,72 @@ +/****************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation. + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * https://www.eclipse.org/legal/epl-2.0. + * + * SPDX-License-Identifier: EPL-2.0 + *****************************************************************************/ +package org.eclipse.openvsx.analytics.timescale; + +import java.util.List; + +import org.jooq.DSLContext; +import org.jooq.impl.DSL; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Qualifier; +import org.springframework.boot.test.context.SpringBootTest; + +import org.eclipse.openvsx.AbstractTimeseriesContainerTest; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * The time-series database stands on its own: nothing reads or writes it yet, so its own migration + * chain applying to a separate database - without disturbing the registry's - is the only thing + * there is to verify. + */ +@SpringBootTest +class TimeseriesDatabaseTest extends AbstractTimeseriesContainerTest { + + @Autowired + @Qualifier("timeseriesDsl") + DSLContext timeseriesDsl; + + // defaultCandidate = false on the time-series beans means plain by-type injection still reaches + // the registry's own DSLContext, which is what this asserts. + @Autowired + DSLContext registryDsl; + + @Test + void appliesItsOwnMigrationsToTheTimeSeriesDatabase() { + // the hypertable and the continuous aggregate the analytics schema is built on + assertThat(namesFrom("timescaledb_information.hypertables", "hypertable_name")) + .contains("download_event"); + assertThat(namesFrom("timescaledb_information.continuous_aggregates", "view_name")) + .contains("download_stats_daily"); + } + + // The registry database must not gain the analytics schema. The two migration sets are siblings + // under db/ rather than parent and child, because Flyway scans locations recursively and a child + // would have been pulled into the registry's own chain - which would also make the registry + // require the timescaledb extension. + @Test + void leavesTheRegistryDatabaseAlone() { + var registryTables = registryDsl + .select(DSL.field("table_name", String.class)) + .from("information_schema.tables") + .where(DSL.field("table_schema").eq("public")) + .fetchInto(String.class); + + assertThat(registryTables).doesNotContain("download_event", "download_stats_daily"); + } + + private List namesFrom(String table, String column) { + return timeseriesDsl.select(DSL.field(column, String.class)).from(table).fetchInto(String.class); + } +} diff --git a/server/src/test/resources/application.yml b/server/src/test/resources/application.yml index cd2c5d886..993116a52 100644 --- a/server/src/test/resources/application.yml +++ b/server/src/test/resources/application.yml @@ -37,6 +37,10 @@ jobrunr: allow-anonymous-data-usage: false ovsx: + analytics: + # the timeseries container is shared the same way, so its pool needs the same treatment + datasource: + maximum-pool-size: 2 elasticsearch: # Disabled by default for tests: the Elasticsearch container is expensive, so only tests that # activate the "test_search" profile (which also provisions the container) enable it, via