viiccwen commented on PR #69971: URL: https://github.com/apache/airflow/pull/69971#issuecomment-4999283420
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. -- 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]
