Repository navigation
Run Dag processors without metadata database access - #74794
ephraimbuddy wants to merge 18 commits into
Conversation
adf1913 to
8c7b7a2
Compare
ea486c9 to
74ccd2d
Compare
74ccd2d to
573c296
Compare
573c296 to
6dbe8d3
Compare
6909890 to
6185d02
Compare
|
Now also has conflicts |
6185d02 to
ea388da
Compare
Resolved |
| api_request_failed: bool = attrs.field(default=False, init=False) | ||
| """Whether the Dag processor could not answer a request of this process through the Execution API.""" | ||
|
|
||
| def _from_frame(self, frame) -> ToDagProcessor | None: |
There was a problem hiding this comment.
api_request_failed is only set once a response is decoded, so an import that times out while its request is still waiting never sets it. During an outage that wait can be long: the manager only reads child requests in _service_processor_sockets, so a request sits while the loop's own heartbeat, claims, publication and ack each wait up to execution_api_timeout, and other children's handlers go first. Once dagbag_import_timeout fires, python_importer.py catches the AirflowTaskTimeout as an import error, the child exits 0, and _finish_callback_attempt acknowledges the callback without running it. Could a request still waiting for its response count as failed too, e.g. a flag set before the write in send/asend and cleared in _from_frame?
There was a problem hiding this comment.
Unanswered requests now count as API failures for both send and asend, and only their matching responses clear them. An import timeout therefore retries the callback batch when none of its targets loaded, within the existing retry deadline. Regression tests cover interrupted imports and response-ID matching.
| try: | ||
| self.work_source.flush(deadline=math.inf) | ||
| except Exception: | ||
| self.log.exception("Unable to acknowledge requested work before exiting") |
There was a problem hiding this comment.
flush() only catches httpx2.HTTPError, but a 403 job_closed comes out of request() as DagProcessorRegistrationRetired. So when the registration retires with acks still queued, which is likely after an outage, this logs a full traceback for the case run_with_api already logs as a warning when client.restart_required is set. Could this do the same?
There was a problem hiding this comment.
Retired registrations now produce a warning without a traceback during shutdown. Unexpected errors still retain their traceback. The test checks both paths; mutation testing confirmed that replacing log.exception with log.error fails the traceback assertion.
| - how often to refresh DAGs from those sources | ||
| - how many files to parse concurrently | ||
| options: | ||
| execution_api_token_file: |
There was a problem hiding this comment.
Long term this should be how the processor always runs, like workers. Workers get their token from the scheduler inside the workload, though, and nothing trusted launches the processor, so it has to bring its own credential. Could API mode be its own setting (off by default for now) with the token file as one credential source, so a projected service-account token or a workload-identity exchange can be added later without becoming another mode switch?
@ashb what do you think?
There was a problem hiding this comment.
I have a PR on workload identity: #74700
There was a problem hiding this comment.
Added [dag_processor] use_execution_api, defaulting to False, independently of execution_api_token_file. A token file alone keeps database mode. Future credential sources can use the same switch, and #74796 now uses it for probes, rollout strategy and replica validation.
There was a problem hiding this comment.
I am wondering though -- do we even need to keep DB mode at all?
There was a problem hiding this comment.
I was thinking we should only delete it in a major. I can yank it if we want to do it now. Solves a lot of problem including better handling of running callback in workers
Standalone processors need to resolve bundle credentials and publish parsed metadata without inheriting database or signing credentials. API-managed Jobs also need lifecycle and requested-work recovery so transient outages do not lose callbacks or priority parses.
Delayed claim responses and repeated reparses must not consume newer work or lose the request that triggered a later import. API-mode processors also need consistent callback delivery semantics and bounded cleanup when recovering from interrupted work.
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.
Callbacks queued behind an active import must still run, and an unavailable bundle must not starve priority work from healthy bundles. Operators also need consistent heartbeat signals and clear credential requirements when moving processors to the Execution API.
…ry tests Recovered files can finish independently after the healthy bundle's work. The regression must require every claim to be acknowledged exactly once without imposing an ordering between separate priority parses.
Claim retries and acknowledgments need a single delivery owner so manager scheduling can evolve without changing callback order or releasing priority work before publication completes.
Transient API failures and health-file errors must not discard requested work or stop an otherwise recoverable processor. Retirement diagnostics must explain when ownership was lost without completing a replaced Job. Typed callbacks must retain independent claim identities, and invalid heartbeat thresholds must fail before registration starts parsing. These changes depend on the callback model and timeout helper in layer 4.
Regression coverage must remain aligned with failed-parse publication and bundle refresh backoff so delivery guarantees stay verifiable as the stack evolves. Callback diagnostics also need protection against accidental metadata database access in API mode. Validation: 804 tests passed, 5 PostgreSQL-only tests skipped; regular static checks passed.
A stop signal can arrive while the manager answers a parse subprocess's request with that file's parse credential. Acknowledging finished work from the signal handler sent those requests with the same credential, so the API refused them and the callbacks ran again once the Job ended. Acknowledgments now go out from the runner's cleanup with the Job credential, and a second signal during that cleanup can no longer skip the final kill of child processes and the acknowledgments. The shutdown signal also reaches child processes through the process group, so only a callback whose process exited cleanly counts as finished; one that the signal stopped is delivered again. Provisioning the token file again under a new session retires the running processor's registration while its Job is still open. Skipping completion after a retirement left that Job's claimed callbacks and priority requests unavailable until scheduler cleanup ended it. An ended or replaced Job is expected to refuse that completion, which is not worth a traceback, but a completion failure after a successful run must still reach the caller as the documentation promises. A bundle whose inventory publication was rejected cannot dispatch until its inventory is retried. Claiming its priority requests in the meantime held capacity that healthy bundles needed, and the claims were then acknowledged without a parse. A claimed priority parse whose failure was held back within stale_dag_threshold kept its claim until the file's next scheduled parse, which never came once the file had reached max_runs, so a failed parse now acknowledges the claim and one that met an Execution API failure is parsed again. A priority request for a file the processor could not stat, such as one in a directory whose permissions changed, stopped the processor and every bundle it served. The per-loop acknowledgment flush, the claim request bounds, the rejection of a callback claim without its request, max_runs waiting for outstanding requested work and the health probe flags could regress without failing any test. The removed CLI test covered a flag that was never released. One callback dispatch test now passes its parse source, because the processor-creation helper requires it once the source consistency changes earlier in this stack land.
The security guide said that dag-processor-sync-bundles --bundle-name selects bundles, but the option only checks the names and skips deactivating catalog entries. The guide and the stale_dag_threshold reference implied that the threshold always applies in API mode, although an accepted parse publishes atomically and the API marks Dags missing from it stale at once; only the members of a failed import get the grace period. The guide also called publication receipts checkpoints, and its shutdown sentence did not say that callbacks stopped by the signal are delivered again. The isolation test's docstring said CI skipped it, but every core integration job runs it, and it did not say that the tests skip on SQLite. Parsing rejects a task whose executor is not in the processor's own configuration. The Compose example's processor starts from a fresh configuration, so it failed to import Dags whose tasks name the deployment's executor, such as the quick-start's CeleryExecutor.
A parse process's Execution API requests are not retried, and the Task SDK answers a failed one as a missing value. One gateway 503 on an import-time Variable.get could keep the Dag from loading; the callback runner logged that and exited normally, so the callback was acknowledged and never delivered again. Such callbacks now run again under their claim, with the backoff and job_heartbeat_timeout expiry used for callbacks whose bundle cannot be prepared, measured from the first failure. A callback that crashes or times out without an API failure is still acknowledged: it reached user code, and running it again would repeat that code's side effects. While a callback waited for its bundle version to be prepared, it counted against max_callbacks_per_loop, so a burst of them for one bundle stopped every other bundle from claiming callbacks until they expired. A failed bundle listing released the bundle's priority claims, and acknowledging them deletes the requests, so a transient listing error lost the reparses users asked for. A priority parse that fails is now reported at once, so its publication settles the claim, and the file stays in the regular parse rotation. A DNS failure and a gateway 503 produced the same claim or acknowledgment warning, so operators could not tell a network problem from an unavailable API server.
Any failed Execution API request in a callback process caused all of its callbacks to run again. The failed request could come from the callback itself, or from an import-time Variable.get whose default still let the Dag load. So callbacks that had already sent their alerts ran again, up to about 11 times within the default job_heartbeat_timeout when the failure persisted, and one failed request repeated every callback in the batch. Only callbacks that never reached user code can run again without repeating side effects. The manager cannot see which callbacks ran, and a new message to it would change the supervisor schema that the Go, Java and TypeScript SDKs generate code from. A process now exits with a dedicated status when none of its callbacks found its Dag or task, and only that status after an API failure is retried. For the same reason, a clean exit at shutdown is acknowledged even after an API failure.
A callback process whose Dags or tasks did not load exited with status 75 so the manager could retry it after an Execution API failure. It also did so in database mode, and in API mode when a Dag had simply been removed, where no retry follows. The supervisor logs every non-zero exit as abnormal, and these processes used to exit 0. Only a failed Execution API request can make running the callbacks again worthwhile, so the status is now kept for that case. The job_heartbeat_timeout description and the security docs said a callback whose Dag or task did not load is retried. That only happens when none of the callbacks processed together for the file found its target: when one did, the process cannot say which ones ran without a new supervisor message, which would change the schema the other language SDKs generate code from. The acknowledgment flush and the Job heartbeat each decided by hand which failed Execution API requests are worth resending. They now use the shared rule, so it cannot drift from the other callers.
The Dag processor isolation test looked up its own container with docker inspect on the hostname. The kerberos integration gives the Breeze container the hostname krb5-machine-example-com, which names no container, so the test failed in the kerberos integration job. Docker mounts /etc/hostname from the container's own directory, which carries the container ID whatever the hostname is.
A slow Execution API could stop requested work from ever reaching a processor. Claims and acknowledgments gave up after a second, but the API assigns claimed work before it responds, and the replayed claim was just as slow. Heartbeats kept succeeding, so the processor looked ready while no callback or reparse ran. The manager answers its parsing processes' API requests one at a time, and each can wait for the full Execution API timeout. The health file was only refreshed once per loop, so an outage with several processes reading Variables at import could fail the liveness probe. A callback process only noticed a failed API request made through the synchronous path, so a Dag file that read a Variable with asyncio at import exited normally, and its callback was acknowledged instead of retried. In API mode the API server, not the processor, reads the stale-Dag threshold, which its description did not say. The remaining changes share helpers that had been copied between modules, and keep each test next to the code it exercises.
Retain callback claims when import-time API requests are interrupted, and resolve archive-member priority claims after the refreshed inventory arrives. Keep database-mode callback completion unchanged. Separate API mode from the token-file credential source, clarify probe and staleness behavior, handle registration retirement during shutdown without a traceback, and consolidate the requested-work test fixtures. The catalog command's --bundle-name never narrowed the sync; it only kept other bundles active, so a host that configures some of the bundles now asks for that with --keep-unconfigured-bundles. A failed parse whose import-error report cannot be built no longer holds its priority request until the file is parsed again. Slow Dag imports can exhaust the processor timeout before a callback runs. Acknowledging those requests loses callbacks, while retrying batches that may already have produced side effects can duplicate them. A burst of callbacks for one unavailable bundle version should not multiply the same expensive preparation failure or delay unrelated healthy work.
ea388da to
6907ed4
Compare
| def _make_frame(self, msg: ToManagerMessage): | ||
| frame = super()._make_frame(msg) | ||
| self._pending_request_ids.add(frame.id) | ||
| return frame | ||
|
|
||
| def _from_frame(self, frame) -> ToDagProcessor | None: | ||
| # Match IDs so another response cannot hide an interrupted sync or async request. | ||
| self._pending_request_ids.discard(frame.id) | ||
| try: | ||
| return super()._from_frame(frame) | ||
| except AirflowRuntimeError as error: | ||
| if error.error.error == ErrorType.API_SERVER_ERROR: | ||
| self._received_api_error = True | ||
| raise |
There was a problem hiding this comment.
Would it be better for the Task SDK to expose this instead, e.g. a public CommsDecoder.has_failed_requests? Overriding here would mean the dag processor will need to be in the same package as the rest of the SDK. It’s a choice, but I’m not sure we want to commit yet.
|
|
||
|
|
||
| class DagCallbacksStarted(BaseModel): | ||
| """Request permission to begin a callback batch after importing its Dag file.""" |
There was a problem hiding this comment.
Is this docstring stale? The server does not seem to do anything about permission.
Enable the complete opt-in processor API mode: registration, heartbeats, completion, callback and priority delivery, local health probes, and CLI configuration. The branch includes its prerequisite layers and can run the processor without metadata database credentials or database network access.
Layer 6 of the seven-part replacement for #74233, in GitHub native stack #74792. Depends on #74793. Base:
dag-processor-http-manager.GitHub's Files changed tab shows this layer. Compare this layer.
Merge order:
Replaces #74233 while preserving this activation layer as part 6. This PR builds on and reuses substantial parts of Kaxil Naik's implementation in #67878.
Closes #67878.
Was generative AI tooling used to co-author this PR?
Generated-by: Codex (GPT-6) following the guidelines