Take the engine off the path node code imports from, and let it stop
The worker script is handed to the interpreter by path, so app/flow was sys.path[0] for every node: `import queue` got the engine's. It now drops its own directory before anything else imports, and runs with the deployment's credentials scrubbed out of its environment. Also: reload builds off the event loop, the pool wakes what is blocked on it when it stops, a refused metrics flush is kept for the next one rather than dropped, and the cascade events are paired through failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017MeiWk3Yq12n2pTvnQWYvt
This commit is contained in:
@@ -123,11 +123,13 @@ async def read_summary(
|
||||
if errored:
|
||||
problems.append(f"{len(errored)} node(s) failed to load")
|
||||
|
||||
failures = session.exec(
|
||||
statement = (
|
||||
select(func.count())
|
||||
.select_from(EngineEvent)
|
||||
.where(col(EngineEvent.ts) >= _since(24), col(EngineEvent.type) != "audit")
|
||||
).one()
|
||||
)
|
||||
# The health page polls this every ten seconds; the driver is synchronous.
|
||||
failures = await run_in_threadpool(lambda: session.exec(statement).one())
|
||||
|
||||
return HealthSummary(
|
||||
status="degraded" if problems else "ok",
|
||||
|
||||
@@ -302,8 +302,12 @@ class FlowController:
|
||||
for flow in published
|
||||
if not self.store.read_enabled(flow.name)
|
||||
}
|
||||
nodes, loaded, initial_values, flow_inputs = self._build_flows(
|
||||
[(flow, False) for flow in published]
|
||||
# Off the loop: building a python node asks its worker to compile,
|
||||
# which waits for a free slot — and a busy node holds one for as
|
||||
# long as its timeout. On the loop that stalls every request, the
|
||||
# websocket and the health check the container is judged by.
|
||||
nodes, loaded, initial_values, flow_inputs = await run_in_threadpool(
|
||||
self._build_flows, [(flow, False) for flow in published]
|
||||
)
|
||||
|
||||
self.loaded = loaded
|
||||
@@ -708,11 +712,16 @@ class FlowController:
|
||||
for node_def in flow.nodes:
|
||||
member = f"{flow.name}.{node_def.id}"
|
||||
node_type = NODE_TYPES.get(node_def.type)
|
||||
key = (
|
||||
node_type.cls.instance_key(node_def.params)
|
||||
if node_type is not None
|
||||
else None
|
||||
)
|
||||
key = None
|
||||
if node_type is not None:
|
||||
try:
|
||||
key = node_type.cls.instance_key(node_def.params)
|
||||
except Exception:
|
||||
# A plugin's own grouping is not worth the whole graph;
|
||||
# this node just stands on its own.
|
||||
logger.exception(
|
||||
"instance_key failed for node type '%s'", node_def.type
|
||||
)
|
||||
gid = f"{node_def.type}:{key}" if key else member
|
||||
gid_of[member] = gid
|
||||
|
||||
|
||||
@@ -298,18 +298,22 @@ class ExecutionService:
|
||||
}
|
||||
)
|
||||
|
||||
pipeline.apply_outputs(node, item.outputs or None)
|
||||
pipeline.run_downstream(
|
||||
node, entry_id=item.entry_id, replay=item.deliveries > 1
|
||||
)
|
||||
self._publish(
|
||||
{
|
||||
"type": "cascade_finished",
|
||||
"run": item.entry_id,
|
||||
"flow": item.flow,
|
||||
"ts": time.time(),
|
||||
}
|
||||
)
|
||||
try:
|
||||
pipeline.apply_outputs(node, item.outputs or None)
|
||||
pipeline.run_downstream(
|
||||
node, entry_id=item.entry_id, replay=item.deliveries > 1
|
||||
)
|
||||
finally:
|
||||
# Paired, or a cascade that raised — state backend gone, say — is a
|
||||
# run left open until the abandoned sweep ten minutes later.
|
||||
self._publish(
|
||||
{
|
||||
"type": "cascade_finished",
|
||||
"run": item.entry_id,
|
||||
"flow": item.flow,
|
||||
"ts": time.time(),
|
||||
}
|
||||
)
|
||||
return True
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
|
||||
@@ -35,6 +35,13 @@ FLUSH_INTERVAL_S = 15.0
|
||||
DETAIL_CAP = 8000
|
||||
#: A cascade still open this long after it started is never finishing.
|
||||
RUN_STALE_S = 600.0
|
||||
#: ``metric_minute.flow``/``node`` and ``flow_run.flow`` are this wide, and a
|
||||
#: node id has no length of its own. One row over it aborts the transaction,
|
||||
#: which would take every other row in the flush with it.
|
||||
NAME_MAX = 255
|
||||
#: How much unwritten history is held while the database is unreachable. Enough
|
||||
#: for a long outage, bounded so the outage cannot become a memory leak.
|
||||
HOLD_MAX = 10_000
|
||||
#: Retention is checked this often, not on every flush.
|
||||
PRUNE_INTERVAL_S = 3600.0
|
||||
|
||||
@@ -117,8 +124,8 @@ class MetricsCollector:
|
||||
# ponytail: one collector, one row per node per minute; coarsen the
|
||||
# bucket if the node count ever reaches thousands.
|
||||
key = (
|
||||
str(event.get("flow") or ""),
|
||||
str(event.get("node") or ""),
|
||||
str(event.get("flow") or "")[:NAME_MAX],
|
||||
str(event.get("node") or "")[:NAME_MAX],
|
||||
_minute(float(event.get("ts") or time.time())),
|
||||
)
|
||||
return self._buckets.setdefault(
|
||||
@@ -247,7 +254,7 @@ class MetricsCollector:
|
||||
return
|
||||
self._runs[run_id] = {
|
||||
"id": run_id,
|
||||
"flow": str(event.get("flow") or ""),
|
||||
"flow": str(event.get("flow") or "")[:NAME_MAX],
|
||||
"source": str(event.get("cause") or ""),
|
||||
"started_ts": ts,
|
||||
"started_at": datetime.fromtimestamp(ts, timezone.utc),
|
||||
@@ -276,6 +283,7 @@ class MetricsCollector:
|
||||
await asyncio.to_thread(self._write, buckets, pending, runs, prune)
|
||||
except Exception:
|
||||
logger.exception("Could not write engine metrics")
|
||||
self._hold(buckets, pending)
|
||||
return
|
||||
if prune:
|
||||
self._last_prune = time.monotonic()
|
||||
@@ -287,6 +295,36 @@ class MetricsCollector:
|
||||
# to are two events, and a flush can fall between them. One per node,
|
||||
# each replaced by that node's next failure.
|
||||
|
||||
def _hold(
|
||||
self,
|
||||
buckets: dict[tuple[str, str, datetime], dict[str, float]],
|
||||
pending: list[EngineEvent],
|
||||
) -> None:
|
||||
"""Take a batch the database refused back, rather than losing it.
|
||||
|
||||
These are the audit trail and the failures with their tracebacks — the
|
||||
rows nobody can reconstruct afterwards. The batch goes in front of what
|
||||
arrived since, so what a long outage drops is the oldest.
|
||||
"""
|
||||
for key, agg in self._buckets.items():
|
||||
held = buckets.get(key)
|
||||
if held is None:
|
||||
buckets[key] = agg
|
||||
continue
|
||||
for name in SUMMED:
|
||||
held[name] += agg[name]
|
||||
for name in MAXIMA:
|
||||
held[name] = max(held[name], agg[name])
|
||||
self._buckets = buckets
|
||||
# A failed commit puts the rows back to unsaved. One that got as far as
|
||||
# being handed an id keeps it, and the sequence has already moved past
|
||||
# it, so adding them to the next session inserts them cleanly.
|
||||
self._pending[:0] = pending
|
||||
|
||||
for key in list(self._buckets)[: max(0, len(self._buckets) - HOLD_MAX)]:
|
||||
del self._buckets[key]
|
||||
del self._pending[: max(0, len(self._pending) - HOLD_MAX)]
|
||||
|
||||
def _write(
|
||||
self,
|
||||
buckets: dict[tuple[str, str, datetime], dict[str, float]],
|
||||
|
||||
@@ -79,12 +79,18 @@ def ensure_venv() -> None:
|
||||
|
||||
|
||||
def sync(requirements: str) -> tuple[bool, str]:
|
||||
"""Make the venv hold exactly these packages. Returns success and uv's output."""
|
||||
ensure_venv()
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as handle:
|
||||
handle.write(requirements)
|
||||
manifest = handle.name
|
||||
"""Make the venv hold exactly these packages. Returns success and uv's output.
|
||||
|
||||
``uv`` missing, or refusing to make the venv, is a failed apply like any
|
||||
other — the caller answers 400 with what came back, which is the only thing
|
||||
a person can act on.
|
||||
"""
|
||||
manifest = ""
|
||||
try:
|
||||
ensure_venv()
|
||||
with tempfile.NamedTemporaryFile("w", suffix=".txt", delete=False) as handle:
|
||||
handle.write(requirements)
|
||||
manifest = handle.name
|
||||
result = subprocess.run(
|
||||
# Empty means empty: without the flag uv refuses to clear a venv,
|
||||
# so deleting the last line would leave the package installed.
|
||||
@@ -101,8 +107,12 @@ def sync(requirements: str) -> tuple[bool, str]:
|
||||
text=True,
|
||||
timeout=SYNC_TIMEOUT,
|
||||
)
|
||||
except (OSError, subprocess.SubprocessError) as exc:
|
||||
detail = str(getattr(exc, "stderr", None) or exc).strip()
|
||||
return False, f"uv could not run: {detail}"
|
||||
finally:
|
||||
Path(manifest).unlink(missing_ok=True)
|
||||
if manifest:
|
||||
Path(manifest).unlink(missing_ok=True)
|
||||
|
||||
output = (result.stdout + result.stderr).strip()
|
||||
if result.returncode == 0:
|
||||
|
||||
@@ -8,6 +8,7 @@ import threading
|
||||
from collections.abc import Iterable
|
||||
from enum import Enum
|
||||
from typing import TYPE_CHECKING, Any, Literal
|
||||
from urllib.parse import urlsplit, urlunsplit
|
||||
|
||||
import httpx
|
||||
from pydantic import BaseModel, ConfigDict, Field
|
||||
@@ -156,7 +157,15 @@ class HttpNode(Node):
|
||||
# prefixed onto it, so two flows both receiving on "/tick" merge into
|
||||
# one neuron. Key on mode as well if that ever misleads.
|
||||
url = params.get("url")
|
||||
return str(url) if url else None
|
||||
if not url:
|
||||
return None
|
||||
# A URL may legally carry credentials, and this key becomes a group id
|
||||
# in the brain graph — rendered into the response and into the DOM.
|
||||
parts = urlsplit(str(url))
|
||||
if parts.username or parts.password:
|
||||
host = parts.netloc.rsplit("@", 1)[-1]
|
||||
return urlunsplit(parts._replace(netloc=host))
|
||||
return str(url)
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
|
||||
@@ -932,16 +932,20 @@ class Pipeline:
|
||||
"ts": time.time(),
|
||||
}
|
||||
)
|
||||
self.apply_outputs(node, outputs)
|
||||
state = self.run_downstream(node, entry_id=run_id)
|
||||
self._publish(
|
||||
{
|
||||
"type": "cascade_finished",
|
||||
"run": run_id,
|
||||
"flow": node.flow,
|
||||
"ts": time.time(),
|
||||
}
|
||||
)
|
||||
try:
|
||||
self.apply_outputs(node, outputs)
|
||||
state = self.run_downstream(node, entry_id=run_id)
|
||||
finally:
|
||||
# Paired, or a cascade that raised leaves the run open until the
|
||||
# abandoned sweep ten minutes later.
|
||||
self._publish(
|
||||
{
|
||||
"type": "cascade_finished",
|
||||
"run": run_id,
|
||||
"flow": node.flow,
|
||||
"ts": time.time(),
|
||||
}
|
||||
)
|
||||
return state
|
||||
|
||||
def publish(
|
||||
|
||||
@@ -48,8 +48,10 @@ class NodeDef(BaseModel):
|
||||
default=None,
|
||||
gt=0,
|
||||
description=(
|
||||
"Seconds this node's code may run before it is stopped. Above 60 "
|
||||
"the engine may deliver its work again while it is still running."
|
||||
"Seconds this node's code may run before it is stopped. This "
|
||||
"covers the first call's imports, which can be much slower than "
|
||||
"the body. Above 60 the engine may deliver its work again while "
|
||||
"it is still running."
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
@@ -14,12 +14,21 @@ instead of corrupting the reply stream.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sys
|
||||
|
||||
# Handed to the interpreter by path, so CPython puts this file's own directory
|
||||
# — the engine's ``app/flow`` — on ``sys.path[0]``. A node doing ``import
|
||||
# queue`` would then get the engine's queue module rather than the standard
|
||||
# library's. Drop it before anything else can import. (``-P`` and
|
||||
# ``PYTHONSAFEPATH`` do this at startup, but both are 3.11+.)
|
||||
_HERE = os.path.dirname(os.path.abspath(__file__))
|
||||
sys.path[:] = [p for p in sys.path if os.path.abspath(p or ".") != _HERE]
|
||||
|
||||
import contextlib
|
||||
import hashlib
|
||||
import io
|
||||
import json
|
||||
import os
|
||||
import sys
|
||||
import traceback
|
||||
from collections.abc import Callable
|
||||
from types import ModuleType
|
||||
|
||||
+58
-10
@@ -36,6 +36,28 @@ WORKER_MAIN = Path(__file__).with_name("worker_main.py")
|
||||
#: something a person is watching a spinner for.
|
||||
COMPILE_TIMEOUT = 60.0
|
||||
|
||||
#: Environment the worker is not given. ``SECRET_KEY`` decrypts every stored
|
||||
#: secret, not only the ones bound to the node asking.
|
||||
ENV_DENY_PREFIXES = ("POSTGRES_", "FIRST_SUPERUSER", "SENTRY_DSN")
|
||||
ENV_DENY_WORDS = ("PASSWORD", "SECRET", "TOKEN")
|
||||
|
||||
|
||||
def worker_env() -> dict[str, str]:
|
||||
"""The engine's environment with its credentials taken out.
|
||||
|
||||
A denylist, so ``PATH``, ``HOME``, the locale and whatever else a venv
|
||||
needs still reach the worker. This is not a sandbox: the worker is the same
|
||||
user in the same container, with the same filesystem and the same network.
|
||||
It only means node code cannot read the deployment's secrets straight out
|
||||
of its own environment.
|
||||
"""
|
||||
return {
|
||||
key: value
|
||||
for key, value in os.environ.items()
|
||||
if not key.startswith(ENV_DENY_PREFIXES)
|
||||
and not any(word in key.upper() for word in ENV_DENY_WORDS)
|
||||
}
|
||||
|
||||
|
||||
class RemoteError(Exception):
|
||||
"""Something that went wrong inside a worker, re-raised on this side.
|
||||
@@ -85,6 +107,7 @@ class _Worker:
|
||||
stdin=subprocess.PIPE,
|
||||
stdout=subprocess.PIPE,
|
||||
close_fds=True,
|
||||
env=worker_env(),
|
||||
)
|
||||
|
||||
def alive(self) -> bool:
|
||||
@@ -155,6 +178,7 @@ class PythonWorkerPool:
|
||||
# cancel kills the newest. Key by run id if that ever matters.
|
||||
self._running: dict[str, _Worker] = {}
|
||||
self._generation = 0
|
||||
self._stopped = False
|
||||
self._lock = threading.Lock()
|
||||
|
||||
# -------------------------------------------------------------------------
|
||||
@@ -167,12 +191,18 @@ class PythonWorkerPool:
|
||||
self._idle.put(None)
|
||||
|
||||
def stop(self) -> None:
|
||||
self._stopped = True
|
||||
self._generation += 1
|
||||
for worker in list(self._running.values()):
|
||||
worker.kill()
|
||||
for slot in self._drain():
|
||||
if slot is not None:
|
||||
slot.kill()
|
||||
# A node thread can be blocked on a slot right now, and the threads are
|
||||
# not daemons — without a sentinel each it waits for a worker that is
|
||||
# never coming back, and the interpreter cannot exit.
|
||||
for _ in range(self.size):
|
||||
self._idle.put(None)
|
||||
|
||||
def respawn_all(self) -> None:
|
||||
"""Retire every worker, so the next call picks up a changed venv.
|
||||
@@ -201,7 +231,12 @@ class PythonWorkerPool:
|
||||
|
||||
def _acquire(self) -> _Worker:
|
||||
"""Take a slot, blocking while every worker is busy — that is the backpressure."""
|
||||
if self._stopped:
|
||||
raise RemoteError("the worker pool is shutting down")
|
||||
slot = self._idle.get()
|
||||
if self._stopped:
|
||||
# The sentinel stop() put back, so this is a wake-up rather than a slot.
|
||||
raise RemoteError("the worker pool is shutting down")
|
||||
if (
|
||||
slot is not None
|
||||
and slot.alive()
|
||||
@@ -256,7 +291,16 @@ class PythonWorkerPool:
|
||||
|
||||
line = worker.read_line(time.monotonic() + timeout)
|
||||
if line:
|
||||
return dict(json.loads(line))
|
||||
try:
|
||||
return dict(json.loads(line))
|
||||
except (TypeError, ValueError) as exc:
|
||||
# A reply we cannot read leaves this worker out of step:
|
||||
# whatever is still in its pipe would be taken by the next
|
||||
# request on this slot as its own answer. Retire it.
|
||||
worker.cancelled = True
|
||||
raise RemoteError(
|
||||
f"unreadable reply from the worker: {exc}"
|
||||
) from exc
|
||||
if worker.cancelled:
|
||||
raise NodeCancelled("cancelled while it was running")
|
||||
if line is None:
|
||||
@@ -264,9 +308,12 @@ class PythonWorkerPool:
|
||||
raise NodeTimeout(f"exceeded {timeout}s and was killed")
|
||||
raise RemoteError("worker died")
|
||||
finally:
|
||||
if node_id:
|
||||
self._running.pop(node_id, None)
|
||||
self._release(worker)
|
||||
# Under the lock, so a cancel that has already read this worker out
|
||||
# of _running cannot kill it after another node has taken the slot.
|
||||
with self._lock:
|
||||
if node_id:
|
||||
self._running.pop(node_id, None)
|
||||
self._release(worker)
|
||||
|
||||
def compile(self, flow: str, node: str, source: str) -> str | None:
|
||||
"""Load this source in a worker. Returns what to show the author, or None."""
|
||||
@@ -330,12 +377,13 @@ class PythonWorkerPool:
|
||||
|
||||
def cancel(self, node_id: str) -> bool:
|
||||
"""Stop a node that is running now. False when there was nothing to stop."""
|
||||
worker = self._running.get(node_id)
|
||||
if worker is None:
|
||||
return False
|
||||
worker.cancelled = True
|
||||
worker.kill()
|
||||
return True
|
||||
with self._lock:
|
||||
worker = self._running.get(node_id)
|
||||
if worker is None:
|
||||
return False
|
||||
worker.cancelled = True
|
||||
worker.kill()
|
||||
return True
|
||||
|
||||
def _publish(self, event: dict[str, Any]) -> None:
|
||||
if self.events is not None:
|
||||
|
||||
Reference in New Issue
Block a user