xchwan opened a new pull request, #75945:
URL: https://github.com/apache/airflow/pull/75945
A single Dag processor parses every Dag bundle, so a heavy or failing bundle
cannot be scaled, sized or scheduled apart from the others. Sets mirror
workers.celery.sets, so each set can override any dagProcessor setting and
parse only the bundles it selects, while bundles stay defined once in
dagBundleConfigList.
related: #61037
<!-- SPDX-License-Identifier: Apache-2.0
https://www.apache.org/licenses/LICENSE-2.0 -->
<!--
Thank you for contributing!
Please provide above a brief description of the changes made in this pull
request.
Write a good git commit message following this guide:
https://chris.beams.io/posts/git-commit/
Please make sure that your code changes are covered with tests.
And in case of new features or big changes remember to adjust the
documentation.
For user-facing UI changes, please attach before/after screenshots (or a
short
screen recording) so reviewers can assess the visual impact.
Feel free to ping (in general) for the review if you do not see reaction for
a few days
(72 Hours is the minimum reaction time you can expect from volunteers) - we
sometimes miss notifications.
In case of an existing issue, reference it using one of the following:
* closes: #ISSUE
* related: #ISSUE
-->
closes: #61037
related: #74849
## Why
The chart runs a single Dag processor Deployment that parses every Dag
bundle. A heavy or failing
bundle cannot be scaled, sized, scheduled or isolated apart from the others,
and every replica shares
one configuration. The core already supports parsing a subset of bundles
(`airflow dag-processor --bundle-name ...`, repeatable), so only the chart
is missing.
This takes over the work started in #61039 (closed after going stale while
waiting for a chart design
direction) and applies the direction agreed in that discussion: follow
`workers.celery.sets` rather than
introducing a parallel configuration tree. The original author is credited
as co-author on the commit.
## What changed compared to #61039
Review feedback on #61039 and how it is handled here:
| Feedback on #61039 | Handling |
|---|---|
| Two parallel lists (bundles and `deployPerBundle.bundleOverrides`) can
drift apart | Bundles are defined once in `dagProcessor.dagBundleConfigList`; a
set selects them with `bundleNames` and the chart fails when a name is not
defined |
| Do not expose yet another `args` parameter | No new `args` knob;
`--bundle-name` flags are appended to the last item of the existing
`dagProcessor.args` |
| Drop the extra `deploymentOverride` level and model it on
`workers.celery.sets` | `dagProcessor.sets` / `enableDefault` mirror the worker
sets values and reuse the same merge helper (`workersMergeValues`) and naming
scheme |
| Wait for Airflow 2 removal / Chart 2.0 so the structure can be simplified
| Implemented on current `main`, after the `workers` deprecation removal and
sets simplification (#66671) |
| A single Dag processor should still be possible next to dedicated ones |
`enableDefault` (default `true`) keeps the existing Deployment; sets are
additive |
The #61039 implementation (helper templates `dag-processor.deployment` /
`dag-processor.poddisruptionbudget`,
`deployPerBundle`, `bundleOverrides`) is not reused; this PR is a fresh
implementation on the sets pattern.
## What it does
- New `dagProcessor.enableDefault`, `dagProcessor.bundleNames` and
`dagProcessor.sets` values (with schema).
- Each item of `sets` requires a `name` and can override any `dagProcessor`
value. It renders its own
Deployment `<release>-dag-processor-<name>` and, when enabled, its own
PodDisruptionBudget
`<release>-dag-processor-pdb-<name>`, selected by a `dag-processor-set`
label.
- `bundleNames` is validated against `dagBundleConfigList` and turned into
`--bundle-name` flags.
- A set cannot define its own `dagBundleConfigList` (the Airflow config is
shared by all Dag processors, so
the override would be silently ignored); the chart fails with an
explanation instead.
- With defaults (`enableDefault: true`, `sets: []`) the rendered Deployment
and PodDisruptionBudget are
identical to `main`.
- Docs: new section in `chart/docs/manage-dag-files.rst`.
Not included: a per-set `image` (requested in #74849). It is a separate
change on top of this structure.
Note: the default Dag processor parses all bundles unless `bundleNames` is
set on it, so when using sets
set `enableDefault: false` (or `bundleNames` on the default one) to avoid
parsing a bundle twice. This is
documented in `values.yaml` and the docs.
## Testing
Executed:
- New `chart/tests/helm_tests/airflow_core/test_dag_processor_sets.py` (14
tests): default output, naming
with/without default, set labels and selectors, value overrides,
`--bundle-name` args (single, multiple,
on the default processor), args untouched without `bundleNames`, all
bundles still present in the shared
config, failures (unknown bundle, missing `name`, `dagBundleConfigList` in
a set), per-set PDB, disabled
Dag processor.
- Together with the existing `test_dag_processor.py` and
`test_worker_sets.py`: 236 tests passed.
- `prek run --stage pre-commit` on the changed files: all hooks passed.
- `helm template` output of the Dag processor Deployment and
PodDisruptionBudget with default values,
compared against `main`: no differences.
- Render time (`helm template`, clean chart directory): about 0.75s on
`main`, 0.9s with this change and
default values, 0.95s with 10 sets and 1.2s with 30 sets (about 10ms per
set).
- Manual check on a local kind cluster (Airflow 3.3.2, two
`LocalDagBundle`s, one set per bundle): the
Deployments, PodDisruptionBudgets and `--bundle-name` args were as
expected, and the logs of each Dag
processor show it only refreshing and scanning its own bundle.
Not verified:
- Dag runs and the UI end to end. On the resource-limited local cluster the
API server and scheduler
did not become ready, so only the Dag processors were checked.
- The complete Helm unit test suite, and other Kubernetes versions than the
test default.
No newsfragment: the feature is opt-in and the chart only takes
newsfragments for major or breaking changes.
Dag runs and the UI end to end. My local machine could not run the full
chart stably: Docker is capped at 8 CPUs (12 GB memory after raising it), and a
single-node kind cluster running the migration job, API server, scheduler and
several Dag processors at once saturated the CPU. The startup/liveness probes
(each of which starts a full airflow process) timed out and kept restarting the
API server and scheduler, even after relaxing the probe timeouts and dropping
the log-groomer sidecars. The Dag processors themselves stayed up, so only they
were checked (Deployments, PodDisruptionBudgets, args and logs). A run on a
machine with more CPU would be welcome.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code (Sonnet 5.5) following [the
guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
---
* Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information. Note: commit author/co-author name and email in commits
become permanently public when merged.
* For fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
* When adding dependency, check compliance with the [ASF 3rd Party License
Policy](https://www.apache.org/legal/resolved.html#category-x).
* For significant user-facing changes create newsfragment:
`{pr_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
You can add this file in a follow-up commit after the PR is created so you
know the PR number.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
