feat: give download analytics its own time-series database - #2133
Open
netomi wants to merge 1 commit into
Open
Conversation
This was referenced Sep 2, 2026
netomi
force-pushed
the
split/timescale-datasource
branch
from
September 2, 2026 16:09
794a759 to
12a0412
Compare
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 <gomezbanaco@gmail.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
netomi
force-pushed
the
split/timescale-datasource
branch
from
September 3, 2026 14:46
cdf9335 to
4159059
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack: 4 of 6. Split out of #2027. Base: #2132 (
split/ingestion-rework).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.What it adds
TimeseriesDatabaseConfigurationbuilds a Hikari pool, a Flyway chain and a jOOQDSLContextagainst a separate PostgreSQL database carrying thetimescaledbextension. All three aredefaultCandidate = false, so Boot still auto-configures the primary datasource, the registry's Flyway chain and the primaryDSLContext— only an explicit@Qualifierreaches the time-series ones.The migration set lives in
db/migration-timeseries, a sibling ofdb/migrationrather than a child. Flyway scans locations recursively, so a child would be pulled into the registry's own chain and would make the registry require thetimescaledbextension. It creates thedownload_eventhypertable and thedownload_stats_dailycontinuous aggregate, with theexecuteInTransaction=falsesidecar those statements need.Also:
docker-composegains apostgres-timeseriesservice on 5433, the dev config points at it and enables analytics, the deployment configs carry the settings commented out, and the test containers can be overridden with-Dovsx.test.postgres.image/-Dovsx.test.timeseries.image.Worth a deliberate look
The connection budget. This is the PR where a deployment goes from one pool to two, and #2027 never states what the combined budget should be against the deployed
max_connections. The test suite already hit that wall — the previous PR in this stack needed a pool cap because the extra Spring context exhausted the shared container — which is a useful signal rather than just a test annoyance. Production sizing is a decision, not a default.The 2-second connection timeout, against Hikari's 30. It is deliberate and worth understanding now, because the reason only becomes live in the next PR: a download records its event on the request path, so an unreachable time-series database has to fail fast enough for the caller to swallow it. At 30 seconds an outage would hold a request thread on every download.
One test that is not from #2027
TimeseriesDatabaseTest. Without it this PR would shipAbstractTimeseriesContainerTestas an abstract base with no subclass — dead code — and nothing would exercise the migration until the analytics land.It asserts the hypertable and the continuous aggregate exist in the time-series database, and that the registry database does not gain them. That second assertion guards the sibling-vs-child location choice above, which is otherwise invisible.
Its fail-first check is weaker than I would like, and I would rather say so than imply otherwise: removing
create_hypertablefrom the migration reddens both tests rather than just the hypertable assertion, because the continuous aggregate depends on it, so the migration fails and the context never starts. It shows the test is coupled to the migration; it does not show each assertion is individually wired.Verification
Full server suite: 1179 tests passing (1177 from the base plus these two).
Refs #2027, #2025