fix(openmetrics): don't block the scrape on stale collector caches (#10409)

Introduced by 2b7b6ba
See XO-2984

CHANGELOG.unreleased.md

@@ -67,6 +67,7 @@
 - **XO 5**:
   - [Netdata] Fix `You must enable Javascript` error due to CSP blocking Netdata's inline scripts (PR [#10275](https://github.com/vatesfr/xen-orchestra/pull/10275))
 - [MCP] Fix the MCP server crashing at startup on Node 26 with `fetch failed`, and make `search_documentation` honour `HTTP_PROXY`/`HTTPS_PROXY`/`NO_PROXY` like the other requests (PR [#10413](https://github.com/vatesfr/xen-orchestra/pull/10413))
+- [OpenMetrics] A host whose IPMI controller does not answer no longer delays the `/openmetrics/metrics` response until Prometheus gives up with `context deadline exceeded`: power and XOSTOR metrics are served from the last collected values and refreshed in the background (PR [#10409](https://github.com/vatesfr/xen-orchestra/pull/10409))
 
 ### Packages to release
 

docs/docs/manage-your-infrastructure/advanced.md

@@ -548,7 +548,7 @@ Packages watched by `xcp_xostor_package_update_available`: `xcp-ng-linstor`, `xc
 
 All XOSTOR metrics carry `sr_uuid`, `pool_id` and `pool_name`. Per-host metrics also expose `host_uuid` and `host_name`; per-node metrics add the LINSTOR `node_name` (the host's hostname) plus `role` and `state`.
 
-The four collectors don't share a cache. Cluster status and alarms refresh every 60 s, SMART every 5 minutes, updates every hour.
+The four collectors don't share a cache. Cluster status and alarms refresh every 60 s, SMART every 5 minutes, updates every hour. Once a cache has expired, the scrape gets the previous values and the refresh runs in the background, so a slow host never delays the response: a value can lag by one scrape interval. The host power metric (IPMI) is cached and refreshed the same way, every 60 s.
 
 #### Storage technology label (`sr_type`)
 

packages/xo-server-openmetrics/src/index.mts

@@ -21,6 +21,7 @@ import type {
   XoVm,
   XoVmController,
 } from '@vates/types'
+import { coalesceCalls } from '@vates/coalesce-calls'
 import { createLogger } from '@xen-orchestra/log'
 import { fork, type ChildProcess } from 'node:child_process'
 import { dirname, join } from 'node:path'
@@ -367,20 +368,39 @@ interface XostorHealthCheckRaw {
 }
 
 /**
- * Time-based cache with in-flight call coalescing.
+ * Time-based cache that serves stale values while reloading.
  *
- * `get()` returns the cached value while fresh; on miss it invokes the
- * supplied loader once and shares the same in-flight promise with any
- * concurrent caller until the loader settles. Keeps the parent process from
- * issuing redundant XAPI plugin calls when several Prometheus scrapes
- * overlap a cache miss.
+ * `get()` returns the snapshot while fresh. Once expired it still returns
+ * it at once and starts one background reload, shared with any concurrent
+ * caller until the loader settles: a scrape never waits on a slow XAPI
+ * plugin call (a dead BMC makes `ipmitool.py` hang until its timeout).
+ * Only the first load, before any snapshot exists, is awaited and its error
+ * reaches the caller. A failed background reload is logged and drops the
+ * snapshot, so the next call loads in the foreground again.
+ *
+ * Served data can therefore be up to TTL + one scrape interval + load time old.
  */
-class TtlCache<T> {
+export class TtlCache<T> {
+  #name: string
   #ttlMs: number
   #snapshot: { value: T; expiresAt: number } | undefined
-  #inFlight: Promise<T> | undefined
+  #reload = coalesceCalls((load: () => Promise<T>) => {
+    const pending = load().then(value => {
+      this.#snapshot = { value, expiresAt: Date.now() + this.#ttlMs }
+      return value
+    })
+    // a background reload has no caller to reject into: report it here
+    pending.catch((error: unknown) => {
+      if (this.#snapshot !== undefined) {
+        logger.warn('background reload failed, dropping the stale snapshot', { cache: this.#name, error })
+        this.#snapshot = undefined
+      }
+    })
+    return pending
+  })
 
-  constructor(ttlMs: number) {
+  constructor(name: string, ttlMs: number) {
+    this.#name = name
     this.#ttlMs = ttlMs
   }
 
@@ -390,19 +410,8 @@ class TtlCache<T> {
     if (snap !== undefined && snap.expiresAt > now) {
       return snap.value
     }
-    if (this.#inFlight !== undefined) {
-      return this.#inFlight
-    }
-    const pending = load()
-      .then(value => {
-        this.#snapshot = { value, expiresAt: Date.now() + this.#ttlMs }
-        return value
-      })
-      .finally(() => {
-        this.#inFlight = undefined
-      })
-    this.#inFlight = pending
-    return pending
+    const pending = this.#reload(load)
+    return snap === undefined ? pending : snap.value
   }
 }
 
@@ -752,10 +761,10 @@ class OpenMetricsPlugin {
   #lastCpuUsage = process.cpuUsage()
   #eluSamplerInterval: ReturnType<typeof setInterval> | undefined
 
-  #xostorHealthCheckCache = new TtlCache<XostorPayload>(XOSTOR_CACHE_TTL_MS)
-  #xostorSmartCache = new TtlCache<XostorSmartPayload>(XOSTOR_SMART_CACHE_TTL_MS)
-  #xostorUpdatesCache = new TtlCache<XostorUpdatesPayload>(XOSTOR_UPDATES_CACHE_TTL_MS)
-  #hostPowerCache = new TtlCache<HostPowerPayload>(IPMI_POWER_CACHE_TTL_MS)
+  #xostorHealthCheckCache = new TtlCache<XostorPayload>('XOSTOR health', XOSTOR_CACHE_TTL_MS)
+  #xostorSmartCache = new TtlCache<XostorSmartPayload>('XOSTOR SMART', XOSTOR_SMART_CACHE_TTL_MS)
+  #xostorUpdatesCache = new TtlCache<XostorUpdatesPayload>('XOSTOR updates', XOSTOR_UPDATES_CACHE_TTL_MS)
+  #hostPowerCache = new TtlCache<HostPowerPayload>('host power', IPMI_POWER_CACHE_TTL_MS)
 
   constructor(xo: XoApp) {
     this.#xo = xo

packages/xo-server-openmetrics/src/index.test.mts

@@ -4,8 +4,9 @@
 
 import assert from 'node:assert/strict'
 import { describe, it } from 'node:test'
+import { setImmediate } from 'node:timers/promises'
 
-import { configurationSchema, ensureSecret } from './index.mjs'
+import { configurationSchema, ensureSecret, TtlCache } from './index.mjs'
 
 describe('configurationSchema', () => {
   it('does not generate the secret as a schema default', () => {
@@ -46,3 +47,63 @@ describe('ensureSecret', () => {
     assert.deepEqual(persisted, [])
   })
 })
+
+describe('TtlCache', () => {
+  it('waits for the first load', async () => {
+    const cache = new TtlCache<string>('test', 0)
+    assert.equal(await cache.get(async () => 'a'), 'a')
+  })
+
+  it('serves the expired snapshot immediately and reloads in the background', async () => {
+    // ttl 0: every snapshot is expired by the next call
+    const cache = new TtlCache<string>('test', 0)
+    await cache.get(async () => 'a')
+
+    // a hanging loader (e.g. ipmitool.py on a dead BMC) must not delay the caller:
+    // the stale value must come back before the event loop turns
+    const stale = cache.get(() => new Promise(() => {}))
+    assert.equal(await Promise.race([stale, setImmediate('still waiting')]), 'a')
+  })
+
+  it('serves the reloaded value once the reload lands', async () => {
+    const cache = new TtlCache<string>('test', 0)
+    await cache.get(async () => 'a')
+    assert.equal(await cache.get(async () => 'b'), 'a')
+    // let the settled reload run its continuations inside the cache
+    await setImmediate()
+    assert.equal(await cache.get(() => new Promise(() => {})), 'b')
+  })
+
+  it('rejects the caller when the first load fails, then retries', async () => {
+    const cache = new TtlCache<string>('test', 0)
+    await assert.rejects(
+      cache.get(() => Promise.reject(new Error('boom'))),
+      { message: 'boom' }
+    )
+    assert.equal(await cache.get(async () => 'a'), 'a')
+  })
+
+  it('drops the snapshot when a background reload fails, so the next call reports the error', async () => {
+    const cache = new TtlCache<string>('test', 0)
+    await cache.get(async () => 'a')
+    assert.equal(await cache.get(() => Promise.reject(new Error('boom'))), 'a')
+    await setImmediate()
+    await assert.rejects(
+      cache.get(() => Promise.reject(new Error('again'))),
+      { message: 'again' }
+    )
+  })
+
+  it('shares one reload between callers while it is in flight', async () => {
+    let loads = 0
+    const cache = new TtlCache<string>('test', 0)
+    await cache.get(async () => 'a')
+    const load = () => {
+      ++loads
+      return new Promise<string>(() => {})
+    }
+    await cache.get(load)
+    await cache.get(load)
+    assert.equal(loads, 1)
+  })
+})