Skip to content

Commit d238a18

Browse files
committed
Describe the per-candidate fast path in ADR-0013
The ADR still described the fast path that compared each Dag file's stored params and required an unchanged candidate set, which was unsound across files. With the whole answer recorded per artifact, a candidate is skipped when its own fingerprint matches, a new artifact is probed by the first file that lists it, and Java JARs are recognised by their cache digest rather than by Main-Class.
1 parent d851bdf commit d238a18

2 files changed

Lines changed: 29 additions & 35 deletions

File tree

‎airflow-core/adr/lang-sdk/0013-persisted-task-handler-bindings.md‎

Lines changed: 27 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,7 @@ For `task_handler_bindings`, `None` and `[]` mean different things, and the diff
305305
`persist_parsing_result` already uses ([`persist_parsing_result`](https://github.com/apache/airflow/blob/79991cd4db0c9346a28b23c453377f6df0c6b4ed/airflow-core/src/airflow/dag_processing/manager.py#L1361-L1364)).
306306
Without it, a transient parse failure would silently wipe every binding the file owns.
307307

308-
On a fast-path skip the child **re-emits the bindings it was given**, unchanged. It does not omit
309-
them. Request and result carrying the same information makes that a copy rather than a special
310-
"keep these" signal the reconcile could get wrong.
308+
A candidate the fast path skips is answered from its recorded `task_handlers`, so the child resolves and validates every stub task of the file on every parse, and a list always holds all of the file's bindings. There is no separate "keep these" signal for the reconcile to get wrong.
311309

312310
**Scheduler → worker.** `ExecuteTask` and `StartupDetails` each gain one optional reference. The
313311
artifact bundle is a second, independent bundle, so it needs its own `BundleInfo`.
@@ -381,15 +379,19 @@ DagFileProcessorProcess(etl.py) [no DB — client con
381379
│ not a silent Python fallback at execution time
382380
│
383381
├─4─ per coordinator: list candidates in its bundle
384-
│ Go → files carrying the AFBNDL01 trailer magic
385-
│ Java → *.jar with a Main-Class manifest attribute
386-
│ TS → *.min.mjs with a valid //# airflowBundle= layout header
382+
│ Go → files carrying the AFBNDL01 trailer magic, whatever
383+
│ their executable bit (one without it is rejected)
384+
│ Java → *.jar whose manifest carries Airflow-Cache-Digest
385+
│ (dependency JARs may declare Main-Class too)
386+
│ TS → *.min.mjs whose first line starts with //# airflowBundle=
387+
│ (one with an invalid layout line is rejected)
387388
│ walk order is deterministic, so conflicts reproduce
388389
│
389390
├─5─ FAST PATH, per candidate (see "The fast path")
390-
│ size + cache_digest match known_artifacts, and the candidate
391-
│ set is unchanged ──▶ skip the launch, echo the bindings
392-
│ anything differs ──▶ probe
391+
│ size + stored cache_digest equal a known artifact's
392+
│ ──▶ skip the launch, use its task_handlers
393+
│ anything else, or no stored digest ──▶ probe
394+
│ rejected by the listing ──▶ import error, no probe
393395
│
394396
├─6─ PROBE, per differing candidate — one subprocess
395397
│ LangSDKTaskHandlerProcessorProcess.start(
@@ -400,17 +402,19 @@ DagFileProcessorProcess(etl.py) [no DB — client con
400402
│ ◀─SDKTaskHandlerParsingResult(task_handlers={"etl": […]})── runtime
401403
│ Get* from the runtime is relayed up ToManager unchanged
402404
│
403-
├─7─ VALIDATE per dag_id, unioned across coordinators
405+
├─7─ VALIDATE per dag_id, against the recorded and the fresh answers,
406+
│ unioned across coordinators
404407
│ task_id sets must match exactly
405-
│ arg_bindings[*] ↔ handler_params[*], per the declaration's binding:
408+
│ arg_bindings[*] ↔ declaration.params[*], per its binding:
406409
│ by position, or by folded or exact name
407-
│ arg_bindings[*].schema ↔ handler_params[*].value_schema,
410+
│ arg_bindings[*].schema ↔ declaration.params[*].value_schema,
408411
│ compared only where neither is null
409412
│ two candidates claiming one (dag_id, task_id) → import error
410413
│ naming both paths
411414
│
412415
└─8─ on success → task_handler_bindings=[…]
413416
on mismatch → import_errors[etl.py]=… AND bindings=None
417+
either way → probed_artifacts=[every fresh answer]
414418
│
415419
├── DagFileParsingResult(serialized_dags=[…], import_errors={…},
416420
▼ task_handler_bindings=[…] | [] | None,
@@ -533,32 +537,24 @@ stored digest, so one cannot substitute for the other.
533537

534538
Validation is expensive (one subprocess per candidate) and a file is re-parsed every
535539
`[dag_processor] min_file_process_interval` seconds, 30 by default. Re-probing unchanged artifacts
536-
every 30 seconds forever is not acceptable, so the probe is skipped when nothing relevant changed.
540+
every 30 seconds forever is not acceptable, so a candidate is not probed while its recorded answer still holds.
537541

538-
Two tiers, cheapest first:
542+
The probe asks for every handler and the answer depends only on the artifact, so the answer recorded with a fingerprint holds for every Dag file that later sees that fingerprint, whichever file probed it. Each candidate is decided on its own, cheapest check first:
539543

540544
```
541545
for each candidate in the coordinator's bundle:
542-
stat(candidate).st_size ≠ known.size_bytes → PROBE
543-
read stored cache_digest ≠ known.cache_digest → PROBE
544-
otherwise → SKIP, echo the binding
546+
rejected by the listing (unusable artifact) → REPORT, never probe
547+
no known artifact at (bundle, relative path) → PROBE
548+
stat(candidate).st_size ≠ known.size_bytes → PROBE
549+
stored cache_digest missing, or ≠ known.cache_digest → PROBE
550+
otherwise → SKIP, use known.task_handlers
545551
```
546552

547-
`mtime` is deliberately absent. It is reset by an object-store download and by container rebuilds, so
548-
it produces churn without adding certainty; size plus digest is sufficient, with size acting only as
549-
a free pre-filter.
553+
`mtime` is deliberately absent. It is reset by an object-store download and by container rebuilds, so it produces churn without adding certainty. Size plus digest is sufficient: the stored digest is read, not recomputed, so an artifact edited in place without a repack keeps it, and the free size check catches most such edits. Execution's integrity check stays the safety net.
550554

551-
Two conditions beyond the per-file comparison:
555+
**A new artifact is probed by the first file that lists it.** It has no recorded answer yet. Its answer lists every Dag it registers, so a collision on `(dag_id, task_id)` with any file's Dag is found when that file is next validated, even when another file probed the artifact first.
552556

553-
**The candidate set must be unchanged.** A newly deployed artifact has no known fingerprint, so a set
554-
that differs from `known_artifacts` forces a probe. Without this, an added artifact that collides on
555-
`(dag_id, task_id)` would never be detected, and the conflict rule above would be unenforceable.
556-
557-
**The Python side is re-validated regardless.** The skip avoids the *subprocess*, not the comparison.
558-
`handler_params` is stored precisely so a changed `.py` (a stub task that gained an argument) is
559-
compared against the cached declaration in process. Skipping the comparison as well would cache a
560-
verdict for a signature that no longer exists, and ship the mismatch to a worker as a runtime
561-
argument error instead of catching it as an import error.
557+
**The Python side is re-validated regardless.** The skip avoids the *subprocess*, not the comparison. The recorded `task_handlers` are kept precisely so a changed `.py` (a stub task that gained an argument) is compared against the cached declaration in process. Skipping the comparison as well would cache a verdict for a signature that no longer exists, and ship the mismatch to a worker as a runtime argument error instead of catching it as an import error.
562558

563559
### Failure handling
564560

@@ -595,9 +591,7 @@ definition whose author can act) naming both artifact paths, since the fix is in
595591
([`_calculate_classpath`](https://github.com/apache/airflow/blob/79991cd4db0c9346a28b23c453377f6df0c6b4ed/task-sdk/src/airflow/sdk/coordinators/java/coordinator.py#L85-L87)), so all handlers in a bundle
596592
share one dependency graph. Isolating conflicting dependency versions requires a second bundle, a
597593
second coordinator instance, and a second queue. This needs documenting.
598-
- Steady-state parsing costs no subprocesses. A cold start (empty tables, or a bundle whose
599-
candidate set changed) costs one subprocess per changed candidate per coordinator, shared across
600-
all Dag files in that parsing loop through `known_artifacts`.
594+
- Steady-state parsing costs no subprocesses. A new or changed artifact costs one subprocess for each Dag file parsed before its answer is recorded, at most the children the manager starts in one loop iteration, and none after that. A Dag that fails validation does not cause a probe on every parse, since the answers it was checked against stay recorded, and an artifact that stores no cache digest is probed on every parse.
601595
- No `DagVersion` coupling. An artifact rebuild does not bump a Dag's version, and a Dag edit does
602596
not invalidate an artifact fingerprint.
603597
- Mixed-language stays Python-primary. A Lang-SDK runtime cannot declare stub tasks, and a native Dag

‎airflow-core/adr/lang-sdk/0014-bundle-metadata-and-cache-digest.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,8 @@ No `dag_id` appears anywhere in it, and no `task_id`.
8888
Candidate *detection* is unaffected and still requires executing nothing — it is exactly the "this is
8989
an Airflow Lang-SDK artifact" marker doing its job: the `AFBNDL01` trailer magic for Go
9090
([`Magic`](https://github.com/apache/airflow/blob/79991cd4db0c9346a28b23c453377f6df0c6b4ed/go-sdk/internal/bundlefooter/footer.go#L57)), the `.min.mjs` suffix plus a valid layout header for
91-
TypeScript ([`BUNDLE_SUFFIX`](https://github.com/apache/airflow/blob/79991cd4db0c9346a28b23c453377f6df0c6b4ed/task-sdk/src/airflow/sdk/coordinators/node/coordinator.py#L43)), a `Main-Class` attribute
92-
for Java.
91+
TypeScript ([`BUNDLE_SUFFIX`](https://github.com/apache/airflow/blob/79991cd4db0c9346a28b23c453377f6df0c6b4ed/task-sdk/src/airflow/sdk/coordinators/node/coordinator.py#L43)), an `Airflow-Cache-Digest` manifest attribute
92+
for Java (dependency JARs in the same bundle may declare `Main-Class` too).
9393

9494
The entrypoint source region stays, for display.
9595

0 commit comments

Comments
 (0)