shahar1 commented on PR #69971: URL: https://github.com/apache/airflow/pull/69971#issuecomment-5003979349
> Hello @shahar1, thanks for raising the concern. I took another look at the guarantees provided by Python and the execution model here. > > Although the reference point of `time.monotonic()` is unspecified, current [Python documentation](https://docs.python.org/3/library/time.html#time.monotonic) explicitly states that the monotonic clock is the same for all processes since Python 3.5. Airflow currently supports Python 3.10+, and the supervisor / runner are local processes on the same host, so both calls use the same system-wide monotonic clock. > > On Linux, this is backed by `CLOCK_MONOTONIC`; the value has an unspecified epoch, but values obtained by separate processes on the same host are still comparable. The unspecified reference point only means that the value cannot be interpreted as an absolute timestamp. > > For this metric, we are measuring an **elapsed duration**, so `time.monotonic()` also avoids negative or inflated values caused by wall-clock adjustments such as NTP corrections or manual clock changes. ([ref](https://docs.python.org/3/library/time.html#time.time)) > > I see here: > > https://github.com/apache/airflow/blob/fe9d8650efc45e11e158f67fb6d3518f877877de/airflow-core/src/airflow/jobs/triggerer_job_runner.py#L272-L278 > > then here: > > https://github.com/apache/airflow/blob/fe9d8650efc45e11e158f67fb6d3518f877877de/airflow-core/src/airflow/jobs/triggerer_job_runner.py#L547-L554 > > so the relation is like: Triggerer host/container -> Triggerer main process -> TriggerRunnerSupervisor -> local TriggerRunner subprocess > > IMO, we can add a comment documenting the same-host and cross-process clock assumption, not just use `max(0, time.time() - queued_at)` to hide negative. Thanks for challenging my claim - it seems like you've got a point :) Worth adding a comment as suggested that it relies on the premise of same host. I wonder if at a later point (if we want to allow executions on remote) we should measure it in the supervisor instead. @SameerMesiah97 - FYI, I'll be happy for your feedback. -- 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]
