Skip to content

Commit a6857cc

Browse files
gpascucciclaude
andcommitted
perf(reporting): address PR #333 review — real disk swap, spill IT, export-failure logging, async timeout
Follow-up to the streaming/virtualizer change, addressing the CHANGES_REQUESTED review: 1. Spill-and-read-back coverage: new ReportVirtualizerSpillIT forces ilcr.reporting.virtualizer.max-size=1 on an allSchedules print and asserts the extracted PDF text is complete — proving virtualized pages are read back off the swap file on the async dispatch thread (the default max-size=300 never pages out in tests). 2. Export failures are now observable: RenderedReport.writeTo throws after the 200 + application/pdf headers are committed, so no @ExceptionHandler can run. ReportController now logs at ERROR with mill/year before rethrowing, so ops can correlate a user's "PDF won't open" with a server-side cause. 3. spring.mvc.async.request-timeout set explicitly (120s, env-overridable) — the streamed export runs under the async clock; make the ceiling a decision, not a container default. 4. Real disk-backed swap in OpenShift: the container runs readOnlyRootFilesystem and /tmp is a memory-backed emptyDir, so java.io.tmpdir would spill to RAM and defeat the heap relief. Mount a dedicated DISK-backed report-swap emptyDir (sizeLimit 512Mi, bounded by an ephemeral-storage limit) and set ILCR_REPORTING_SWAP_DIR to it. 5. Documented the max-size=300 rationale (conservative page-count default, tunable) and the swap-directory→real-ephemeral-volume requirement in application.yml. All 34 reporting tests green (incl. the new spill IT); checkstyle + compile green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent a4cc449 commit a6857cc

4 files changed

Lines changed: 136 additions & 6 deletions

File tree

backend/openshift.deploy.yml

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,11 @@ objects:
137137
value: "true"
138138
- name: ILCR_DATASOURCE_ENABLED
139139
value: ${ILCR_DATASOURCE_ENABLED}
140+
# Story 29.2 — point the Jasper report virtualizer's swap file at the dedicated
141+
# DISK-backed ephemeral volume below, NOT java.io.tmpdir (/tmp here is a Memory-backed
142+
# emptyDir, so spilling there would stay in RAM and defeat the heap relief).
143+
- name: ILCR_REPORTING_SWAP_DIR
144+
value: /var/tmp/report-swap
140145
- name: ORACLEDB_HOST
141146
valueFrom:
142147
secretKeyRef:
@@ -182,9 +187,13 @@ objects:
182187
requests:
183188
cpu: ${CPU_REQUEST}
184189
memory: ${MEMORY_REQUEST}
185-
ephemeral-storage: "200Mi"
190+
ephemeral-storage: "256Mi"
186191
limits:
187192
memory: ${MEMORY_LIMIT}
193+
# Bound total ephemeral (node) storage: base image scratch + the 512Mi report-swap
194+
# volume below (Story 29.2). A print that would exceed this is evicted — a bounded,
195+
# observable failure — rather than filling node disk.
196+
ephemeral-storage: "1Gi"
188197
securityContext:
189198
allowPrivilegeEscalation: false
190199
runAsNonRoot: true
@@ -196,6 +205,8 @@ objects:
196205
volumeMounts:
197206
- name: tmp
198207
mountPath: /tmp
208+
- name: report-swap
209+
mountPath: /var/tmp/report-swap
199210
- name: api-cert
200211
mountPath: /cert
201212
startupProbe:
@@ -221,6 +232,15 @@ objects:
221232
emptyDir:
222233
medium: Memory
223234
sizeLimit: 256Mi
235+
# Story 29.2 — dedicated DISK-backed ephemeral volume for the Jasper report swap file.
236+
# Deliberately NOT medium: Memory (unlike /tmp above): spilling large report page objects
237+
# here relieves the JVM heap onto node ephemeral storage instead of RAM, which is the whole
238+
# point of the virtualizer. Bounded by sizeLimit so a runaway print is evicted (a bounded,
239+
# observable failure) rather than silently consuming node disk. Writable despite the
240+
# read-only root filesystem because it is a mounted volume.
241+
- name: report-swap
242+
emptyDir:
243+
sizeLimit: 512Mi
224244
# Oracle keystore written by the init container, read by the app
225245
- name: api-cert
226246
persistentVolumeClaim:

backend/src/main/java/ca/bc/gov/nrs/ilcr/reporting/ReportController.java

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
import ca.bc.gov.nrs.ilcr.millcontext.MillContextService.MillYearContext;
55
import ca.bc.gov.nrs.ilcr.reporting.api.PrintRequest;
66
import ca.bc.gov.nrs.ilcr.reporting.api.ReportApi;
7+
import org.slf4j.Logger;
8+
import org.slf4j.LoggerFactory;
79
import org.springframework.http.HttpHeaders;
810
import org.springframework.http.MediaType;
911
import org.springframework.http.ResponseEntity;
@@ -29,6 +31,8 @@
2931
@ConditionalOnProperty(name = "ilcr.datasource.enabled", havingValue = "true")
3032
public class ReportController implements ReportApi {
3133

34+
private static final Logger log = LoggerFactory.getLogger(ReportController.class);
35+
3236
private final MillContextService millContextService;
3337
private final ReportService reportService;
3438
private final PrintService printService;
@@ -51,7 +55,7 @@ public ResponseEntity<StreamingResponseBody> getSchedule9Pdf(
5155
// export streams, so a rejected render still produces a problem+json error, never a half-written PDF.
5256
RenderedReport report = reportService.renderSchedule9(context.millId(), context.year());
5357
String filename = "schedule9_" + context.millId() + "_" + context.year() + ".pdf";
54-
return pdfResponse(filename, report);
58+
return pdfResponse(filename, context.millId(), context.year(), report);
5559
}
5660

5761
@Override
@@ -62,7 +66,7 @@ public ResponseEntity<StreamingResponseBody> printSchedules(
6266
MillYearContext context = millContextService.validateMillYearActive(millId, year);
6367
validateSelection(request);
6468
RenderedReport report = printService.render(context, request);
65-
return pdfResponse("schedules_print.pdf", report);
69+
return pdfResponse("schedules_print.pdf", context.millId(), context.year(), report);
6670
}
6771

6872
/**
@@ -71,12 +75,23 @@ public ResponseEntity<StreamingResponseBody> printSchedules(
7175
* and try-with-resources closes the {@link RenderedReport} on both success and failure, so the
7276
* virtualizer's swap file is never leaked. The status + headers are set on the ResponseEntity here,
7377
* before any byte is written, so the attachment filename and content type are always applied.
78+
*
79+
* <p>An export failure surfaces DIFFERENTLY from the pre-fill guards: by the time bytes are written
80+
* the 200 + {@code application/pdf} headers are already committed, so no {@code @ExceptionHandler}
81+
* can turn it into a {@code problem+json} — the client just gets a truncated PDF. It is therefore
82+
* logged at ERROR with the mill/year (the only server-side signal ops can correlate with a user's
83+
* "the PDF won't open") before being rethrown so the container aborts the response. The async render
84+
* runs under {@code spring.mvc.async.request-timeout}; a timeout produces the same truncated shape.
7485
*/
7586
private static ResponseEntity<StreamingResponseBody> pdfResponse(
76-
String filename, RenderedReport report) {
87+
String filename, long millId, int year, RenderedReport report) {
7788
StreamingResponseBody body = out -> {
7889
try (report) {
7990
report.writeTo(out);
91+
} catch (RuntimeException e) {
92+
log.error("Report export failed after the response was committed for mill {} year {} ({}) — "
93+
+ "the client received a truncated PDF", millId, year, filename, e);
94+
throw e;
8095
}
8196
};
8297
return ResponseEntity.ok()

backend/src/main/resources/application.yml

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ spring:
1616
# deferred-work.md:180; turning the feature off closes it for every schedule at once rather
1717
# than leaving Schedule 5 the only safe one.
1818
accept-float-as-int: false
19+
mvc:
20+
async:
21+
# Story 29.2 — report/print PDFs now stream via StreamingResponseBody, so the export runs under
22+
# the async request clock. Set it EXPLICITLY (rather than inheriting the container default) so a
23+
# slow or large "all schedules" render has a deliberate ceiling. On timeout Spring completes the
24+
# response; for an already-committed streamed PDF that means a truncated body (logged at ERROR in
25+
# ReportController) — so size this to the worst-case render, not tight. 120s covers a big combined
26+
# print with headroom; raise via SPRING_MVC_ASYNC_REQUEST_TIMEOUT if a legitimate render exceeds it.
27+
request-timeout: ${SPRING_MVC_ASYNC_REQUEST_TIMEOUT:120000}
1928
datasource:
2029
url: ${SPRING_DATASOURCE_URL:}
2130
username: ${SPRING_DATASOURCE_USERNAME:}
@@ -81,10 +90,19 @@ ilcr:
8190
reporting:
8291
# Story 29.2 — the Jasper swap-file virtualizer that spills report page objects to disk under a
8392
# large or combined ("all schedules") fill, so the section object graph never pins the JVM heap.
84-
# swap-directory empty → the JVM temp dir; max-size is the number of pages kept in memory before
85-
# the least-recently-used ones page out. The swap file is deleted when each render finishes.
8693
virtualizer:
94+
# swap-directory: where the per-render swap file lives. Empty → the JVM temp dir (java.io.tmpdir).
95+
# In OpenShift the container root fs is read-only AND /tmp is a MEMORY-backed emptyDir, so the
96+
# deploy sets ILCR_REPORTING_SWAP_DIR to a dedicated DISK-backed ephemeral volume (see
97+
# backend/openshift.deploy.yml) — otherwise "spill to disk" would spill to RAM and defeat the
98+
# whole point. The file is created eagerly per render and deleted when the render finishes.
8799
swap-directory: ${ILCR_REPORTING_SWAP_DIR:}
100+
# max-size: pages kept in memory before the least-recently-used ones page out to the swap file.
101+
# A deliberate, conservative default (a page count, not a measured byte budget): high enough that
102+
# an ordinary single-schedule print never pages out (avoiding swap churn on the common path), low
103+
# enough to bound heap on a big combined print. Tune via ILCR_REPORTING_VIRTUALIZER_MAX_SIZE if
104+
# profiling shows a real workload wants a different ceiling. (The spill path itself is proven by
105+
# ReportVirtualizerSpillIT, which forces max-size=1.)
88106
max-size: ${ILCR_REPORTING_VIRTUALIZER_MAX_SIZE:300}
89107

90108
features:
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package ca.bc.gov.nrs.ilcr.reporting;
2+
3+
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
5+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
6+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
7+
8+
import ca.bc.gov.nrs.ilcr.support.AbstractOracleIT;
9+
import org.apache.pdfbox.Loader;
10+
import org.apache.pdfbox.pdmodel.PDDocument;
11+
import org.apache.pdfbox.text.PDFTextStripper;
12+
import org.junit.jupiter.api.DisplayName;
13+
import org.junit.jupiter.api.Test;
14+
import org.springframework.http.MediaType;
15+
import org.springframework.test.context.TestPropertySource;
16+
import org.springframework.test.web.servlet.MvcResult;
17+
18+
/**
19+
* Spill-and-read-back acceptance test for the Jasper virtualizer (Story 29.2). {@code
20+
* ilcr.reporting.virtualizer.max-size=1} forces the fill to keep just ONE page in memory and page all
21+
* the rest out to the swap file — so a real "all schedules" print (mill 517/2021 has data for all six
22+
* in-scope schedules) genuinely exercises the riskiest new path: reading virtualized pages BACK off
23+
* the swap file during export, which runs on the async dispatch thread AFTER the request thread that
24+
* filled them has returned. The other reporting ITs run at the default max-size (300) and never page
25+
* out, so this is the only test that would catch a devirtualization regression.
26+
*
27+
* <p>The assertion is that the extracted PDF text is still COMPLETE — every section's heading and a
28+
* seeded body value from each — proving no page was lost or corrupted across the spill/read-back.
29+
* Security is OFF (isolated from authz).
30+
*/
31+
@DisplayName("POST /api/v1/reports/print — virtualizer spills to swap and reads back (max-size=1)")
32+
@TestPropertySource(properties = {
33+
"ilcr.security.enabled=false",
34+
"ilcr.reporting.virtualizer.max-size=1"
35+
})
36+
class ReportVirtualizerSpillIT extends AbstractOracleIT {
37+
38+
private static final String ENDPOINT = "/api/v1/reports/print";
39+
40+
@Test
41+
@DisplayName("allSchedules print with max-size=1 -> complete PDF (every section survives the spill)")
42+
void allSchedules_withAggressiveSpill_producesCompletePdf() throws Exception {
43+
// max-size=1 guarantees pages page out to the swap file mid-fill; a complete PDF proves they are
44+
// read back correctly on the async export thread.
45+
String selection = """
46+
{"allSchedules":true,"printScheduleInformation":true,"printComments":true}
47+
""";
48+
MvcResult result = streamPdf(post(ENDPOINT).param("millId", "517").param("year", "2021")
49+
.contentType(MediaType.APPLICATION_JSON).content(selection)
50+
.accept(MediaType.APPLICATION_PDF))
51+
.andExpect(status().isOk())
52+
.andExpect(content().contentType(MediaType.APPLICATION_PDF))
53+
.andReturn();
54+
55+
byte[] pdf = result.getResponse().getContentAsByteArray();
56+
assertThat(new String(pdf, 0, 4)).isEqualTo("%PDF");
57+
58+
String text;
59+
try (PDDocument document = Loader.loadPDF(pdf)) {
60+
text = new PDFTextStripper().getText(document);
61+
}
62+
// Every in-scope section's heading is present despite the aggressive spill...
63+
assertThat(text).contains("Schedule 5: Camp and Access Expense");
64+
assertThat(text).contains("Schedule 6: Road Management Costs");
65+
assertThat(text).contains("Schedule 7A: Bridge Costs");
66+
assertThat(text).contains("Schedule 7B: Culvert Costs");
67+
assertThat(text).contains("Miscellaneous");
68+
assertThat(text).contains("Schedule 11: Basic Silviculture");
69+
// ...and a seeded body value from each, proving real page content survived the swap round-trip.
70+
assertThat(text).contains("Submitted Camp"); // Schedule 5 camp name (517/2021)
71+
assertThat(text).contains("03B"); // Schedule 6 supply block (RMR 8303)
72+
assertThat(text).contains("Harbour Overpass"); // Schedule 7A bridge location (7651)
73+
assertThat(text).contains("Pipe Arch"); // Schedule 7B culvert type desc (7851 'PA')
74+
assertThat(text).contains("CTR-517"); // Schedule 9 contractor id (record 9110)
75+
assertThat(text).contains("Cedar Ridge Reforest"); // Schedule 11 location (V20260816)
76+
}
77+
}

0 commit comments

Comments
 (0)