diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index 39022faa9d..e6c9909d42 100644 --- a/CHANGELOG.unreleased.md +++ b/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 diff --git a/docs/docs/manage-your-infrastructure/advanced.md b/docs/docs/manage-your-infrastructure/advanced.md index d024384cc5..ce0e9663fe 100644 --- a/docs/docs/manage-your-infrastructure/advanced.md +++ b/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`) diff --git a/packages/xo-server-openmetrics/src/index.mts b/packages/xo-server-openmetrics/src/index.mts index 5e1e5e5d3c..84572faba5 100644 --- a/packages/xo-server-openmetrics/src/index.mts +++ b/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 { +export class TtlCache { + #name: string #ttlMs: number #snapshot: { value: T; expiresAt: number } | undefined - #inFlight: Promise | undefined + #reload = coalesceCalls((load: () => Promise) => { + 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 { 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 | undefined - #xostorHealthCheckCache = new TtlCache(XOSTOR_CACHE_TTL_MS) - #xostorSmartCache = new TtlCache(XOSTOR_SMART_CACHE_TTL_MS) - #xostorUpdatesCache = new TtlCache(XOSTOR_UPDATES_CACHE_TTL_MS) - #hostPowerCache = new TtlCache(IPMI_POWER_CACHE_TTL_MS) + #xostorHealthCheckCache = new TtlCache('XOSTOR health', XOSTOR_CACHE_TTL_MS) + #xostorSmartCache = new TtlCache('XOSTOR SMART', XOSTOR_SMART_CACHE_TTL_MS) + #xostorUpdatesCache = new TtlCache('XOSTOR updates', XOSTOR_UPDATES_CACHE_TTL_MS) + #hostPowerCache = new TtlCache('host power', IPMI_POWER_CACHE_TTL_MS) constructor(xo: XoApp) { this.#xo = xo diff --git a/packages/xo-server-openmetrics/src/index.test.mts b/packages/xo-server-openmetrics/src/index.test.mts index 122ebc14b7..1bd131bd15 100644 --- a/packages/xo-server-openmetrics/src/index.test.mts +++ b/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('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('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('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('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('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('test', 0) + await cache.get(async () => 'a') + const load = () => { + ++loads + return new Promise(() => {}) + } + await cache.get(load) + await cache.get(load) + assert.equal(loads, 1) + }) +})