Parallelise enumeration of MCP Features

[Steve-Mcl]

Description

Current logic in the MCP Features enumeration routine (see mcp/features endpoint in forge/ee/routes/expert/index.js) loops the candidate MCP servers to check they are online.

This should be Parallelised to reduce compound impact from a slow responding instance or device. There are other considerations:

MCP Features enumeration — parallelisation

TL;DR

The MCP features endpoint checks each candidate instance/device for liveness one at a time (pass 2), so one slow or unreachable instance holds up all the others (worst case N × timeout). We should make these checks run concurrently but bounded. While we're in here, pass 3 should switch to allSettled so a single device timeout can't sink the whole response (the new Device.status check reduces how often that happens, but doesn't remove it).

Context

• Pass 2 is sequential and does the timeout prone calls: instance.liveState() (hosted), deviceComms.sendCommandAwaitReply(..., { timeout: 3000 }) (device), and getOrCreateToken(). Worst case latency is N × timeout - an unreachable instance stalls everything behind it.
• Pass 3 (the feature fetch) is already Promise.all parallel, so it already carries some of the risks below.

What should be looked at

• Convert pass 2 from a sequential for loop to bounded concurrency execution (batch/pool, e.g. 10–20 in flight) instad of an unbounded Promise.all
• Keep each parallel task self-contained with its own try..catch (or use allSettled) so one slow/failing instance is skipped, never rejecting the whole batch.
• Harden the existing pass 3 Promise.all to allSettled for the same reason (at scale, the probability of timeout will sink the whole response).
• Add an overall request deadline (AbortController) - return partial results with an explicit "unchecked/skipped" list rather than waiting for the slowest chain.
• Consider a per-request instance cap (backstop) - query the first N instances, report the remainder as not-checked.

Why this should be considered

• DB connection pool exhaustion - liveState() and getOrCreateToken() touch the DB; Sequelize's pool is relevant here. Unbounded concurrency queues on pool and can hit acquire timeout errors - thrashing, not parallelism.
• Broker / comms pressure - e.g. 1000 simultaneous sendCommandAwaitReply = 1000 in-flight MQTT requests + pending reply-handlers - risk of overwhelming the broker and the pending-reply map.
• Container driver / k8s API rate limits - liveState() on hosted instances hits the driver / k8s API.
• Batch size vs tail latency - with concurrency C and timeout T, worst case ≈ ceil(N/C) × T. At N=1000, C=20, T=3s that's ~150s - so concurrency alone is not enough at 1000; the overall deadline + partial results are required.
• Timeout behaviour of sendCommandAwaitReply rejects On timeout it rejects with Error('Command timed out') (forge/comms/devices.js), and rejects on other error paths too.
  • Pass 2 is safe today - the await is inside a per-iteration try/catch { continue }.
  • Pass 3 has a latent failure: the rejecting promises feed a bare Promise.all with no per-item catch, so a single device timeout rejects the whole batch and fails the endpoint. Switching pass 3 to allSettled (or per-item .catch()) is required, not optional.
  • Partially mitigated by the new Device.status virtual column: pass 1 now skips devices whose status is not online (i.e. lastSeenAt older than 30 min), so known-stale devices never reach pass 3 and can't trigger the timeout-driven batch rejection. This shrinks the problem but doesnt remove it altogether. A device that is online (seen within 30 min) but unresponsive right now will still time out. i.e. a good argument for allSettled.

Epic/Story

No response

Have you provided an initial effort estimate for this issue?

I have provided an initial effort estimate