Cut the round trips a message costs the engine
Measured with `make bench-engine` against a real Redis: 103.6 -> 164.4 messages a second on a five-node chain (p50 latency 2125 -> 1171 ms) and 34.8 -> 63.2 on a fan-out of twenty. Against the memory backend, which is what a pip install runs on, 262 -> 626. The two that bought most of it: - `StateBackend.record` puts a published value, its timestamp, its series and its version counter in one round trip. They were four calls building four pipelines, and a value crossing an edge pays them twice. A released rate-limit hold rides along instead of a DEL per port. - the readiness check reads a node's inputs and hands them to the node, rather than reading the triggering ones to count them and having the node read the same keys again a moment later. `apply_outputs` was a second copy of `_record_outputs` and is now the same code plus the event that distinguishes it. The rest, each small: - `_derive` builds a node-by-id map and a `consumes` index, so dispatching an item and publishing a value stop scanning every node in the installation. - `read_all` is memoised against the store revision — it sits on the publish path, so a dashboard slider was reading and validating every flow file per value. Same mechanism `_wiring` already uses. - the `message_value` source block is built once per node instead of per emission. - both timer threads ask the queue to promote only when something is actually due, which takes an idle engine from ~4 Redis round trips a second to one. - the shared httpx client is bounded (32 connections, one retry); its default pool is 100 with no per-host cap, so one slow endpoint could take it and every other sender node with it. - the MQTT and delay nodes no longer log a line per message at INFO. Robustness, in the same pass: - `MemoryWorkQueue._done` was a set nothing ever removed from — one entry per non-idempotent node per item, for the life of the process, in the default configuration. Capped, the way the Redis side expires its markers. - a saturated engine can claim from the due lane past the cascade limit. The capacity gate sits in front of the claim, so the due lane's priority — decided inside it — did not apply while every slot was held: a motor's stop was not behind the long nodes, it was unread. Only after a slot has genuinely failed to free for half a second, and briefly, so the backlog is not starved in turn. - `reclaim_stale` dispatches through that same gate. It could return sixty entries and push in-flight far past the limit the gate exists to hold. - a flow's nodes are stopped together rather than one after another. Each gets `NODE_STOP_TIMEOUT`, so a flow whose broker was unreachable took five seconds per node — long enough to outlast `REBUILD_WAIT` and 503 the deploy. - the worker pool and the HTTP client are closed on a thread, not on the event loop, and a run closes the state backend it built (on Redis, a client and a connection pool per run). - the five background tasks say something when they die. Each catches exceptions inside its loop, so one raised anywhere else left the engine serving with no metrics, no alerts or no artifact sweep, silently. `tests/flow/test_round_trips.py` counts the state operations one message costs — four, where it was about eleven — because none of the above would fail a behavioural test if it were undone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01M6hPWS6YEbT1P8LxhhFb2T
This commit is contained in:
@@ -41,6 +41,13 @@ TOUCH_INTERVAL_S = 20.0
|
||||
# resolution of a delay: a delayed item is waited for exactly, so what a timer
|
||||
# fires late by is a wake-up and a promotion rather than up to a whole second.
|
||||
DELAYED_INTERVAL_S = 1.0
|
||||
# Cascade slots a promoted timer may use past `max_cascades`. A due item was
|
||||
# already waited for, so making it queue behind whatever long node happens to
|
||||
# hold the pool is the one lateness the sleeping timer thread cannot remove.
|
||||
DUE_RESERVE = 2
|
||||
# How long the saturated engine waits on the due lane before going back to
|
||||
# check whether a cascade slot has freed.
|
||||
DUE_CLAIM_BLOCK_MS = 200
|
||||
#: How many cascades may be in flight, unless the service is given a number.
|
||||
#: Sustained throughput is this over the mean cascade time, so an installation
|
||||
#: whose nodes wait on a network rather than a CPU may want more of them —
|
||||
@@ -172,11 +179,17 @@ class ExecutionService:
|
||||
if not self._intake.is_set():
|
||||
self._intake.wait(timeout=0.5)
|
||||
continue
|
||||
free = self._await_capacity()
|
||||
free, due_only = self._await_capacity()
|
||||
if not free:
|
||||
continue
|
||||
try:
|
||||
items = self.queue.claim(free, CLAIM_BLOCK_MS)
|
||||
items = self.queue.claim(
|
||||
free,
|
||||
# Briefly, in the due-only case: this is the saturated
|
||||
# engine, and a slot freeing has to be noticed promptly.
|
||||
DUE_CLAIM_BLOCK_MS if due_only else CLAIM_BLOCK_MS,
|
||||
due_only,
|
||||
)
|
||||
failures = 0
|
||||
except Exception as exc:
|
||||
failures += 1
|
||||
@@ -189,7 +202,7 @@ class ExecutionService:
|
||||
for item in items:
|
||||
self._dispatch(item)
|
||||
|
||||
def _sleep_until_due(self) -> None:
|
||||
def _sleep_until_due(self) -> bool:
|
||||
"""Wait for the soonest deadline, the housekeeping cap, or a new one.
|
||||
|
||||
A fixed poll here made every delayed item late by 0-1000ms whatever the
|
||||
@@ -209,6 +222,10 @@ class ExecutionService:
|
||||
due = None
|
||||
wait = DELAYED_INTERVAL_S if due is None else due - time.time()
|
||||
self._timer_wake.wait(min(max(wait, 0.0), DELAYED_INTERVAL_S))
|
||||
# What the caller promotes for: the deadline this woke for has passed,
|
||||
# or the read failed and it should look anyway. An idle engine reads
|
||||
# `next_due` once a second and asks for nothing.
|
||||
return due is None or due <= time.time()
|
||||
|
||||
def _tick(self) -> None:
|
||||
"""Promote delayed items, and take back what a dead engine dropped."""
|
||||
@@ -216,11 +233,12 @@ class ExecutionService:
|
||||
last_touch = 0.0
|
||||
last_backlog = 0.0
|
||||
while not self._stop.is_set():
|
||||
self._sleep_until_due()
|
||||
promote = self._sleep_until_due()
|
||||
if self._stop.is_set():
|
||||
break
|
||||
try:
|
||||
self.queue.move_due(time.time())
|
||||
if promote:
|
||||
self.queue.move_due(time.time())
|
||||
except Exception as exc:
|
||||
logger.error("Could not promote delayed work: %s", exc)
|
||||
# The item is still due, so the wait above would be zero and
|
||||
@@ -255,6 +273,12 @@ class ExecutionService:
|
||||
item.node,
|
||||
item.deliveries,
|
||||
)
|
||||
# Through the same gate the main loop uses: a reclaim can
|
||||
# return sixty-odd entries at once, and dispatching them
|
||||
# all would push `_inflight` far past `max_cascades` —
|
||||
# exactly the overcommit the gate exists to prevent.
|
||||
if not self._await_capacity()[0]:
|
||||
break
|
||||
self._dispatch(item)
|
||||
except Exception as exc:
|
||||
logger.error("Could not reclaim stale work: %s", exc)
|
||||
@@ -291,8 +315,8 @@ class ExecutionService:
|
||||
}
|
||||
)
|
||||
|
||||
def _await_capacity(self) -> int:
|
||||
"""How many cascades may be claimed now. Zero means the service stops.
|
||||
def _await_capacity(self) -> tuple[int, bool]:
|
||||
"""How many cascades may be claimed now, and whether only due ones.
|
||||
|
||||
Claiming past what the pool can run makes nothing faster: the extra
|
||||
items queue up inside the pool, count as in flight and hold their
|
||||
@@ -300,11 +324,29 @@ class ExecutionService:
|
||||
came to report hundreds busy on a healthy engine. Work left in the
|
||||
stream is work that is still anyone's to take; work that is claimed is
|
||||
work that is actually being run.
|
||||
|
||||
The gate sat in front of the claim, though, and the due lane's
|
||||
priority is decided *inside* it — so with every slot held by a long
|
||||
node, a motor's stop was not merely behind them, it was unread. Past
|
||||
the limit this therefore keeps claiming, from the due lane alone:
|
||||
a promoted timer is work that was already waited for, and there are
|
||||
only ever as many of them as there are deadlines.
|
||||
"""
|
||||
with self._inflight_lock:
|
||||
while self._inflight >= self.max_cascades and not self._stop.is_set():
|
||||
self._inflight_lock.wait(0.5)
|
||||
return 0 if self._stop.is_set() else self.max_cascades - self._inflight
|
||||
while not self._stop.is_set():
|
||||
free = self.max_cascades - self._inflight
|
||||
if free > 0:
|
||||
return free, False
|
||||
# Only once a slot has genuinely failed to free: the due lane
|
||||
# is usually empty, and going to look at it ahead of waiting
|
||||
# would leave the backlog unclaimed for the length of that
|
||||
# read every time the pool filled up.
|
||||
if self._inflight_lock.wait(0.5):
|
||||
continue
|
||||
reserve = self.max_cascades + DUE_RESERVE - self._inflight
|
||||
if reserve > 0:
|
||||
return reserve, True
|
||||
return 0, False
|
||||
|
||||
def _dispatch(self, item: WorkItem) -> None:
|
||||
with self._inflight_lock:
|
||||
|
||||
Reference in New Issue
Block a user