feat(reporting): dedicated read-only datasource for Jasper fills (Story 29.1)

[gpascucci]

Implements Epic 29 / Story 29.1 — the review's highest-value fix.

Problem

There was one @Primary datasource. The Schedule 9 SQL-in-template fill borrowed a connection from it and held it for the entire Jasper render (embedded SQL + PDF formatting) — the longest-held connection in the app. With maximum-pool-size defaulting to 5, ~5 concurrent report renders could starve ordinary schedule requests to the 30s connection-timeout. (The "dedicated reporting datasource" in the architecture note wasn't actually implemented.)

Change

• New reportingDataSource bean: a separate, read-only, small Hikari pool (default max 3, env-tunable via SPRING_DATASOURCE_REPORTING_MAX_POOL_SIZE / _MIN_IDLE / _POOL_NAME), same DB.
• ReportService now draws its fill connection from it via @Qualifier("reportingDataSource").
• The @Primary bean and the jdbcTemplate / namedParameterJdbcTemplate / transactionManager wiring are unchanged — writes still roll back through the transactional pool.
• No leak-detection on the reporting pool (a render legitimately holds its connection for the full fill+format).

Net: report fills can only ever exhaust their own small pool; the transactional pool that serves schedule requests is isolated.

Tests

• DataSourceConfigurationTest (unit, no DB): the reporting config is read-only, has its own named pool, a small ceiling, leak-detection disabled.
• ReportingDataSourceIsolationIT (Oracle Testcontainers): the reporting datasource is a distinct read-only pool; saturating it leaves the @Primary pool acquirable.

Backend compiles, unit test green, checkstyle clean. The IT runs under the Oracle Testcontainers profile (CI / WSL) — not run on this Windows box.

Tracked in ilcr-bmad as Story 29.1 (draft PR #67 → will flip to review).

🤖 Generated with Claude Code

---

Thanks for the PR!

Deployments, as required, will be available below:

• Frontend
• Backend

Please create PRs in draft mode. Mark as ready to enable:

• Analysis Workflow

After merge, new images are deployed in:

• Merge Workflow

[SScholefield]

Reviewed the diff plus DataSourceConfiguration, ValidatingDataSource, ReportService, AbstractOracleIT, the pom and openshift.deploy.yml. The approach is right and the isolation is wired the way the description claims: the new bean is not @primary, so jdbcTemplate / namedParameterJdbcTemplate / transactionManager and every by-type DataSource injection keep resolving to the transactional pool, and only ReportService draws from the reporting pool. A few things worth resolving before merge.

1. The IT has never been executed — please confirm it's green in CI before merging. ReportingDataSourceIsolationIT asserts connection.isReadOnly() is true (line ~227). Oracle's JDBC driver has historically treated setReadOnly as an advisory no-op, and its isReadOnly behaviour is not something I'd assume without seeing the assertion pass against gvenzl/oracle-free. This is the first place that test will ever run, so it's a coin-flip on red CI.

2. saturatingTheReportingPoolLeavesTheTransactionalPoolAcquirable isn't sensitive to the regression it's guarding. It holds reportingMax (3) connections and then asserts the primary pool is still acquirable. Under the pre-fix wiring — one shared pool of 5 — holding 3 connections still leaves 2, so the assertion passes there too. As written it can't fail if someone reverts the qualifier on ReportService. To make it a real guard, either drive reporting max ≥ the primary max via @TestPropertySource (SPRING_DATASOURCE_REPORTING_MAX_POOL_SIZE) so saturating it would provably have drained a shared pool, or assert on the primary HikariPoolMXBean — total/active connections unmoved while the reporting pool sits at its ceiling.

3. The reporting pool inherits the primary's 30s connection-timeout (DataSourceConfiguration.java:82 reads spring.datasource.hikari.connection-timeout). With a ceiling of 3, the 4th concurrent render now parks a servlet thread for a full 30 seconds before failing. That's a smaller blast radius than starving the transactional pool, but it's still 30s of held Tomcat thread per queued report. Worth its own (shorter) spring.datasource.reporting.hikari.connection-timeout and a deliberate 503/"try again" rather than a 30s hang into a 500.

4. readOnly = true is a hint, not enforcement. The javadoc and yml comment ("read-only", "report fills never write") read as a guarantee; on Oracle it's a pool-level flag, not a privilege. It's fine defence-in-depth, but if the intent is that this path cannot write, the mechanism is a read-only DB account/role, not HikariConfig.setReadOnly. Suggest softening the wording so a future reader doesn't lean on it.

5. The two pool builders have drifted apart structurally. The primary bean still constructs its HikariConfig inline (:44-57) while reporting goes through the extracted reportingHikariConfig (:104-126) — ~12 near-identical setters in two places, plus the same three requireProperty calls. The next property added to the primary pool will silently miss the reporting one. One shared builder with the reporting-specific overrides (read-only, no leak detection, own name/ceiling) applied on top keeps the invariants the new unit test pins, without the copy.

6. No observability on the thing this PR is about. The premise is pool exhaustion, but neither pool exposes hikaricp_connections_pending / _active — the HikariDataSource is hand-built and wrapped in a DelegatingDataSource, so Boot's Hikari metrics binding doesn't pick it up, and spring-boot-jdbc isn't on the classpath anyway. Passing the MeterRegistry into both configs (setMetricRegistry) would make "reporting pool saturating" alertable instead of inferred from latency. Fine as a follow-up, but it's the difference between fixing this once and knowing whether 3 is the right number.

[Rylan-cgi]

PR #304 Code Review: Requested Changes

Here is a summary of the requested changes and specific technical fixes to harden the dedicated reporting pool implementation (Story 29.1).

1. Set a Shorter, Dedicated Connection Timeout (application.yml & DataSourceConfiguration.java)

To prevent reporting pool saturation from blocking Tomcat threads, introduce a fast-failing 5000ms (5s) timeout for the reporting pool:

# backend/src/main/resources/application.yml
spring:
  datasource:
    reporting:
      hikari:
        connection-timeout: ${SPRING_DATASOURCE_REPORTING_CONNECTION_TIMEOUT:5000}

In DataSourceConfiguration.java:

@Value("${spring.datasource.reporting.hikari.connection-timeout:5000}") long connectionTimeout,

2. Prevent Code Duplication & Configuration Drift (DataSourceConfiguration.java)

Dry up the config by extracting shared pool settings into a helper, then override properties specifically for the reporting pool:

private static void configureBasicHikari(HikariConfig config, String url, String username, String password, 
                                         String driverClassName, long idleTimeout, long maxLifetime, 
                                         long keepaliveTime, String validationQuery) {
    config.setJdbcUrl(url);
    config.setUsername(username);
    config.setPassword(password);
    config.setDriverClassName(driverClassName);
    config.setIdleTimeout(idleTimeout);
    config.setMaxLifetime(maxLifetime);
    config.setKeepaliveTime(keepaliveTime);
    config.setConnectionTestQuery(validationQuery);
}

3. Wire Up Metrics & Observability for Alerting (DataSourceConfiguration.java)

Inject Spring's MeterRegistry into DataSourceConfiguration and register both Hikari pools (config.setMetricRegistry(meterRegistry)) to make connection active/pending metrics alertable under load.

4. Guard Against Resource Leaks in Tests (ReportingDataSourceIsolationIT.java)

Isolate connection closing in a try-catch block to ensure one failing close does not bypass the remaining cleanup, preventing DB-session starvation in test runs:

finally {
    for (Connection connection : held) {
        if (connection != null) {
            try {
                connection.close();
            } catch (SQLException e) {
                // Log and continue to ensure remaining connections are closed
            }
        }
    }
}

5. Strengthen the Pool Isolation Assertions (ReportingDataSourceIsolationIT.java)

The current test is weak (holding 3 connections still leaves 2 available if it accidentally fell back to the shared primary pool of 5). Inject HikariPoolMXBean or configure the test reporting max size to exceed the primary max using @TestPropertySource to prove true pool independence.

6. Make Reporting DataSource Bean Conditional

Ensure the reporting bean carries conditional properties matching ReportService:

@Bean("reportingDataSource")
@ConditionalOnProperty(name = "ilcr.reporting.enabled", havingValue = "true", matchIfMissing = true)
public DataSource reportingDataSource(...) { ... }

[gpascucci]

Thanks @SScholefield and @Rylan-cgi — addressed in 3fa4f26.

IT is now green (SScholefield #1). I ran ReportingDataSourceIsolationIT against gvenzl/oracle-free in WSL: Tests run: 2, Failures: 0, BUILD SUCCESS. And per your point, I dropped the connection.isReadOnly() assertion (Oracle treats setReadOnly as advisory, so that was a coin-flip) — the test now asserts the pool config isReadOnly() instead.

Isolation test is now a real regression guard (SScholefield #2 / Rylan #5). @TestPropertySource sets the reporting pool max to 6, deliberately above the primary's 5. Saturating it with 6 held connections is only possible with a separate pool — if someone reverted the @Qualifier and it fell back to the shared pool of 5, the 6th acquire would block to the timeout and fail the test. It also now asserts the primary pool's HikariPoolMXBean active connections stay 0 during reporting saturation.

Fast-fail timeout (SScholefield #3 / Rylan #1). The reporting pool has its own spring.datasource.reporting.hikari.connection-timeout (SPRING_DATASOURCE_REPORTING_CONNECTION_TIMEOUT, default 5000ms), so the Nth queued render fails fast instead of parking a Tomcat thread for 30s.

DRY (SScholefield #5 / Rylan #2). Both pools now build through a shared applyCommonHikari(...) (URL/credentials/driver/lifetimes/validation); reportingHikariConfig layers only the reporting overrides, so a new shared setting can't silently miss one pool.

Metrics (SScholefield #6 / Rylan #3). Both pools bind to the MeterRegistry (setMetricRegistry via ObjectProvider), so hikaricp_connections_active/_pending are exposed for alerting rather than inferred from latency.

read-only wording (SScholefield #4). Softened in the javadoc, the yml comment, and the IT — it's a Hikari/JDBC hint + defence-in-depth, not Oracle-enforced write-prevention (that would be a read-only DB account).

Rylan #4 (test connection close): now best-effort in a per-connection try/catch so one failing close can't leak the rest.

One I did NOT take — Rylan #6 (@ConditionalOnProperty("ilcr.reporting.enabled") on the bean): with matchIfMissing=true it's on by default, but if anyone set it false the bean would vanish while ReportService's required @Qualifier("reportingDataSource") injection still demands it → the context would fail to start. It'd need a fallback-to-primary path to be safe, and there's no use case to run reporting off while the datasource is on (both are already gated by ilcr.datasource.enabled). Happy to add a real toggle-with-fallback if you'd like it as a follow-up.

Unit test + checkstyle green locally; the isolation IT green in WSL.

[Rylan-cgi]

Issues fixed. Looks good!