Skip to content

prometheus: fix scrape duration growing unbounded (#13667)#13678

Open
PrashantBhanage wants to merge 1 commit into
apache:mainfrom
PrashantBhanage:fix/13667-prometheus-scrape-executor
Open

prometheus: fix scrape duration growing unbounded (#13667)#13678
PrashantBhanage wants to merge 1 commit into
apache:mainfrom
PrashantBhanage:fix/13667-prometheus-scrape-executor

Conversation

@PrashantBhanage

Copy link
Copy Markdown

Description

This PR fixes unbounded growth of Prometheus /metrics scrape duration
against the CloudStack management server (root cause tracked in #13586,
improvements requested in #13667).

Before: the exporter's HttpServer used the JDK default single-threaded
executor, so a slow scrape serialized/queued every other scrape behind it.
Additionally, updateMetrics() had no guard against concurrent or rapid
recomputation, so scrape frequency could multiply backend load
indefinitely. Only a full management server restart reset the growth.

After: three targeted, contained changes, all inside
plugins/integrations/prometheus/:

  1. Bounded HTTP executorPrometheusExporterServerImpl#start() now
    sets an explicit Executors.newFixedThreadPool(2) as the HttpServer's
    executor instead of relying on the JDK default, and shuts it down
    cleanly in stop().
  2. TTL / in-flight guardPrometheusExporterImpl#updateMetrics() is
    now synchronized and checks a lastMetricsUpdateTime timestamp
    against a new dynamic global setting,
    prometheus.exporter.metrics.min.refresh.interval (default 5 seconds).
    Scrapes arriving within that window reuse the previously computed
    metrics instead of triggering a fresh, expensive recomputation.
  3. Timing instrumentationupdateMetrics() now logs its wall-clock
    execution time in milliseconds at info level on every run (including
    on the exception path), so operators/CI can identify which
    sub-collector is slow and confirm the fix closes the growth.

This PR deliberately does not touch AlertManagerImpl or the
capacity-calculation executor in server/ — that code already creates a
fresh thread pool per invocation and reads capacity.calculate.workers
dynamically on main, and rewriting it was flagged in the issue comments
as unnecessary scope creep that introduced its own bugs (stale config,
swallowed shutdown exceptions, thread-pool leaks).

Fixes: #13667
Ref: #13586

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • Major

Screenshots (if appropriate):

N/A — backend-only change, no UI impact.

How Has This Been Tested?

  • Unit tests added to PrometheusExporterImplTest:
    • testUpdateMetricsTTLGuardSkipsSecondCall — two rapid calls to
      updateMetrics() within the TTL window result in only one
      recomputation.
    • testUpdateMetricsTTLGuardAllowsAfterInterval — after the configured
      interval elapses, a subsequent call triggers a fresh recomputation.
  • mvn -pl plugins/integrations/prometheus test → 6/6 tests pass (4
    existing + 2 new).
  • mvn -pl plugins/integrations/prometheus -am install → build success.
  • Manually verified repeated rapid scrapes against /metrics reuse cached
    output within the TTL window, and that logged updateMetrics() duration
    stays flat instead of climbing across successive scrapes.

How did you try to break this feature and the system with this change?

  • Fired concurrent scrape requests against /metrics to confirm the
    bounded executor and synchronized guard prevent overlapping
    recomputation instead of deadlocking or throwing.
  • Verified stop() shuts down the new executor cleanly with no resource
    leak warnings.
  • Confirmed the new prometheus.exporter.metrics.min.refresh.interval
    setting is dynamically adjustable at runtime without a restart, and
    that a value of 0 effectively disables the guard (falls back to
    original per-scrape recomputation behavior) for anyone who wants that.
  • Confirmed no other module (AlertManagerImpl, CapacityManagerImpl,
    etc.) was touched, keeping this change fully isolated to the exporter
    plugin.

Three changes scoped exclusively to the prometheus exporter plugin:

1. Bounded HTTP executor — give the HttpServer a FixedThreadPool(2)
   instead of the JDK default single-threaded executor; shut it down
   cleanly in stop().

2. TTL guard on updateMetrics() — add a synchronized guard with a
   last-run timestamp so scrapes arriving faster than the configurable
   minimum interval (prometheus.exporter.metrics.min.refresh.interval,
   default 5 s) reuse the previously computed metrics instead of
   triggering a new recomputation.

3. Timing instrumentation — wrap updateMetrics() body with
   System.nanoTime() start/end and log elapsed wall-clock time at
   info level on every run (including exception path), so slow
   sub-collectors can be identified from logs.

Fixes: apache#13667
Ref: apache#13586
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.

prometheus improvements

1 participant