Skip to content

Fix Prometheus scrape allocations and bound JFR recording - #756

Merged
ar merged 2 commits into
mainfrom
fix/issue-755
Aug 30, 2026
Merged

Fix Prometheus scrape allocations and bound JFR recording#756
ar merged 2 commits into
mainfrom
fix/issue-755

Conversation

@ar-agt

@ar-agt ar-agt commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

What Problem This Solves

Idle Q2 instances can accumulate humongous Prometheus scrape allocations in Old Gen, expose an oversized TM operation histogram, and let the always-on JFR repository grow without bounds.

Why This Change Was Made

  • stream Prometheus exposition directly to the HTTP response using chunked transfer encoding instead of building and copying a large String
  • disable Micrometer's automatic percentile histogram for jpos.tm.op, retaining the six explicit SLO buckets
  • cap the default JFR recording at 250 MiB and four hours
  • encode the small status response once with UTF-8

User Impact

Prometheus scrapes no longer create repeated full-payload byte arrays, TM-bearing instances expose a substantially smaller metrics document, and Q2's default JFR repository remains bounded during long uptimes.

Evidence

  • ./gradlew :jpos:test --tests org.jpos.q2.Q2Test --no-daemon
  • ./gradlew :jpos:test --no-daemon
  • regression test verifies that TM operation timers expose only the six configured SLO buckets plus +Inf

Fixes #755

@ar

ar commented Aug 30, 2026

Copy link
Copy Markdown
Member

Non-blocking observation on testTMOperationHistogramUsesOnlyServiceLevelObjectives: it asserts through the process-global registry. Q2.meterRegistry is Metrics.globalRegistry, and every Q2 constructed anywhere in the test JVM runs registerMicroMeter(), which clear()s that shared registry and stacks one more copy of the TM_OPERATION filter plus one more PrometheusMeterRegistry member onto it. The test passes today because the timer is registered and scraped inside one method, but it turns order-sensitive the moment another test constructs a Q2 between registration and assertion (several tests in Q2Test do construct extra instances), or registers a conflicting jpos.tm.op meter.

Two levels of improvement:

  • Test-local hardening (cheap, could go in this PR): tag the timer with a unique per-run value (e.g. a UUID) and remove the meter in a finally, so nothing leaks into the shared registry for later tests.
  • Actual fix (follow-up material, not this PR): give each Q2 instance its own CompositeMeterRegistry instead of mutating Metrics.globalRegistry from the constructor path. That makes registerMicroMeter() idempotent per instance, stops filter/member accumulation when multiple Q2s live in one JVM (embedded users, not just tests), and lets tests assert against a fully isolated registry.

@ar-agt

ar-agt commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator Author

Addressed in 2688018. The regression test now uses a per-run UUID tag and removes its timer from the shared registry in a finally, avoiding collisions and meter leakage between tests.

I kept per-Q2 registry ownership out of this PR as suggested; that broader change needs a separate compatibility and lifecycle discussion.

Verification:

  • ./gradlew :jpos:test --tests org.jpos.q2.Q2Test --no-daemon
  • ./gradlew jpos:check --no-daemon

@ar
ar merged commit 3bd4af0 into main Aug 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Idle Q2 instances park at max heap and leak /tmp: oversized Prometheus scrape, non-streaming exposition, unbounded default JFR recording

2 participants