Thanks Vincent, that makes sense. Dropping the 72-hour framing, let's just treat this as an open discussion. Appreciate the correction.
Joel On Thu, 22 Oct 2026 at 15:17, Vincent Beck <[email protected]> wrote: > It is either a discuss thread or a lazy consensus but not both :) If you > want to start a lazy consensus, then the thread subject should call it out > with [LAZY CONSENSUS] instead of [DISCUSS]. Though, in this case I think a > discuss thread makes more sense since many people do not have the context > and 72 hours is a bit rough to make an opinion :) > > On 2026-10-22 18:56 UTC Joel Emmanuel wrote: > > Hi all, > > > > I'm an MLH Fellowship participant working on this with Dennis Ferruzzi > > (mentor), posting here per his guidance before any implementation starts. > > > > Problem > > > > /api/v2/monitor/health already reports per-instance liveness for > Scheduler, > > Triggerer, and DagProcessor. Each registers itself as a row in the Jobs > > table (airflow.jobs.job.Job) with a job_type, hostname, and a > > latest_heartbeat that updates on a timer; the health check queries > > unfinished rows of that job_type and reports which ones are still > > heartbeating. This is the per-instance shape PR #70416 (Jung-Hyun Andrew > > Kim) added, with Vincent Beck's #73222 building on top of that work. > > > > Executors don't participate in this at all. There's no ExecutorJobRunner > > and no executor job_type anywhere in airflow-core/src/airflow/jobs/. > > BaseExecutor has a heartbeat() method, but it's purely internal: it > drives > > the executor's own sync() loop, and none of that is written anywhere the > > health endpoint can read. > > > > In practice, if an executor crashes, gets wedged, or loses its connection > > to its backend, there's no dedicated signal for that. Metrics like > > scheduler.executor_heartbeat_duration and executor.open_slots exist if > > enabled (they're off by default) and could in theory be pieced together > by > > someone watching them, but none of that surfaces a dead executor as > clearly > > as heartbeat staleness already does for Scheduler, Triggerer, and > > DagProcessor. What actually reaches the health endpoint today can even be > > misleading: if an executor's heartbeat() call hangs, it shows up as "the > > scheduler is hung," which might be true, but isn't necessarily the real > > cause. > > > > Proposed approach > > > > The goal is to make a stuck or dead executor show up the same way a stuck > > Scheduler or Triggerer already does: through the existing health > endpoint, > > using the existing status / detailed_status / instances shape, so > operators > > have one place to check instead of a special case just for executors. > This > > is meant as a first step, not the whole fix. The longer-term goal is > > letting executors eventually run as independent processes, and this > health > > check is the visibility work that needs to exist before that > restructuring > > can happen safely. > > > > Concretely: > > > > - Dashboard placement: executor health is a new top-level entry alongside > > Scheduler/Triggerer/DagProcessor, not nested under Scheduler. > > - Storage: reuse the Jobs table instead of adding a new one. A new > > ExecutorJob row reports state=RUNNING for liveness, the same bookkeeping > > pattern the other three components already use, plus a new column on the > > Jobs table holding the actual health verdict (ExecutorHealth: > > healthy/unhealthy (reason) / unknown, default unknown). That avoids > forcing > > JobState (RUNNING/SUCCESS/RESTARTING/FAILED) to carry meaning it was > never > > designed for, since none of those values maps cleanly onto executor > status. > > - Mechanism: the scheduler polls each executor on a timer via > > get_executor_health(), a timed call to executor.get_health() that runs > in a > > thread with a timeout and skips a poll if the previous one is still > > pending. It's registered next to the existing > _handle_tasks_stuck_in_queued > > timer. Executors reply healthy, unhealthy (with a reason), or unknown, > > where unknown is the default so an executor that hasn't implemented a > > health check doesn't show up as dead. No reply at all, though, counts as > > unhealthy. Three things here protect the main scheduler loop, which must > > never be delayed: the worker thread, the timeout, and the in-flight > guard. > > The scheduler's other periodic actions, like > _handle_tasks_stuck_in_queued, > > run inline on the main loop thread because they only touch the local > > database and return fast. get_executor_health() can't assume that: it > calls > > out to the executor itself, a broker or a cluster API, and that call can > > hang for reasons the scheduler doesn't control, so it gets its own worker > > thread rather than running inline. The timeout also lives on the > > scheduler's side rather than inside each executor's get_health(). The > > scheduler can't guarantee every executor implementation bounds its own > call > > correctly, so a slow or hung executor should never be the one deciding > how > > long the main loop waits. Finally, the in-flight guard skips a new poll > if > > the previous one hasn't returned yet, so one stuck executor can't pile up > > threads by never responding. > > - Executor-instance identity: for Celery, the broker counts as the > > instance. Celery workers are ephemeral and scale dynamically, so there's > no > > stable per-worker identity to key off (worker concurrency settings cap > how > > many processes run; they don't define an expected worker count). For > > Kubernetes, the pod manager/cluster counts as the instance for the same > > reason: pod counts aren't fixed in config either. LocalExecutor is out of > > scope for now, since its liveness is just the scheduler's own liveness, > so > > a dedicated check wouldn't show anything new. > > - Scope split: PR1 is core scaffolding only, covering the Alembic > migration > > for the new column, the storage and endpoint changes, the ExecutorHealth > > enum aggregating into detailed_status, and tests. PR2 adds the > > CeleryExecutor-specific get_health() override, which is the PoC target > for > > this round. LocalExecutor and Kubernetes each get a named follow-up issue > > instead of a design we'd just be guessing at right now. > > > > What this deliberately doesn't do > > > > - Doesn't design LocalExecutor's or KubernetesExecutor's checks yet. > That's > > a deliberate follow-up, not an oversight. > > - Doesn't pretend the Job-row pattern is a perfect fit just because it's > > precedent. It's reused because it avoids duplicating liveness bookkeeping > > that Scheduler, Triggerer, and DagProcessor already do, not because > > JobState was ever designed with executors in mind. > > - Doesn't try to solve process separation. This is the visibility layer a > > later restructuring would depend on, and it's scoped narrowly on purpose. > > > > Posting this under lazy consensus. Will plan to open the first PR (core > > scaffolding, no behavior change for existing executors) if there's no > > objection in 72 hours. > > > > Joel > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > >
