Skip to content

Commit cc01bcc

Browse files
JackKellyclaude
andcommitted
Add principle 14: couple through data at rest, not run status
Production jobs read whatever is on disk when they run and record how stale it was. None of them asks whether the job producing its input succeeded, or ran at all. That is why a failed ingest makes the 06:00 forecast slightly staler rather than absent. The property was already implemented and already argued for in one place (dropping the promoted_model lineage edge), but the docs never stated it, and two places framed it as a defect awaiting a fix: the TODO on power_time_series_and_metadata_schedule proposed a run-status sensor that would only fire live_forecasts once the ingest had succeeded, and the Airflow comparison table listed "run-status coordination" as planned work. Both would have introduced exactly the cascade the design avoids. live_forecasts wants to run at 00/06/12/18 regardless, so there is no acceleration to be had from a sensor there. Sensors are still wanted on the ingest assets, where they save booting Fargate for a no-op fetch; that is what #324 is actually for, and the table row now says so. Note the asymmetry that makes this work: the lineage edge stays. It is what builds a developer's inputs on a laptop. What it must never become is a runtime gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 66c3d3c commit cc01bcc

5 files changed

Lines changed: 63 additions & 17 deletions

File tree

docs/architecture/why-dagster-not-airflow.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ around the Dagster UI and would need rewriting.
200200
| `add_output_metadata` tables, asset catalog, lineage | every asset | Asset-event `extra` JSON (2.10+) in the events list | Partial — raw JSON, no rendered tables or history plots |
201201
| Asset checks — non-blocking WARN, attached to an asset, dedicated Checks view (`power_data_is_fresh`, `nwp_has_no_unexpected_nulls`) | power ingest, `ecmwf_ens` | Data-quality as ordinary tasks (`common.sql` check operators; Great Expectations / Soda / dbt-test); no first-class check primitive or Checks UI, blocking by default (as of 3.3.0) | Partial — the capability exists as tasks; the non-blocking severity and check-status surface do not |
202202
| `EcsRunLauncher` (laptop = subprocess, cloud = Fargate, switched by `dagster.yaml`) | control plane | ECS executor (Amazon provider, Fargate launch type) | Exists; per-*task* rather than per-run granularity |
203-
| Sensors / run-status coordination (planned, [#324](https://github.com/openclimatefix/nged-substation-forecast/issues/324)) | ingest → forecast ordering | Asset-triggered DAGs, event-driven scheduling | Parity, arguably cleaner in Airflow |
203+
| Data-arrival sensors (planned, [#324](https://github.com/openclimatefix/nged-substation-forecast/issues/324)) | firing the *ingest* jobs only when there is new data to fetch, to avoid booting Fargate for a no-op | Asset-triggered DAGs, event-driven scheduling | Parity, arguably cleaner in Airflow |
204204

205205
The asset-checks row is worth one extra sentence, because the gap there is architectural rather
206206
than cosmetic: non-blocking WARN checks are the *mechanism* by which this service stays

docs/design-philosophy/design-principles.md

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ principle behind it is a claim we are merely hoping comes true.
7373
check in the repo is non-blocking `WARN`; there is deliberately no `ERROR`-severity check
7474
anywhere. *Serves:* [Hypothesis 1: a service that mostly runs
7575
itself](engineering-hypotheses.md#h1-a-service-that-mostly-runs-itself). *Detail:* [Inherent
76-
Stability](inherent-stability.md), whose [ten rules](inherent-stability.md#the-rules) are the
76+
Stability](inherent-stability.md), whose [eleven rules](inherent-stability.md#the-rules) are the
7777
fine-grained form of this principle together with the complexity-offline and strict-contracts
7878
principles.
7979

@@ -401,6 +401,42 @@ principle behind it is a claim we are merely hoping comes true.
401401
[When would a REST API earn its keep?](../architecture/forecast-delivery.md#when-would-a-rest-api-earn-its-keep),
402402
[Considered but rejected designs](../architecture/production-deployment.md#considered-but-rejected-designs).
403403

404+
14. **Production jobs are coupled through data at rest, never through run status.** Each scheduled
405+
production job reads whatever is on disk at the moment it runs, and records how stale that input
406+
turned out to be. No production job asks whether the job that produces its input succeeded, or
407+
ran at all.
408+
The common alternative is a chain of scheduled jobs — A at 06:00, B at 06:15, C at 06:30 — in
409+
which B's real input is *the event of A having run*. That design has no way to distinguish "A
410+
failed" from "A is still running" from "A had nothing to do", so one bad morning upstream takes
411+
out every job downstream of it for the rest of the day, and recovery means replaying the chain in
412+
order.
413+
Ours cannot propagate a failure because there is no channel for it to propagate down.
414+
`live_forecasts` does not care whether `ecmwf_ens` succeeded in the last 24 hours, or whether
415+
this hour's telemetry pull ran: it selects the freshest NWP run genuinely present as of its own
416+
init time and stamps the row with how old that was. A failed ingest makes the 06:00 forecast
417+
slightly staler. It cannot make it late, and it cannot make it absent.
418+
Note the deliberate asymmetry with the *lineage* graph, which is a different thing from a runtime
419+
precondition. Dagster still knows that `live_forecasts` depends on `ecmwf_ens` and
420+
`power_time_series_and_metadata`; that edge is what lets a developer materialise one asset on a
421+
laptop and have its inputs built for them, and what makes the graph legible in the UI. What we
422+
decline is letting that edge become a gate in production.
423+
*Without it:* one failed 06:00 ingest suppresses the 06:00 forecast, and then the 12:00 and 18:00
424+
ones behind it; NGED receive nothing at all rather than something slightly stale, which
425+
[inverts the whole degradation ladder](inherent-stability.md#the-degradation-ladder); and someone
426+
has to re-run the chain in order, out of hours, to catch up — the precise out-of-hours
427+
intervention that
428+
[T1.1](engineering-hypotheses.md#h1-a-service-that-mostly-runs-itself) predicts will never be
429+
needed.
430+
*Decided:* the three production jobs run on three independent schedules with no run-status
431+
coupling between them, and the `:55` offset on the telemetry pull is an optimisation for
432+
freshness, not a precondition — if it is missed, the forecast still runs. `promoted_model` was
433+
deliberately *removed* from `live_forecasts`' deps once it became clear the model arrives by
434+
filesystem path rather than by data flow: declaring the edge bought nothing at runtime and
435+
created a permanently un-materialised parent on the production box, which has no MLflow and never
436+
runs promotion. *Serves:*
437+
[Hypothesis 1: a service that mostly runs itself](engineering-hypotheses.md#h1-a-service-that-mostly-runs-itself).
438+
*Detail:* [Inherent Stability](inherent-stability.md#the-rules), rule 11.
439+
404440
## Deliberately absent
405441

406442
We have **no availability service-level objective (SLO) and no error

docs/design-philosophy/inherent-stability.md

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,8 +151,8 @@ Nothing here is a 2am page. The uptime posture that makes that acceptable is arg
151151
## The rules
152152

153153
These are the imperative form of everything above: the checklist to follow when in doubt while
154-
changing production code. It is deliberately self-contained, so three of the ten restate a
155-
[design principle](design-principles.md) rather than adding anything new. Those three are marked
154+
changing production code. It is deliberately self-contained, so four of the eleven restate a
155+
[design principle](design-principles.md) rather than adding anything new. Those four are marked
156156
below — if you change one, change its matching principle too. The remaining seven are specific to
157157
degradation and appear nowhere else.
158158

@@ -185,6 +185,12 @@ degradation and appear nowhere else.
185185
10. **Damp the corrections.** Bounded retries with backoff, rate limits on retraining and hysteresis
186186
on model promotion (the latter two designed but not built 🚧) are as much a part of this
187187
principle as the degradation ladder is.
188+
11. **Never make one production job's run status a precondition for another's.** Couple them through
189+
data at rest: read whatever is on disk, record how stale it was, and carry on. A dependency in
190+
the *lineage* graph is fine and useful — it is what builds a developer's inputs on a laptop — but
191+
it must never become a runtime gate, because a gate turns one failed upstream run into a missing
192+
forecast, which is rung 0 of the ladder reached by the one route the ladder cannot catch. *(The
193+
coupling-through-data-at-rest principle, in imperative form.)*
188194

189195
## Where complexity should live
190196

docs/live_service/operations.md

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -76,11 +76,13 @@ then point the task definition at the new tag — see
7676

7777
Once a model is promoted, `live_forecasts` produces a new forecast automatically every 6 hours —
7878
at 00:00, 06:00, 12:00, and 18:00 UTC — via `live_forecasts_schedule`. `power_time_series_and_metadata`
79-
(a separate, hourly-scheduled job `live_forecasts` depends on but isn't ordered against) is
80-
itself scheduled 5 minutes *before* each hour so that hour's pull has landed by the time
81-
`live_forecasts` ticks — a cheap mitigation, not a guarantee; see
82-
`power_time_series_and_metadata_schedule`'s docstring (`defs/schedules.py`) for the more rigorous
83-
fix still to explore. This needs the Dagster daemon running (see
79+
(a separate, hourly-scheduled job `live_forecasts` depends on but is deliberately not ordered
80+
against) is itself scheduled 5 minutes *before* each hour so that hour's pull has landed by the
81+
time `live_forecasts` ticks. That offset is an optimisation for freshness, not a precondition: if
82+
the pull fails or runs long, the forecast still goes out on time against whatever is already on
83+
disk, and records how stale that input was — see
84+
[Inherent Stability, rule 11](../design-philosophy/inherent-stability.md#the-rules). This needs the
85+
Dagster daemon running (see
8486
[Prerequisites](#prerequisites-a-running-dagster-instance) above) to fire on time.
8587

8688
To materialise one 6-hourly slot yourself — e.g. right after promoting a model, so you don't have

src/nged_substation_forecast/defs/schedules.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,14 @@
2828
)
2929
"""Fires at :55 past every hour — 5 minutes *before* the top of the hour — so this hour's pull
3030
has landed by the time ``live_forecasts_schedule`` ticks at 00/06/12/18 UTC.
31-
``live_forecasts`` depends on ``power_time_series_and_metadata`` (``defs/production_assets.py``)
32-
but the two run as separate jobs on separate schedules, so nothing actually enforces the
33-
ordering; this offset is a cheap mitigation, not a guarantee.
3431
35-
TODO: explore a more rigorous fix — e.g. a run-status sensor that only fires
36-
``live_forecasts_job`` once that hour's ``power_time_series_and_metadata_job`` has actually
37-
succeeded, rather than assuming a fixed offset is always enough."""
32+
``live_forecasts`` declares ``power_time_series_and_metadata`` as a dep, but the two run as
33+
separate jobs on separate schedules and nothing enforces the ordering at runtime. That is
34+
deliberate, not a gap: the offset is an optimisation for freshness, and if it is missed —
35+
because this pull failed, or ran long — ``live_forecasts`` still runs on time against whatever
36+
is already on disk, and records how stale that input was. Making the ordering a precondition
37+
would convert one failed ingest into a missing forecast, which is the failure mode the whole
38+
design exists to avoid. See ``docs/design-philosophy/inherent-stability.md``, rule 11."""
3839

3940
ecmwf_ens_job = define_asset_job(
4041
"ecmwf_ens_job",
@@ -69,5 +70,6 @@ def ecmwf_ens_schedule(context: ScheduleEvaluationContext) -> RunRequest:
6970
live_forecasts_schedule = build_schedule_from_partitioned_job(live_forecasts_job)
7071
"""Ticks at 00/06/12/18 UTC, materialising the just-completed window with default run config
7172
(``availability_mode="live"``) — the schedule is always live; replays are manual, launched from
72-
the UI with ``availability_mode="replay"``. See ``power_time_series_and_metadata_schedule``'s
73-
docstring above for how the two schedules' relative timing is (loosely) coordinated."""
73+
the UI with ``availability_mode="replay"``. This slot fires on the clock regardless of whether
74+
the ingest jobs succeeded; see ``power_time_series_and_metadata_schedule``'s docstring above for
75+
why the two schedules are deliberately not ordered against each other."""

0 commit comments

Comments
 (0)