akhilpratap1991 commented on PR #71348: URL: https://github.com/apache/airflow/pull/71348#issuecomment-5238670555
Static checks are green on the current head — the initial run was sitting in the first-time-contributor approval gate (`action_required`), not failing; after approval, `CI image checks / Static checks` passed. On the real code path — the concrete trigger is an executor opening and closing a **scoped** session while adopting: 1. `KubernetesExecutor.try_adopt_task_instances()` → completed-pod adoption scoping (added in #66400) runs `with create_session():` — in released providers up to **10.17.x**, which is what Airflow 3.2.2 constraints pin. 2. `create_session()` defaults to the **thread-scoped** registry (`settings.Session`), so it returns the very session `adopt_or_reset_orphaned_tasks()` is holding, and its `finally: session.close()` **expunges everything the orphan query loaded**. 3. Back in the scheduler loop, `prepare_db_for_next_try()` → `TaskInstanceHistory.record_ti()` reads columns deferred by the `load_only` (plus `last_heartbeat_at` / `dag_run.conf` in the adopt branch) on now-detached instances → `DetachedInstanceError`, unhandled in `_run_scheduler_loop`. This is exactly the production incident we hit (3.2.2 + cncf provider 10.17.1, scheduler CrashLoopBackOff): the scheduler dies before the reset flushes, so the same orphan kills every restart. Where the attributes happen to be loaded, the resets are **silently lost** instead — the loop "succeeds" without persisting anything. #67850 already fixed that one call site provider-side (`scoped=False`, released in 10.18.0), and #67822 fixed the first read (`repr`). Both are point fixes: executors are separately released (and can be third-party), and any scoped-session close during adoption re-creates the condition — so this PR makes the scheduler loop itself robust by operating on re-selected, session-bound rows. I've updated the test accordingly: the executor mock now performs the exact operation the released provider does (`with create_session(): pass`) instead of manually expunging. Without this PR's change it fails with the production `DetachedInstanceError` (deferred load of `try_number`); with it, it passes. --- Drafted-by: Claude Code (Opus 4.8); reviewed by @akhilpratap1991 before posting -- 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]
