Skip to content

Keep Dag parse results tied to their source snapshot - #74789

Merged
ephraimbuddy merged 11 commits into
mainfrom
dag-processor-source-consistency
Nov 6, 2026
Merged

ephraimbuddy merged 11 commits into
mainfrom
dag-processor-source-consistency

Conversation

@ephraimbuddy

@ephraimbuddy ephraimbuddy commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Capture bundle version metadata when a parse starts, defer refreshes while the current source is in use, and discard results from superseded sources. Failed discovery preserves existing definitions and import errors for the affected source instead of treating them as deleted.

While a refresh waits, the bundle's files are held back and return to the front of the queue afterwards. A failed refresh pauses parsing for that bundle until a refresh succeeds. New metrics: dag_processing.files_awaiting_source and dag_processing.results_discarded_on_refresh.

Layer 2 of the seven-part replacement for #74233, in GitHub native stack #74792. Depends on #74788. Base: dag-processor-fab-validation.

GitHub's Files changed tab shows this layer. Compare this layer.

Merge order:

  1. FAB validation
  2. Source consistency
  3. Publication API
  4. Durable work delivery
  5. HTTP manager
  6. Complete processor API mode
  7. Helm and Compose deployment

Was generative AI tooling used to co-author this PR?
  • Yes — Codex (GPT-6), Claude Code (Opus 5.5)

Generated-by: Codex (GPT-6) and Claude Code (Opus 5.5) following the guidelines

@ephraimbuddy
ephraimbuddy added this pull request to stack #74792 October 15, 2026 10:04
@ephraimbuddy
ephraimbuddy marked this pull request as ready for review October 15, 2026 14:57
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from bffe26c to 81e0b40 Compare October 16, 2026 15:35
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from 445893d to cfb451e Compare October 20, 2026 10:56
Base automatically changed from dag-processor-fab-validation to main October 20, 2026 14:15
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch 2 times, most recently from 1f4ec6a to 9b7dfd8 Compare October 20, 2026 17:21
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from 9b7dfd8 to 562a387 Compare October 22, 2026 16:58
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch 3 times, most recently from 49ece41 to 9e9ecf2 Compare October 26, 2026 15:19
Comment thread providers/git/docs/changelog.rst Resolved
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from 9e9ecf2 to 8e824a6 Compare October 28, 2026 20:16
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

The re-run of the jobs stopped by EC2 Spot interruptions finished: attempt 2 passed.

@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from 8e824a6 to ef49963 Compare October 29, 2026 16:55
@ephraimbuddy
ephraimbuddy requested a review from kaxil October 30, 2026 09:18
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from ef49963 to 56b6f88 Compare October 30, 2026 15:40

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this layer reads well after the two review rounds. Checked the generation-based discard/requeue, the drain-before-refresh only for refresh_changes_source bundles (so LocalDagBundle never waits), the cooldown on refreshes that fail with an intact source, keeping Dags and import errors for sources whose discovery failed, and the git provider's getattr probe for DagBundleRefreshError so it stays compatible with older cores. Approving; leaving the rebase and merge order of the stack to you.


Drafted-by: Claude Code (Claude Opus 5); reviewed by @potiuk before posting

@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch 2 times, most recently from 1eccbc6 to 9bc77b1 Compare November 5, 2026 13:11
Refreshing a bundle while imports still use its files can associate Dag
results with the wrong version or persist results from a replaced source.
Discovery failures also cannot establish that previously observed Dag
definitions have been deleted.
Holding a bundle's files back until its in-flight imports finish made
reparse requests lose their place in the queue, let a steady stream of
callbacks keep the refresh waiting indefinitely, and gave operators no
sign that files were held back. Checking every bundle on each loop while
one waited also added a state query per bundle per loop.

A parse that crashed after its bundle re-initialized says nothing about
the new source, and copying version metadata on every dispatch grows
with manifest size. Files parsed more often than max_runs were queued
again, so `airflow dag-processor --num-runs` could never finish.
Fresh CI workers can reach this test before the first 300-second interval has elapsed. The test needs a due refresh followed by a retry within the same interval to cover import draining reliably.
Safe refresh failures should not stop ordinary Dag parsing or consume a
reparse request before fresh source is available. Callbacks and finite
processing runs must preserve that distinction, and metrics must remain
meaningful while files wait for a usable source.

Known limitation: the Git provider's Airflow 3.4 version gate still assumes
DagBundleRefreshError exists in the installed core. Compatibility with
unpatched 3.4 core requires a follow-up before publication.
Some 3.4 core builds predate the refresh-error contract even though they
satisfy the provider's version gate. Git bundle configuration must remain
usable with those supported cores and retain the original fetch error
when the contract is unavailable.
The refresh-error contract is not available in every Airflow 3.4 core build, so provider guidance must describe capability detection. Logger values in the callback source regression only need identity, not mock behavior.
Temporary bundle outages should not delay unrelated parsing or hide its metrics, and credential failures must not discard an intact checkout. Provider compatibility guidance must reflect the features available in the installed core.
When a bundle's refresh succeeded but listing its files or recording the
refresh then failed, the bundle was due again at the next check, and a
bundle that changes files on refresh drains its active parses before
each refresh. A persistent failure, such as a recursive symlink in a Git
bundle, therefore cut that bundle's parse throughput at every refresh
check for as long as it lasted. Those retries now wait for the bundle's
refresh interval, but only when a retry would drain parses. A bundle
with nothing listed yet, or one whose refresh never changes files, still
retries at the next check, so a failed first listing does not hide its
Dags for the whole interval.

Passing the failed discovery sources as a new argument to the
import-error cleanup made manager subclasses that override it with its
original signature crash on their first successful listing, the same
kind of break #64245 fixed for Dag deactivation. The cleanup keeps its
original signature and reads the failed sources the manager records for
the bundle, as Dag deactivation already does.

In multi-team deployments, team-filtered dashboards rely on the
team_name tag that every other per-bundle Dag processing metric carries,
so the per-bundle count of files awaiting a source now carries it too.
Starting a processor now requires the parse source, because the dispatch
loop always chooses it first and that choice also decides whether the
file must wait; the fallback bypassed that decision and its result was
discarded as stale in a running processor.

The Dag bundle guide now says that object-store bundles pause on every
refresh failure, that the retry delay applies only when the provider
reports an intact source or when listing or recording a successful
refresh fails, and that a processor started with --num-runs waits for a
paused bundle, which can otherwise look like a hang. Tests now cover
the pause warning, the failed sources reaching Dag deactivation, a
skip-refresh override on a bundle's first refresh, and a regular
duplicate of a running file.
The provider supports every Airflow 3 core, and cores from 3.0 onwards
can lack the refresh-error class, not only some 3.4 builds. The import
test patched an Airflow version the provider no longer reads, so its two
cases were identical, and its version-based skip kept it from running on
the older cores where the probe matters most.
The retry after a failed listing or state write is held for the
bundle's refresh interval rather than capped by it, and only a failed
listing keeps the earlier file list, so the caveat misdescribed both.

Pausing a bundle's parsing while its refresh fails changes what S3, GCS
and Azure Blob Storage bundle users saw before, and a processor started
with --num-runs can now wait for that refresh to succeed. Upgraders need
to read that before they upgrade.
@ephraimbuddy
ephraimbuddy force-pushed the dag-processor-source-consistency branch from 9bc77b1 to 4552a6b Compare November 6, 2026 09:07
@ephraimbuddy
ephraimbuddy merged commit 887f6d2 into main Nov 6, 2026
176 checks passed
@ephraimbuddy
ephraimbuddy deleted the dag-processor-source-consistency branch November 6, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants