Skip to content

Avoid retaining intermediate SQL reporting rows - #23

Merged
phax merged 1 commit into
phax:mainfrom
beint-no:map-sql-reporting-rows-directly
Sep 5, 2026
Merged

Avoid retaining intermediate SQL reporting rows#23
phax merged 1 commit into
phax:mainfrom
beint-no:map-sql-reporting-rows-directly

Conversation

@gregjotau

Copy link
Copy Markdown
Contributor

The SQL backend currently loads every row into a cloned DBResultRow collection, then builds the PeppolReportingItem collection in a second pass. Large reporting periods retain both the intermediate row/column wrappers and the final objects.

Use the existing DBExecutor.queryAll row callback to construct each reporting item immediately. The SQL, date bounds, column conversions and eager Iterable API remain the same; no dependency, JDBC fetch setting, cursor API or schema changes are needed.

The query's ESuccess is checked before returning the collection. A failed query now throws the already-declared PeppolReportingBackendException, rather than returning an empty report as before; failure after some rows also cannot expose a partial collection. The database executor continues to log the underlying error.

Measured against main 55153b3 using local PostgreSQL 18.6 and 200,000 synthetic rows (the existing PeppolReportingTestItems.sendingItem with distinct end-user IDs), OpenJDK 26.0.2.1/macOS arm64, -Xms64m -Xmx768m, five alternating fresh JVM runs per version:

Median Before After
Peak process RSS (/usr/bin/time -l) 642.2 MiB 496.1 MiB
Query + item mapping elapsed time 387.6 ms 342.0 ms
Current-thread query allocation 443.6 MiB 423.7 MiB
Heap after GC with the returned items retained 165.3 MiB 165.4 MiB

All runs returned 200,000 rows with identical checksums. This local workload uses about 146 MiB (23%) less peak process memory. The final collection size is unchanged; this removes intermediate retention, not the eager collection or JDBC driver's own buffering.

Validation:

  • mvn -B -pl peppol-reporting-backend-sql -am -Dtest=SPITest,PeppolReportingBackendSqlTest -Dsurefire.failIfNoSpecifiedTests=false clean verify: all four reactor modules pass, three selected tests, no failures/errors/skips.
  • All 16 existing shared backend contract tests run against the changed SQL backend on an isolated local PostgreSQL database, with zero failures or skipped assumptions. These cover field round trips, inclusive dates, multiple rows, empty results, callbacks and lifecycle behavior. Other database dialects were not exercised.
  • The new database-independent regression test verifies query failure cannot be mistaken for an empty report. It fails against the original implementation and passes with this change.

@phax
phax merged commit 0b71b90 into phax:main Sep 5, 2026
1 check passed
@phax

phax commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Thanks, another quick win :)

@phax phax self-assigned this Sep 5, 2026
@phax phax added backend-sql Reporting Backend SQL related enhancement New feature or request labels Sep 5, 2026
@gregjotau
gregjotau deleted the map-sql-reporting-rows-directly branch September 5, 2026 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend-sql Reporting Backend SQL related enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants